Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-58716

Failure to check out specified tag for incremental version

    XMLWordPrintable

Details

    Description

      I tried to run the PCT against an incremental release of a plugin, and it failed:

      Created plugin checkout dir : .../pct-work/jsch
      Checking out from SCM connection URL : scm:git:git://github.com/jenkinsci/jsch-plugin.git (jsch-0.1.55.1-rc41.4943eb07c811)
      [INFO] Executing: /bin/sh -c cd '.../pct-work' && 'git' 'clone' 'git://github.com/jenkinsci/jsch-plugin.git' '.../pct-work/jsch'
      [INFO] Working directory: .../pct-work
      [INFO] Executing: /bin/sh -c cd '.../pct-work/jsch' && 'git' 'fetch' 'git://github.com/jenkinsci/jsch-plugin.git'
      [INFO] Working directory: .../pct-work/jsch
      [INFO] Executing: /bin/sh -c cd '.../pct-work/jsch' && 'git' 'checkout' 'jsch-0.1.55.1-rc41.4943eb07c811'
      [INFO] Working directory: .../pct-work/jsch
      Error : The git-checkout command failed. || Cloning into '.../pct-work/jsch'...
      From git://github.com/jenkinsci/jsch-plugin
       * branch            HEAD       -> FETCH_HEAD
      error: pathspec 'jsch-0.1.55.1-rc41.4943eb07c811' did not match any file(s) known to git
      

      That is because this code improperly assumes that ${project.artifactId}-${project.version} is going to be a valid tag. Maven makes no such guarantee. Rather, this code should be looking for a /project/scm/tag and honoring it if present. In this case it would have found

          <tag>4943eb07c81131909f1d3b16bf18dec8a8b91a1b</tag>
      

      which is, in fact, the correct hash to check out.

      Attachments

        Issue Links

          Activity

            basil Basil Crow added a comment -

            I got a prototype of this working with CHANGE_FORK as follows:

            diff --git a/git-changelist-maven-extension/src/main/java/io/jenkins/tools/incrementals/git_changelist_maven_extension/Main.java b/git-changelist-maven-extension/src/main/java/io/jenkins/tools/incrementals/git_changelist_maven_extension/Main.java
            index 9e9220a..bd17830 100644
            --- a/git-changelist-maven-extension/src/main/java/io/jenkins/tools/incrementals/git_changelist_maven_extension/Main.java
            +++ b/git-changelist-maven-extension/src/main/java/io/jenkins/tools/incrementals/git_changelist_maven_extension/Main.java
            @@ -137,6 +137,15 @@ public class Main extends AbstractMavenLifecycleParticipant {
                         } else {
                             log.info("Declining to override the `changelist` or `scmTag` properties");
                         }
            +            String changeFork = System.getenv("CHANGE_FORK");
            +            if (changeFork != null) {
            +                if (!props.containsKey("gitHubOrg")) {
            +                    log.info("Setting: -DgitHubOrg=" + changeFork);
            +                    props.setProperty("gitHubOrg", changeFork);
            +                } else {
            +                    log.info("Declining to override the `gitHubOrg` property");
            +                }
            +            }
                     } else {
                         log.debug("Skipping Git version setting unless run with -Dset.changelist");
                     }
            

            This prototype helped me understand that CHANGE_FORK would work if we used two properties in the POM (say, gitHubOrg and gitHubProject), but it CHANGE_FORK wouldn't work if we used one property in the POM (say, gitHubRepo). That's because we'd need to set that property in git-changelist-maven-extension, and at the time git-changelist-maven-extension is running we only have access to the incomplete information in CHANGE_FORK and not the complete information from the parsed POM.

            So I think we have a choice: either continue this Knuthian yak shave into implementing a CHANGE_FORK_FULL solution for JENKINS-58450, settle for imperfection with CHANGE_FORK and two POM properties, or give up. If you're willing to review and merge the PRs, I'm down to continue the yak shave. What do you think?

            basil Basil Crow added a comment - I got a prototype of this working with CHANGE_FORK as follows: diff --git a/git-changelist-maven-extension/src/main/java/io/jenkins/tools/incrementals/git_changelist_maven_extension/Main.java b/git-changelist-maven-extension/src/main/java/io/jenkins/tools/incrementals/git_changelist_maven_extension/Main.java index 9e9220a..bd17830 100644 --- a/git-changelist-maven-extension/src/main/java/io/jenkins/tools/incrementals/git_changelist_maven_extension/Main.java +++ b/git-changelist-maven-extension/src/main/java/io/jenkins/tools/incrementals/git_changelist_maven_extension/Main.java @@ -137,6 +137,15 @@ public class Main extends AbstractMavenLifecycleParticipant { } else { log.info("Declining to override the `changelist` or `scmTag` properties"); } + String changeFork = System.getenv("CHANGE_FORK"); + if (changeFork != null) { + if (!props.containsKey("gitHubOrg")) { + log.info("Setting: -DgitHubOrg=" + changeFork); + props.setProperty("gitHubOrg", changeFork); + } else { + log.info("Declining to override the `gitHubOrg` property"); + } + } } else { log.debug("Skipping Git version setting unless run with -Dset.changelist"); } This prototype helped me understand that CHANGE_FORK would work if we used two properties in the POM (say, gitHubOrg and gitHubProject ), but it CHANGE_FORK wouldn't work if we used one property in the POM (say, gitHubRepo ). That's because we'd need to set that property in git-changelist-maven-extension , and at the time git-changelist-maven-extension is running we only have access to the incomplete information in CHANGE_FORK and not the complete information from the parsed POM. So I think we have a choice: either continue this Knuthian yak shave into implementing a CHANGE_FORK_FULL solution for JENKINS-58450 , settle for imperfection with CHANGE_FORK and two POM properties, or give up. If you're willing to review and merge the PRs, I'm down to continue the yak shave. What do you think?
            jglick Jesse Glick added a comment -

            I am happy to review PRs for JENKINS-58450 if that is what you are proposing, though I do not own these plugins so I cannot make any guarantees about a timeline for release. I am reluctant to go with the two-property trick since that might make it more complicated to do the right thing later.

            From https://github.com/jenkinsci/build-token-root-plugin/pull/21 FTR, the remotes are

             origin	https://github.com/jenkinsci/build-token-root-plugin.git (fetch)
             origin	https://github.com/jenkinsci/build-token-root-plugin.git (push)
            

            and some interesting env vars are:

             BRANCH_NAME=PR-21
             BUILD_URL=https://ci.jenkins.io/job/Plugins/job/build-token-root-plugin/job/PR-21/1/
             CHANGE_BRANCH=remotes
             CHANGE_FORK=jglick
             CHANGE_ID=21
             CHANGE_TARGET=master
             CHANGE_URL=https://github.com/jenkinsci/build-token-root-plugin/pull/21
             JOB_NAME=Plugins/build-token-root-plugin/PR-21
             JOB_URL=https://ci.jenkins.io/job/Plugins/job/build-token-root-plugin/job/PR-21/
            

            As a temporary trick, I think Main could define gitHubRepo based on CHANGE_FORK plus an appropriately processed part of JOB_NAME, yielding in this case jglick/build-token-root-plugin.

            jglick Jesse Glick added a comment - I am happy to review PRs for JENKINS-58450 if that is what you are proposing, though I do not own these plugins so I cannot make any guarantees about a timeline for release. I am reluctant to go with the two-property trick since that might make it more complicated to do the right thing later. From https://github.com/jenkinsci/build-token-root-plugin/pull/21 FTR, the remotes are origin https://github.com/jenkinsci/build-token-root-plugin.git (fetch) origin https://github.com/jenkinsci/build-token-root-plugin.git (push) and some interesting env vars are: BRANCH_NAME=PR-21 BUILD_URL=https://ci.jenkins.io/job/Plugins/job/build-token-root-plugin/job/PR-21/1/ CHANGE_BRANCH=remotes CHANGE_FORK=jglick CHANGE_ID=21 CHANGE_TARGET=master CHANGE_URL=https://github.com/jenkinsci/build-token-root-plugin/pull/21 JOB_NAME=Plugins/build-token-root-plugin/PR-21 JOB_URL=https://ci.jenkins.io/job/Plugins/job/build-token-root-plugin/job/PR-21/ As a temporary trick, I think Main could define gitHubRepo based on CHANGE_FORK plus an appropriately processed part of JOB_NAME , yielding in this case jglick/build-token-root-plugin .
            jglick Jesse Glick added a comment -

            Working! Verified that I could run bom PCT against an unmerged PR of workflow-step-api.

            jglick Jesse Glick added a comment - Working! Verified that I could run bom PCT against an unmerged PR of workflow-step-api .
            jglick Jesse Glick added a comment -

            Beware JENKINS-63598.

            jglick Jesse Glick added a comment - Beware JENKINS-63598 .
            danielbeck Daniel Beck added a comment -

            That is because this code improperly assumes that ${project.artifactId}-${project.version} is going to be a valid tag. Maven makes no such guarantee.

            In fact, there's tagNameFormat in m-r-p configuration to override this even when users don't like typing on the interactive prompt.

            danielbeck Daniel Beck added a comment - That is because this code improperly assumes that ${project.artifactId}-${project.version } is going to be a valid tag. Maven makes no such guarantee. In fact, there's tagNameFormat in m-r-p configuration to override this even when users don't like typing on the interactive prompt.

            People

              jglick Jesse Glick
              jglick Jesse Glick
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: