Removing a site or platform could remove all of /var/aegir

I agree that this should not be possible.

Deleting probably happens in drush_provision_git_post_provision_delete()

Finding out the top level of the Git repo is done in provision_git_is_repo(). Since we have the repo_docroot property from #2838489: Support drupal being in a subdirectory of a git repo we might be able to make that code a bit smarter...

PS: I'm not sure if it's practical to keep the whole /var/aegir in Git ... I have /var/aegir/config in Git on some servers via etckeeper.

I created a quick test repo for this https://gitlab.com/aegir/test_drupal_docroot_in_subdir but trying to get that working as it should is already a challenge :(

In the mean time I've disabled the line that's doing the delete.

Here's what happened:

In #2544906: Pickup git setting from disk a patch was committed that imported the repo_url and git_ref property by running git commands in the $repo_path directory.

The problem is, it did this, whether or not $repo_path was an actual git repo. So when you verified a site, it ran "git config --local --get remote.origin.url" in the sites/domain.com folder, and it got a good result from /var/aegir/.git or from a platform that was a git clone.

So I'm fixing that over in that issue.

For this problem, we just need to add some extra checks here, to be sure $repo_path is the right repo_path. We could also add a dead-man's switch to prevent us from attempting to delete /var/aegir in the case that repo_root got changed to that somehow.

So if you had /var/aegir/.git, and you've been using hosting_git since that patch has been committed, you should check your site and platform nodes and contexts for incorrect git repo information.

I've committed this failsafe, and have addressed the other issue in #2544906: Pickup git setting from disk.

+
+  // Do not allow /var/aegir to be repo_path.
+  // See https://www.drupal.org/node/2893588
+  if ($repo_path == '/var/aegir') {
+    $repo_path = NULL;
+    drush_set_option('repo_path', $repo_path);
+  }
+

This check happens even if your repo path is already set to /var/aegir.

Helmo: Can you review and approve this, and then revert this commit: http://cgit.drupalcode.org/hosting_git/commit/?id=e8d4fbc

Hmm we should add a drush warning there, shouldn't we? This is bad data that we can't automatically fix.

Oops! turned out I already did. Needed improvement...

Here's the final result:

I decided a hard error was needed here. If /var/aegir is set to repo root, there's a problem that needs to be fixed.

+
+  // Do not allow /var/aegir to be repo_path.
+  // See https://www.drupal.org/node/2893588
+  if ($repo_path == '/var/aegir') {
+    $repo_path = NULL;
+    drush_set_option('repo_path', $repo_path);
+    return drush_set_error('PROVISION_GIT_ERROR', dt('Warning: The "repo_path" for this !type is set to "/var/aegir". This is not allowed. Please manually edit the drush alias file in /var/aegir/.drush/!file.alias.drushrc.php.  Option repo_path has been unset.', array(
+      '!file' => ltrim(d()->name, '@'),
+      '!type' => d()->type,
+    )));
+  }
+

Looks good.

Reopening.

provision-delete on a platform is still reporting:

NOT deleteing See https://www.drupal.org/node/2893588

However, the platform codebase is removed, but the drush alias file is not removed...

Looking at your error message d()->repo_path was probably empty ... what about this extra test?

committed.