diff --git a/gradle/changelog/mirror_with_workdir_cache.yaml b/gradle/changelog/mirror_with_workdir_cache.yaml new file mode 100644 index 0000000000..f260803569 --- /dev/null +++ b/gradle/changelog/mirror_with_workdir_cache.yaml @@ -0,0 +1,2 @@ +- type: Fixed + description: Edge cases in mirror command with cached workdirs ([#1812](https://github.com/scm-manager/scm-manager/pull/1812)) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMirrorCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMirrorCommand.java index 278811ca8b..fe115f39a9 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMirrorCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMirrorCommand.java @@ -34,6 +34,7 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevTag; @@ -48,6 +49,7 @@ import org.slf4j.LoggerFactory; import sonia.scm.repository.Changeset; import sonia.scm.repository.GitChangesetConverter; import sonia.scm.repository.GitChangesetConverterFactory; +import sonia.scm.repository.GitHeadModifier; import sonia.scm.repository.GitWorkingCopyFactory; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Tag; @@ -99,14 +101,16 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman private final GitChangesetConverterFactory converterFactory; private final GitTagConverter gitTagConverter; private final GitWorkingCopyFactory workingCopyFactory; + private final GitHeadModifier gitHeadModifier; @Inject - GitMirrorCommand(GitContext context, MirrorHttpConnectionProvider mirrorHttpConnectionProvider, GitChangesetConverterFactory converterFactory, GitTagConverter gitTagConverter, GitWorkingCopyFactory workingCopyFactory) { + GitMirrorCommand(GitContext context, MirrorHttpConnectionProvider mirrorHttpConnectionProvider, GitChangesetConverterFactory converterFactory, GitTagConverter gitTagConverter, GitWorkingCopyFactory workingCopyFactory, GitHeadModifier gitHeadModifier) { super(context); this.mirrorHttpConnectionProvider = mirrorHttpConnectionProvider; this.converterFactory = converterFactory; this.gitTagConverter = gitTagConverter; this.workingCopyFactory = workingCopyFactory; + this.gitHeadModifier = gitHeadModifier; } @Override @@ -116,7 +120,23 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman @Override public MirrorCommandResult update(MirrorCommandRequest mirrorCommandRequest) { - return inClone(git -> new Worker(context, mirrorCommandRequest, repository, git), workingCopyFactory, null); + // we have to select an existing branch here (and we cannot depend on a correct "default branch" in the + // configuration), because otherwise the clone will fail with an unresolvable branch. + String headBranch = resolveHeadBranch(); + return inClone(git -> new Worker(context, mirrorCommandRequest, this.repository, git), workingCopyFactory, headBranch); + } + + private String resolveHeadBranch() { + try { + Repository repository = context.open(); + Ref headRef = repository.findRef(Constants.HEAD); + if (headRef == null) { + throw new InternalRepositoryException(context.getRepository(), "Cannot handle missing HEAD in repository"); + } + return headRef.getTarget().getName().substring("refs/heads/".length()); + } catch (IOException e) { + throw new InternalRepositoryException(context.getRepository(), "could not resolve HEAD", e); + } } private class Worker extends GitCloneWorker { @@ -146,6 +166,11 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman * pushing an empty ref to the central repository. */ private boolean deleteMasterAfterInitialSync = false; + /** + * We store a branch that has not been rejected here, so we can easily correct the HEAD reference + * afterwards (see #setHeadIfMirroredBranchExists) + */ + private String acceptedBranch; private Worker(GitContext context, MirrorCommandRequest mirrorCommandRequest, sonia.scm.repository.Repository repository, Git git) { super(git, context, repository); @@ -179,12 +204,37 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman } push(generatePushRefSpecs().toArray(new String[0])); + setHeadIfMirroredBranchExists(); cleanUpMasterIfNecessary(); return new MirrorCommandResult(result, mirrorLog, stopwatch.stop().elapsed()); } + private void setHeadIfMirroredBranchExists() { + if (acceptedBranch != null) { + // Ensures that HEAD is set to an existing mirrored branch in the working copy (if this is set to a branch that + // should not have been mirrored, this branch cannot be deleted otherwise; see #cleanupMasterIfNecessary) and + // in the "real" mirror repository (here a HEAD with a not existing branch will lead to errors in the next clone + // call). + try { + RefUpdate refUpdate = git.getRepository().getRefDatabase().newUpdate(Constants.HEAD, true); + refUpdate.setForceUpdate(true); + refUpdate.link(acceptedBranch); + } catch (IOException e) { + throw new InternalRepositoryException(getRepository(), "Error while setting HEAD", e); + } + gitHeadModifier.ensure(repository, acceptedBranch.substring("refs/heads/".length())); + } + } + private void cleanUpMasterIfNecessary() { if (deleteMasterAfterInitialSync) { + try { + // we have to delete the master branch in the working copy, because otherwise it may be pushed + // to the mirror in the next synchronization call, when the working directory is cached. + git.branchDelete().setBranchNames("master").setForce(true).call(); + } catch (GitAPIException e) { + LOG.error("Could not delete master branch in mirror repository {}", getRepository().getNamespaceAndName(), e); + } push(":refs/heads/master"); } } @@ -269,7 +319,11 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman private Result testFilterForBranch() { try { - return filter.acceptBranch(filterContext.getBranchUpdate(ref.getLocalName())); + Result filterResult = filter.acceptBranch(filterContext.getBranchUpdate(ref.getLocalName())); + if (filterResult.isAccepted()) { + acceptedBranch = ref.getLocalName(); + } + return filterResult; } catch (Exception e) { return handleExceptionFromFilter(e); } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMirrorCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMirrorCommandTest.java index 3dcad14627..5aa640dac7 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMirrorCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMirrorCommandTest.java @@ -30,10 +30,13 @@ import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.junit.http.AppServer; import org.eclipse.jgit.junit.http.SimpleHttpServer; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevWalk; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -43,13 +46,17 @@ import sonia.scm.net.GlobalProxyConfiguration; import sonia.scm.net.HttpURLConnectionFactory; import sonia.scm.repository.GitChangesetConverterFactory; import sonia.scm.repository.GitConfig; +import sonia.scm.repository.GitHeadModifier; +import sonia.scm.repository.GitUtil; import sonia.scm.repository.api.MirrorCommandResult; import sonia.scm.repository.api.MirrorFilter; import sonia.scm.repository.api.SimpleUsernamePasswordCredential; import sonia.scm.repository.work.NoneCachingWorkingCopyPool; +import sonia.scm.repository.work.SimpleWorkingCopyFactory; import sonia.scm.repository.work.WorkdirProvider; import sonia.scm.security.GPG; import sonia.scm.store.InMemoryConfigurationStoreFactory; +import sonia.scm.util.IOUtil; import javax.net.ssl.TrustManager; import java.io.File; @@ -80,9 +87,10 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase { private final GitTagConverter gitTagConverter = new GitTagConverter(gpg); private File clone; + private File workdirAfterClose; private GitMirrorCommand command; - + private final GitHeadModifier gitHeadModifier = mock(GitHeadModifier.class); @Before public void bendContextToNewRepository() throws IOException, GitAPIException { @@ -92,7 +100,9 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase { GitContext emptyContext = createMirrorContext(clone); SimpleGitWorkingCopyFactory workingCopyFactory = new SimpleGitWorkingCopyFactory( - new NoneCachingWorkingCopyPool(new WorkdirProvider(repositoryLocationResolver)), new SimpleMeterRegistry()); + new KeepingWorkingCopyPool(new WorkdirProvider(repositoryLocationResolver)), + new SimpleMeterRegistry() + ); MirrorHttpConnectionProvider mirrorHttpConnectionProvider = new MirrorHttpConnectionProvider( new HttpURLConnectionFactory( @@ -106,7 +116,15 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase { mirrorHttpConnectionProvider, gitChangesetConverterFactory, gitTagConverter, - workingCopyFactory); + workingCopyFactory, + gitHeadModifier); + } + + @After + public void cleanupWorkdir() { + if (workdirAfterClose != null) { + IOUtil.deleteSilently(workdirAfterClose); + } } @Test @@ -154,6 +172,10 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase { assertThat(createdMirror.branchList().call().stream().filter(r -> r.getName().contains("master")).findAny()) .isEmpty(); } + try (Repository workdirRepository = GitUtil.open(workdirAfterClose)) { + assertThat(workdirRepository.findRef(Constants.HEAD).getTarget().getName()).isEqualTo("refs/heads/test-branch"); + } + verify(gitHeadModifier).ensure(repository, "test-branch"); } @Test @@ -840,4 +862,16 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase { private interface InvocationCheck { void invoked(); } + + private class KeepingWorkingCopyPool extends NoneCachingWorkingCopyPool { + + public KeepingWorkingCopyPool(WorkdirProvider workdirProvider) { + super(workdirProvider); + } + + @Override + public void contextClosed(SimpleWorkingCopyFactory.WorkingCopyContext workingCopyContext, File workdir) { + workdirAfterClose = workdir; + } + } }