Pick up force-pushed branches in multi-revision repo strategy

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

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()
+    }
+
 }