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

Failure to check out specified tag for incremental version

    Details

    • Similar Issues:
    • Epic Link:

      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

            Hide
            jglick Jesse Glick added a comment -

            Yes, modulo my comment in JENKINS-58450.

            Show
            jglick Jesse Glick added a comment - Yes, modulo my comment in JENKINS-58450 .
            Hide
            basil Basil Crow added a comment -

            Yes, modulo my comment in JENKINS-58450.

            Your comments there sound reasonable to me, but since I'm neither implementing nor reviewing those changes, I don't want to be blocked on them. Are you OK with moving forward with the existing CHANGE_FORK environment variable for this bug, and then using the new CHANGE_FORK_FULL variable in a separate change if and when it becomes available?

            Show
            basil Basil Crow added a comment - Yes, modulo my comment in JENKINS-58450 . Your comments there sound reasonable to me, but since I'm neither implementing nor reviewing those changes, I don't want to be blocked on them. Are you OK with moving forward with the existing CHANGE_FORK environment variable for this bug, and then using the new CHANGE_FORK_FULL variable in a separate change if and when it becomes available?
            Hide
            jglick Jesse Glick added a comment -

            I suppose that makes sense, yes.

            Show
            jglick Jesse Glick added a comment - I suppose that makes sense, yes.
            Hide
            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?

            Show
            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?
            Hide
            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.

            Show
            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 .

              People

              • Assignee:
                Unassigned
                Reporter:
                jglick Jesse Glick
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: