[JIRA] (JENKINS-58716) Failure to check out specified tag for incremental version

19 views
Skip to first unread message

jglick@cloudbees.com (JIRA)

unread,
Jul 29, 2019, 3:59:04 PM7/29/19
to jenkinsc...@googlegroups.com
Jesse Glick created an issue
 
Jenkins / Bug JENKINS-58716
Failure to check out specified tag for incremental version
Issue Type: Bug Bug
Assignee: Unassigned
Components: plugin-compat-tester
Created: 2019-07-29 19:58
Priority: Major Major
Reporter: Jesse Glick

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.

Add Comment Add Comment
 
This message was sent by Atlassian Jira (v7.11.2#711002-sha1:fdc329d)

me@basilcrow.com (JIRA)

unread,
Aug 1, 2019, 6:50:03 PM8/1/19
to jenkinsc...@googlegroups.com

Rather, this code should be looking for a /project/scm/tag and honoring it if present

As explained in jenkinsci/plugin-compat-tester#181 (comment), this is necessary but not sufficient in order to solve the problem.

me@basilcrow.com (JIRA)

unread,
Aug 1, 2019, 7:55:02 PM8/1/19
to jenkinsc...@googlegroups.com

this is necessary but not sufficient in order to solve the problem

Here's a sketch of a possible solution. Introduce a new property for the GitHub organization, say scmOrganization. Then use it from <connection>, <developerConnection>, and <url>:

<properties>
    <scmOrganization>jenkinsci</scmOrganization>
</properties>

<scm>
    <connection>scm:git:git://github.com/${scmOrganization}/${project.artifactId}-plugin.git</connection>
    <developerConnection>scm:git:g...@github.com:${scmOrganization}/${project.artifactId}-plugin.git</developerConnection>
    <url>https://github.com/${scmOrganization}/${project.artifactId}-plugin</url>
    [...]
</scm>

(To implement this, the archetype would need to be updated and IncrementalifyMojo#update would need to evolve to be able to update an existing POM with this change.)

With this in place, the git-changelist-maven-extension in incrementals-tools could be updated to set this property for incrementals builds. In particular, we should be able to look up the Git remote in Main#afterSessionStart. From there, we can parse out the organization and set scmOrganization to the appropriate value for incrementals builds.

jglick@cloudbees.com (JIRA)

unread,
Aug 6, 2019, 1:57:02 PM8/6/19
to jenkinsc...@googlegroups.com
Jesse Glick updated an issue
 
Change By: Jesse Glick
Component/s: incrementals-tools

jglick@cloudbees.com (JIRA)

unread,
Aug 6, 2019, 1:57:03 PM8/6/19
to jenkinsc...@googlegroups.com

we should be able to look up the Git remote

I am not so sure. git remote -v in a forked PR build shows https://github.com/jenkinsci/XXX-plugin.git. This is because Jenkins first clones the origin repo at master, then merges in the PR head.

As to the property, I think you want to introduce a property for the entire repository location, so

<properties>
    <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
</properties>
<scm>
    <connection>scm:git:git://github.com/${gitHubRepo}.git</connection>
    <developerConnection>scm:git:g...@github.com:${gitHubRepo}.git</developerConnection>
    <url>https://github.com/${gitHubRepo}</url>
    <tag>${scmTag}</tag>
</scm>

since a developer’s fork may not have the same name as the origin repository. In particular, if the developer already has some unrelated “sources” repo or fork named foo-plugin, then jenkinsci/foo-plugin will be forked to jqhacker/foo-plugin-1 or similar.

You also would likely want to

  • Make incrementals:incrementalify do this transformation.
  • Adjust jenkinsci/archetypes to encourage new plugins to do this.

me@basilcrow.com (JIRA)

unread,
Aug 6, 2019, 5:35:02 PM8/6/19
to jenkinsc...@googlegroups.com

I am not so sure. git remote -v in a forked PR build shows https://github.com/jenkinsci/XXX-plugin.git. This is because Jenkins first clones the origin repo at master, then merges in the PR head.

Ah, that's unfortunate. However, the information we need is exposed by the CHANGE_FORK environment variable, although today it only consists of the username and not necessarily the repository name. However, JENKINS-58450 seeks to rectify that by setting CHANGE_FORK to $user/$repo when the repository name of the fork differs from the upstream repository name.

Given the above, I think we could check CHANGE_FORK from git-changelist-maven-extension. If it's just a username, we can update githubRepo to the username from CHANGE_FORK and the repository name from the original value of githubRepo. If it's a username and a repository (post-JENKINS-58450), we can update gitHubRepo to the value of CHANGE_FORK verbatim.

What do you think?

jglick@cloudbees.com (JIRA)

unread,
Aug 7, 2019, 3:14:02 PM8/7/19
to jenkinsc...@googlegroups.com

me@basilcrow.com (JIRA)

unread,
Aug 7, 2019, 3:38:02 PM8/7/19
to jenkinsc...@googlegroups.com

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?

jglick@cloudbees.com (JIRA)

unread,
Aug 7, 2019, 3:55:05 PM8/7/19
to jenkinsc...@googlegroups.com

me@basilcrow.com (JIRA)

unread,
Aug 7, 2019, 9:36:02 PM8/7/19
to jenkinsc...@googlegroups.com

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@cloudbees.com (JIRA)

unread,
Aug 13, 2019, 11:38:02 AM8/13/19
to jenkinsc...@googlegroups.com

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

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.

Reply all
Reply to author
Forward
0 new messages