Fix default branch in git mirror command

The git mirror command did not set the default branch correctly in all cases. If the source repository contained a 'master' branch, no default branch was set. This led to an error in the refresh process, when globally another branch has been configured as dafault (when not changed, this is 'main'), because the working copy should be created with the main branch checked out. Because no default was specified, the globally configured default (by default 'main') had been taken and that branch does not exist in the mirror.

So all subsequent mirror updates failed with a message like this one:

could not find branch with id main in repository with id ...

With this fix, the default branch is always set in the SCM-Manager configuration for the repository and therefore cannot be missing in subsequent updates.

Committed-by: Konstantin Schaper <konstantin.schaper@cloudogu.com>
Co-authored-by: René Pfeuffer <rene.pfeuffer@cloudogu.com>

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMirrorCommand.java

@@ -26,6 +26,7 @@ package sonia.scm.repository.spi;
 
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Strings;
+import org.apache.commons.lang.StringUtils;
 import org.eclipse.jgit.api.FetchCommand;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
@@ -214,7 +215,9 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman
         return new MirrorCommandResult(result, mirrorLog, stopwatch.stop().elapsed(), lfsUpdateResult);
       }
 
-      defaultBranchSelector.newDefaultBranch().ifPresent(this::setNewDefaultBranch);
+      String currentDefaultBranchInRepository = getCurrentDefaultBranch();
+      defaultBranchSelector.newDefaultBranch(currentDefaultBranchInRepository)
+        .ifPresent(this::setNewDefaultBranch);
 
       String[] pushRefSpecs = generatePushRefSpecs().toArray(new String[0]);
       forcePush(pushRefSpecs);
@@ -223,17 +226,18 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman
     }
 
     private void setNewDefaultBranch(String newDefaultBranch) {
-      mirrorLog.add("Old default branch deleted. Setting default branch to '" + newDefaultBranch + "'.");
-
       try {
-        String oldBranch = git.getRepository().getBranch();
-        RefUpdate refUpdate = git.getRepository().getRefDatabase().newUpdate(Constants.HEAD, true);
-        refUpdate.setForceUpdate(true);
-        RefUpdate.Result result = refUpdate.link(Constants.R_HEADS + newDefaultBranch);
-        if (result != RefUpdate.Result.FORCED) {
-          throw new InternalRepositoryException(getRepository(), "Could not set HEAD to new default branch");
+        String oldBranch = getCurrentDefaultBranch();
+        if (!StringUtils.equals(oldBranch, newDefaultBranch)) {
+          mirrorLog.add("Old default branch deleted. Setting default branch to '" + newDefaultBranch + "'.");
+          RefUpdate refUpdate = git.getRepository().getRefDatabase().newUpdate(Constants.HEAD, true);
+          refUpdate.setForceUpdate(true);
+          RefUpdate.Result result = refUpdate.link(Constants.R_HEADS + newDefaultBranch);
+          if (result != RefUpdate.Result.FORCED) {
+            throw new InternalRepositoryException(getRepository(), "Could not set HEAD to new default branch");
+          }
+          git.branchDelete().setBranchNames(oldBranch).setForce(true).call();
         }
-        git.branchDelete().setBranchNames(oldBranch).setForce(true).call();
       } catch (GitAPIException | IOException e) {
         throw new InternalRepositoryException(getRepository(), "Error while switching branch to change default branch", e);
       }
@@ -242,6 +246,14 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman
       storeProvider.setDefaultBranch(repository, newDefaultBranch);
     }
 
+    private String getCurrentDefaultBranch() {
+      try {
+        return git.getRepository().getBranch();
+      } catch (IOException e) {
+        return null;
+      }
+    }
+
     private Collection<String> generatePushRefSpecs() {
       Collection<String> refSpecs = new ArrayList<>();
       refSpecs.add("refs/heads/*:refs/heads/*");
@@ -759,9 +771,9 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman
       return changed;
     }
 
-    public Optional<String> newDefaultBranch() {
-      if (initialDefaultBranch == null && newBranches.contains("master") || remainingBranches.contains(initialDefaultBranch)) {
-        return empty();
+    public Optional<String> newDefaultBranch(String currentDefaultBranchInRepository) {
+      if (initialDefaultBranch == null && newBranches.contains(currentDefaultBranchInRepository) || remainingBranches.contains(initialDefaultBranch)) {
+        return of(currentDefaultBranchInRepository);
       } else if (!newBranches.isEmpty() && initialBranches.isEmpty()) {
         return of(newBranches.iterator().next());
       } else if (initialDefaultBranch == null && newBranches.isEmpty()) {

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMirrorCommandTest.java

@@ -78,6 +78,7 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static sonia.scm.repository.api.MirrorCommandResult.ResultType.FAILED;
 import static sonia.scm.repository.api.MirrorCommandResult.ResultType.OK;
@@ -165,7 +166,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
 
   @Test
   public void shouldAcceptEmptyInitialMirror() throws IOException, GitAPIException {
-    MirrorCommandResult result = callMirrorCommand(repositoryDirectory.getAbsolutePath(), c -> {
+    MirrorCommandResult result = callMirrorCommand(repositoryDirectory.getAbsolutePath(), c ->
       c.setFilter(new MirrorFilter() {
         @Override
         public Filter getFilter(FilterContext context) {
@@ -181,8 +182,8 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
             }
           };
         }
-      });
-    });
+      })
+    );
 
     assertThat(result.getResult()).isEqualTo(REJECTED_UPDATES);
     assertThat(result.getLog()).contains("Branches:")
@@ -200,7 +201,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
   @Test
   public void shouldAcceptOnlyTagInInitialMirror() {
     assertThrows(IllegalStateException.class, () ->
-      callMirrorCommand(repositoryDirectory.getAbsolutePath(), c -> {
+      callMirrorCommand(repositoryDirectory.getAbsolutePath(), c ->
         c.setFilter(new MirrorFilter() {
           @Override
           public Filter getFilter(FilterContext context) {
@@ -211,13 +212,13 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
               }
             };
           }
-        });
-      }));
+        })
+      ));
   }
 
   @Test
   public void shouldFilterMasterBranchWhenFilteredOnInitialMirror() throws IOException, GitAPIException {
-    MirrorCommandResult result = callMirrorCommand(repositoryDirectory.getAbsolutePath(), c -> {
+    MirrorCommandResult result = callMirrorCommand(repositoryDirectory.getAbsolutePath(), c ->
       c.setFilter(new MirrorFilter() {
         @Override
         public Filter getFilter(FilterContext context) {
@@ -232,8 +233,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
             }
           };
         }
-      });
-    });
+      }));
 
     assertThat(result.getResult()).isEqualTo(REJECTED_UPDATES);
     assertThat(result.getLog())
@@ -808,6 +808,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
   @Test
   public void shouldSelectNewHeadIfOldHeadIsDeleted() throws IOException, GitAPIException {
     callMirrorCommand();
+    reset(storeProvider);
 
     try (Git updatedSource = Git.open(repositoryDirectory)) {
       updatedSource.checkout().setName("test-branch").call();
@@ -879,7 +880,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       selector.accepted(BRANCH, "master");
       selector.accepted(BRANCH, "something");
 
-      assertThat(selector.newDefaultBranch()).isEmpty();
+      assertThat(selector.newDefaultBranch("master")).get().isEqualTo("master");
     }
 
     @Test
@@ -890,7 +891,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       selector.accepted(BRANCH, "new");
       selector.deleted(BRANCH, "two");
 
-      assertThat(selector.newDefaultBranch()).isEmpty();
+      assertThat(selector.newDefaultBranch("master")).get().isEqualTo("master");
     }
 
     @Test
@@ -900,7 +901,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
 
       selector.deleted(BRANCH, "master");
 
-      assertThat(selector.newDefaultBranch()).get().isIn("one", "two", "three");
+      assertThat(selector.newDefaultBranch("master")).get().isIn("one", "two", "three");
     }
 
     @Test
@@ -912,7 +913,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       selector.deleted(BRANCH, "one");
       selector.deleted(BRANCH, "three");
 
-      assertThat(selector.newDefaultBranch()).get().isEqualTo("two");
+      assertThat(selector.newDefaultBranch("master")).get().isEqualTo("two");
     }
 
     @Test
@@ -924,7 +925,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       selector.deleted(BRANCH, "one");
       selector.deleted(BRANCH, "three");
 
-      assertThat(selector.newDefaultBranch()).get().isEqualTo("two");
+      assertThat(selector.newDefaultBranch("master")).get().isEqualTo("two");
     }
 
     @Test
@@ -938,7 +939,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       selector.accepted(BRANCH, "new");
       selector.deleted(BRANCH, "three");
 
-      assertThrows(IllegalStateException.class, selector::newDefaultBranch);
+      assertThrows(IllegalStateException.class, () -> selector.newDefaultBranch("master"));
     }
 
     @Test
@@ -949,7 +950,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       selector.accepted(BRANCH, "main");
       selector.deleted(BRANCH, "master");
 
-      assertThat(selector.newDefaultBranch()).get().isEqualTo("main");
+      assertThat(selector.newDefaultBranch("master")).get().isEqualTo("main");
     }
 
     @Test
@@ -959,7 +960,7 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
 
       selector.accepted(BRANCH, "main");
 
-      assertThat(selector.newDefaultBranch()).get().isEqualTo("main");
+      assertThat(selector.newDefaultBranch("master")).get().isEqualTo("main");
     }
   }
 
@@ -1113,4 +1114,10 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       workdirAfterClose = workdir;
     }
   }
+
+  @Override
+  protected String getZippedRepositoryResource()
+  {
+    return "sonia/scm/repository/spi/scm-git-spi-mirror-test.zip";
+  }
 }