diff --git a/modules/nextflow/src/main/groovy/nextflow/scm/MultiRevisionRepositoryStrategy.groovy b/modules/nextflow/src/main/groovy/nextflow/scm/MultiRevisionRepositoryStrategy.groovy index fc0233d5cc..1e7c0b1e90 100644 --- a/modules/nextflow/src/main/groovy/nextflow/scm/MultiRevisionRepositoryStrategy.groovy +++ b/modules/nextflow/src/main/groovy/nextflow/scm/MultiRevisionRepositoryStrategy.groovy @@ -30,10 +30,13 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException import org.eclipse.jgit.internal.storage.file.FileRepository import org.eclipse.jgit.lib.Constants import org.eclipse.jgit.lib.Ref +import org.eclipse.jgit.lib.RefUpdate import org.eclipse.jgit.lib.Repository import org.eclipse.jgit.lib.SubmoduleConfig import org.eclipse.jgit.storage.file.FileRepositoryBuilder +import org.eclipse.jgit.transport.FetchResult import org.eclipse.jgit.transport.RefSpec +import org.eclipse.jgit.transport.TrackingRefUpdate /** * Multi-revision repository strategy that uses a bare repository with shared clones. @@ -183,7 +186,7 @@ class MultiRevisionRepositoryStrategy extends AbstractRepositoryStrategy { if( provider.hasCredentials() ) { fetch.setCredentialsProvider(provider.getGitCredentials()) } - fetch.call() + verifyFetchResult(fetch.call(), updateRevision, "bare repo") } private void createBareRepo(Manifest manifest) { @@ -323,7 +326,7 @@ class MultiRevisionRepositoryStrategy extends AbstractRepositoryStrategy { .setRefSpecs(refSpecForName(downloadRevision)) if( recurseSubmodules ) fetch.setRecurseSubmodules(SubmoduleConfig.FetchRecurseSubmodulesMode.YES) - fetch.call() + verifyFetchResult(fetch.call(), downloadRevision, "shared clone") getCommitGit().checkout().setName(downloadRevision).call() } catch( Throwable t ) { @@ -341,7 +344,8 @@ class MultiRevisionRepositoryStrategy extends AbstractRepositoryStrategy { final branchName = "refs/heads/$revision".toString() final branch = getBareGit().getRepository().findRef(branchName) if( branch != null ) { - return new RefSpec("$branchName:$branchName") + // force-update: branches are mutable and may be force-pushed remotely + return new RefSpec("+$branchName:$branchName") } // Check if it's a local tag @@ -356,7 +360,7 @@ class MultiRevisionRepositoryStrategy extends AbstractRepositoryStrategy { // Is it a remote branch? if( remoteRefs.containsKey(branchName) ) { - return new RefSpec("$branchName:$branchName") + return new RefSpec("+$branchName:$branchName") } // Is it a remote tag? @@ -368,6 +372,22 @@ class MultiRevisionRepositoryStrategy extends AbstractRepositoryStrategy { return new RefSpec("$revision:$tagName") } + private void verifyFetchResult(FetchResult result, String revision, String repoLabel) { + for( TrackingRefUpdate update : result.getTrackingRefUpdates() ) { + final r = update.result + final detail = "${project} [revision: $revision]: ${update.localName} <- ${update.remoteName} => $r (old=${update.oldObjectId?.name()}, new=${update.newObjectId?.name()})" + if( r == RefUpdate.Result.FORCED ) { + log.warn "Non-fast-forward (force) update on $repoLabel for $detail" + } + else if( r == RefUpdate.Result.NEW || r == RefUpdate.Result.FAST_FORWARD || r == RefUpdate.Result.NO_CHANGE ) { + log.debug "${repoLabel.capitalize()} fetch update for $detail" + } + else { + throw new AbortOperationException("Unable to update revision '${revision}' in $repoLabel for ${project}: ${update.localName} => $r") + } + } + } + @Override File getLocalPath() { return this.commitPath ?: legacyRepoPath diff --git a/modules/nextflow/src/test/groovy/nextflow/scm/MultiRevisionRepositoryStrategyTest.groovy b/modules/nextflow/src/test/groovy/nextflow/scm/MultiRevisionRepositoryStrategyTest.groovy index 849b8f5ce2..99b3d80de8 100644 --- a/modules/nextflow/src/test/groovy/nextflow/scm/MultiRevisionRepositoryStrategyTest.groovy +++ b/modules/nextflow/src/test/groovy/nextflow/scm/MultiRevisionRepositoryStrategyTest.groovy @@ -17,7 +17,13 @@ package nextflow.scm import nextflow.config.Manifest +import nextflow.exception.AbortOperationException import org.eclipse.jgit.api.Git +import org.eclipse.jgit.lib.ObjectId +import org.eclipse.jgit.lib.RefUpdate +import org.eclipse.jgit.transport.FetchResult +import org.eclipse.jgit.transport.RefSpec +import org.eclipse.jgit.transport.TrackingRefUpdate import static MultiRevisionRepositoryStrategy.BARE_REPO import static MultiRevisionRepositoryStrategy.REVISION_SUBDIR @@ -211,4 +217,124 @@ class MultiRevisionRepositoryStrategyTest extends Specification { folder.resolve(REPOS_SUBDIR + '/nextflow-io/hello/' + REVISION_SUBDIR + '/1b420d060d3fad67027154ac48e3bdea06f058da/.git').isDirectory() } + private TrackingRefUpdate mockUpdate(RefUpdate.Result result, String localName = 'refs/heads/main', String remoteName = 'refs/heads/main') { + Mock(TrackingRefUpdate) { + getResult() >> result + getLocalName() >> localName + getRemoteName() >> remoteName + getOldObjectId() >> ObjectId.fromString('3851da8d5cfe3f451dc1ff6d3f41e67b91cf2219') + getNewObjectId() >> ObjectId.fromString('d17b807dc70a0f88757addf4960b2d905b3b13bc') + } + } + + def 'verifyFetchResult should pass silently for happy results'() { + given: + def strategy = new MultiRevisionRepositoryStrategy('test/project') + def fetchResult = Mock(FetchResult) { + getTrackingRefUpdates() >> [mockUpdate(resultCode)] + } + + when: + strategy.invokeMethod('verifyFetchResult', [fetchResult, 'main', 'bare repo'] as Object[]) + + then: + noExceptionThrown() + + where: + resultCode << [RefUpdate.Result.NEW, RefUpdate.Result.FAST_FORWARD, RefUpdate.Result.NO_CHANGE] + } + + def 'verifyFetchResult should warn but not throw on FORCED'() { + given: + def strategy = new MultiRevisionRepositoryStrategy('test/project') + def fetchResult = Mock(FetchResult) { + getTrackingRefUpdates() >> [mockUpdate(RefUpdate.Result.FORCED)] + } + + when: + strategy.invokeMethod('verifyFetchResult', [fetchResult, 'main', 'bare repo'] as Object[]) + + then: + noExceptionThrown() + } + + def 'verifyFetchResult should abort on rejected or failure results'() { + given: + def strategy = new MultiRevisionRepositoryStrategy('test/project') + def fetchResult = Mock(FetchResult) { + getTrackingRefUpdates() >> [mockUpdate(resultCode, 'refs/tags/v1.0', 'refs/tags/v1.0')] + } + + when: + strategy.invokeMethod('verifyFetchResult', [fetchResult, 'v1.0', 'bare repo'] as Object[]) + + then: + def ex = thrown(AbortOperationException) + ex.message.contains('v1.0') + ex.message.contains('bare repo') + + where: + resultCode << [ + RefUpdate.Result.REJECTED, + RefUpdate.Result.LOCK_FAILURE, + RefUpdate.Result.IO_FAILURE, + RefUpdate.Result.REJECTED_OTHER_REASON, + RefUpdate.Result.REJECTED_MISSING_OBJECT + ] + } + + def 'should pick up force-pushed branch on subsequent fetch'() { + given: 'a bare upstream repo' + def folder = tempDir.getRoot() + def upstreamDir = folder.resolve('upstream.git').toFile() + upstreamDir.mkdirs() + Git.init().setDirectory(upstreamDir).setBare(true).setInitialBranch('main').call().close() + + and: 'a working repo with an initial commit on main, pushed to upstream' + def workDir = folder.resolve('work').toFile() + def work = Git.init().setDirectory(workDir).setInitialBranch('main').call() + new File(workDir, 'a.txt').text = 'A' + work.add().addFilepattern('a.txt').call() + def commitA = work.commit().setMessage('A').call() + work.push().setRemote(upstreamDir.absolutePath).setRefSpecs(new RefSpec('refs/heads/main:refs/heads/main')).call() + work.close() + + and: 'a strategy targeting the local upstream' + def provider = Mock(RepositoryProvider) { + hasCredentials() >> false + getCloneUrl() >> upstreamDir.absolutePath + } + def strategy = new MultiRevisionRepositoryStrategy('test/forced', 'main') + strategy.setProvider(provider) + def manifest = Mock(Manifest) { + getDefaultBranch() >> 'main' + getRecurseSubmodules() >> false + } + + when: 'first fetch creates the bare repo' + strategy.checkBareRepo(manifest) + + then: 'local bare repo points at commit A' + def bareGit = Git.open(strategy.getBareRepo()) + bareGit.getRepository().resolve('refs/heads/main').name() == commitA.name() + bareGit.close() + + when: 'upstream main is force-pushed to a divergent commit' + work = Git.open(workDir) + new File(workDir, 'a.txt').text = 'B' + work.add().addFilepattern('a.txt').call() + def commitB = work.commit().setAmend(true).setMessage('B').call() + work.push().setRemote(upstreamDir.absolutePath).setForce(true).setRefSpecs(new RefSpec('refs/heads/main:refs/heads/main')).call() + work.close() + + and: 'strategy fetches again' + strategy.checkBareRepo(manifest) + + then: 'local bare repo now points at commit B' + def bareGitB = Git.open(strategy.getBareRepo()) + bareGitB.getRepository().resolve('refs/heads/main').name() == commitB.name() + commitB.name() != commitA.name() + bareGitB.close() + } + }