From a7c26ceea2a4d739ac561cc3d149c40ad39558cf Mon Sep 17 00:00:00 2001 From: Sergio-Mira Date: Thu, 10 Dec 2020 14:22:25 +0000 Subject: [PATCH 1/8] [JENKINS-56063] Fix refSpec with expanded variables on first clone --- .../git/extensions/impl/CloneOption.java | 32 ++++++-- .../impl/CloneOptionHonorRefSpecTest.java | 76 +++++++++++++++++++ 2 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java diff --git a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java index 2f5d1abf0e..7fc60d8c8d 100644 --- a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java +++ b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java @@ -14,6 +14,7 @@ import hudson.plugins.git.util.GitUtils; import hudson.slaves.NodeProperty; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import org.eclipse.jgit.transport.RefSpec; @@ -26,6 +27,7 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.CheckForNull; /** * @author Kohsuke Kawaguchi @@ -139,6 +141,14 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas listener.getLogger().println("Avoid fetching tags"); cmd.tags(false); } + + Node node = GitUtils.workspaceToNode(git.getWorkTree()); + EnvVars env = build.getEnvironment(listener); + Computer comp = node.toComputer(); + if (comp != null) { + env.putAll(comp.getEnvironment()); + } + if (honorRefspec) { listener.getLogger().println("Honoring refspec on initial clone"); // Read refspec configuration from the first configured repository. @@ -149,23 +159,29 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas // Git plugin does not support multiple independent repositories // in a single job definition. RemoteConfig rc = scm.getRepositories().get(0); - List refspecs = rc.getFetchRefSpecs(); - cmd.refspecs(refspecs); + cmd.refspecs(getRefSpecs(rc, env)); } cmd.timeout(timeout); - Node node = GitUtils.workspaceToNode(git.getWorkTree()); - EnvVars env = build.getEnvironment(listener); - Computer comp = node.toComputer(); - if (comp != null) { - env.putAll(comp.getEnvironment()); - } for (NodeProperty nodeProperty: node.getNodeProperties()) { nodeProperty.buildEnvVars(env, listener); } cmd.reference(env.expand(reference)); } + @NonNull + private static String getParameterString(@CheckForNull String original, @NonNull EnvVars env) { + return env.expand(original); + } + + private static List getRefSpecs(RemoteConfig repo, EnvVars env) { + List refSpecs = new ArrayList<>(); + for (RefSpec refSpec : repo.getFetchRefSpecs()) { + refSpecs.add(new RefSpec(getParameterString(refSpec.toString(), env))); + } + return refSpecs; + } + /** * {@inheritDoc} */ diff --git a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java new file mode 100644 index 0000000000..d3b9e33492 --- /dev/null +++ b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java @@ -0,0 +1,76 @@ +package hudson.plugins.git.extensions.impl; + +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; +import hudson.model.Result; +import org.jvnet.hudson.test.Issue; +import hudson.plugins.git.AbstractGitTestCase; +import hudson.plugins.git.BranchSpec; +import hudson.plugins.git.GitSCM; +import hudson.plugins.git.UserRemoteConfig; +import org.eclipse.jgit.transport.RefSpec; +import org.jenkinsci.plugins.gitclient.CloneCommand; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +@RunWith(Parameterized.class) +public class CloneOptionHonorRefSpecTest extends AbstractGitTestCase { + + private String refSpecVar; + + public CloneOptionHonorRefSpecTest(String useRefSpecVariable) { + this.refSpecVar = useRefSpecVariable; + } + + @Parameterized.Parameters(name = "{0}") + public static Collection permuteRefSpecVariable() { + List values = new ArrayList<>(); + String[] allowed = {"${BUILD_NUMBER}","${BUILD_ID}", + "${GIT_COMMIT}"}; + for (String refSpecValue : allowed) { + Object[] combination = {refSpecValue}; + values.add(combination); + } + return values; + } + + /** + * This test confirms behavior of refspecs on initial clone with expanded variables. + * @throws Exception on error + */ + @Test + @Issue("JENKINS-56063") + public void testRefSpecWithExpandedVariables() throws Exception { + List repos = new ArrayList<>(); + repos.add(new UserRemoteConfig(testRepo.gitDir.getAbsolutePath(), "origin", "+refs/heads/master:refs/remotes/origin/master" + refSpecVar, null)); + + /* Set CloneOption to honor refspec on initial clone with expanded var */ + FreeStyleProject projectWithMasterExpanded = setupProject(repos, Collections.singletonList(new BranchSpec("master" + refSpecVar)), null, false, null); + CloneOption cloneOptionMasterExpanded = new CloneOption(false, null, null); + cloneOptionMasterExpanded.setHonorRefspec(true); + ((GitSCM)projectWithMasterExpanded.getScm()).getExtensions().add(cloneOptionMasterExpanded); + + /* Set CloneOption to honor refspec on initial clone without expanded var, should fail */ + FreeStyleProject projectWithMaster = setupProject(repos, Collections.singletonList(new BranchSpec("master")), null, false, null); + CloneOption cloneOptionMaster = new CloneOption(false, null, null); + cloneOptionMaster.setHonorRefspec(true); + ((GitSCM)projectWithMaster.getScm()).getExtensions().add(cloneOptionMaster); + + // create initial commit + final String commitFile1 = "commitFile1"; + commit(commitFile1, johnDoe, "Commit in master"); + // create branch and make initial commit + git.checkout().ref("master").branch("foo").execute(); + commit(commitFile1, johnDoe, "Commit in foo"); + + build(projectWithMaster, Result.FAILURE); + build(projectWithMasterExpanded, Result.SUCCESS, commitFile1); + } +} From 238ed35505296333590ad2bef950c92180aa7fbf Mon Sep 17 00:00:00 2001 From: Sergio-Mira Date: Thu, 10 Dec 2020 15:32:00 +0000 Subject: [PATCH 2/8] Change tested envs --- .../git/extensions/impl/CloneOptionHonorRefSpecTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java index d3b9e33492..07feb6891b 100644 --- a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java +++ b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java @@ -32,8 +32,7 @@ public CloneOptionHonorRefSpecTest(String useRefSpecVariable) { @Parameterized.Parameters(name = "{0}") public static Collection permuteRefSpecVariable() { List values = new ArrayList<>(); - String[] allowed = {"${BUILD_NUMBER}","${BUILD_ID}", - "${GIT_COMMIT}"}; + String[] allowed = {"${GIT_REVISION}", "${GIT_COMMIT}"}; for (String refSpecValue : allowed) { Object[] combination = {refSpecValue}; values.add(combination); From 37e67e5fe669d4cfc1d168ab1eaafc96fd302eca Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Thu, 10 Dec 2020 19:23:15 -0700 Subject: [PATCH 3/8] Revise test to check system vars and Jenkins vars Checks both system environment variables (like USER on Unix variants and USERNAME on Windows) and Jenkins provided environment variables (like JOB_NAME). Asserts that the variable name does not appear in the build log. Uses a branch based on the value of the variable and asserts that the job is successful. A successful job means that the branch name based on the value of the variable was found and used in the job. Does not assert that the specific commit from the branch is the one that is used in the job. That could be added. --- .../impl/CloneOptionHonorRefSpecTest.java | 107 +++++++++++++----- 1 file changed, 76 insertions(+), 31 deletions(-) diff --git a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java index 07feb6891b..d0f1b9434a 100644 --- a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java +++ b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java @@ -3,73 +3,118 @@ import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; import hudson.model.Result; -import org.jvnet.hudson.test.Issue; import hudson.plugins.git.AbstractGitTestCase; import hudson.plugins.git.BranchSpec; import hudson.plugins.git.GitSCM; import hudson.plugins.git.UserRemoteConfig; -import org.eclipse.jgit.transport.RefSpec; -import org.jenkinsci.plugins.gitclient.CloneCommand; -import org.junit.Before; + import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.jvnet.hudson.test.Issue; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Random; +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; @RunWith(Parameterized.class) public class CloneOptionHonorRefSpecTest extends AbstractGitTestCase { - private String refSpecVar; + private final String refSpecName; + private final String refSpecExpectedValue; + private final Boolean honorRefSpec; - public CloneOptionHonorRefSpecTest(String useRefSpecVariable) { - this.refSpecVar = useRefSpecVariable; + private static final Random random = new Random(); + + public CloneOptionHonorRefSpecTest(String refSpecName, String refSpecExpectedValue, Boolean honorRefSpec) { + this.refSpecName = refSpecName; + this.refSpecExpectedValue = refSpecExpectedValue; + this.honorRefSpec = honorRefSpec; } - @Parameterized.Parameters(name = "{0}") + private static String getExpectedValue(String reference) { + if (reference.startsWith("JOB_")) { + return "test0"; + } + if (reference.contains("USER")) { + return System.getProperty("user.name", "java-user.name-property-not-found"); + } + return "not-master"; // fake value for other variables + } + + @Parameterized.Parameters(name = "{0}-{1}-{2}") public static Collection permuteRefSpecVariable() { List values = new ArrayList<>(); - String[] allowed = {"${GIT_REVISION}", "${GIT_COMMIT}"}; - for (String refSpecValue : allowed) { - Object[] combination = {refSpecValue}; + /* Should behave same with honor refspec enabled or disabled */ + boolean honorRefSpec = random.nextBoolean(); + + /* Variables set by Jenkins */ + String[] allowed = {"JOB_BASE_NAME", "JOB_NAME", "JENKINS_USERNAME"}; + for (String refSpecName : allowed) { + String refSpecExpectedValue = getExpectedValue(refSpecName); + Object[] combination = {refSpecName, refSpecExpectedValue, honorRefSpec}; values.add(combination); + honorRefSpec = !honorRefSpec; } + + /* Variable set by the operating system */ + String refSpecName = isWindows() ? "USERNAME" : "USER"; + String refSpecExpectedValue = getExpectedValue(refSpecName); + Object[] combination = {refSpecName, refSpecExpectedValue, honorRefSpec}; + values.add(combination); + return values; } /** - * This test confirms behavior of refspecs on initial clone with expanded variables. + * This test confirms behavior of refspecs on initial clone with expanded + * variables. When an environment variable reference is embedded in the + * refspec, it should be expanded in all cases. + * * @throws Exception on error */ @Test @Issue("JENKINS-56063") public void testRefSpecWithExpandedVariables() throws Exception { - List repos = new ArrayList<>(); - repos.add(new UserRemoteConfig(testRepo.gitDir.getAbsolutePath(), "origin", "+refs/heads/master:refs/remotes/origin/master" + refSpecVar, null)); - - /* Set CloneOption to honor refspec on initial clone with expanded var */ - FreeStyleProject projectWithMasterExpanded = setupProject(repos, Collections.singletonList(new BranchSpec("master" + refSpecVar)), null, false, null); - CloneOption cloneOptionMasterExpanded = new CloneOption(false, null, null); - cloneOptionMasterExpanded.setHonorRefspec(true); - ((GitSCM)projectWithMasterExpanded.getScm()).getExtensions().add(cloneOptionMasterExpanded); - - /* Set CloneOption to honor refspec on initial clone without expanded var, should fail */ - FreeStyleProject projectWithMaster = setupProject(repos, Collections.singletonList(new BranchSpec("master")), null, false, null); - CloneOption cloneOptionMaster = new CloneOption(false, null, null); - cloneOptionMaster.setHonorRefspec(true); - ((GitSCM)projectWithMaster.getScm()).getExtensions().add(cloneOptionMaster); // create initial commit final String commitFile1 = "commitFile1"; - commit(commitFile1, johnDoe, "Commit in master"); + commit(commitFile1, johnDoe, "Commit in master branch"); + // create branch and make initial commit - git.checkout().ref("master").branch("foo").execute(); - commit(commitFile1, johnDoe, "Commit in foo"); + git.checkout().ref("master").branch(refSpecExpectedValue).execute(); + commit(commitFile1, johnDoe, "Commit in '" + refSpecExpectedValue + "' branch"); + + // Use the variable reference in the refspec + // Should be expanded by the clone option whether or not honor refspec is enabled + List repos = new ArrayList<>(); + repos.add(new UserRemoteConfig( + testRepo.gitDir.getAbsolutePath(), + "origin", + "+refs/heads/${" + refSpecName + "}:refs/remotes/origin/${" + refSpecName + "}", null)); + + /* Use the variable or its value as the branch name. + * Same result expected in either case. + */ + String branchName = random.nextBoolean() ? "${" + refSpecName + "}" : refSpecExpectedValue; + FreeStyleProject project = setupProject(repos, Collections.singletonList(new BranchSpec(branchName)), null, false, null); + + /* Same result expected whether refspec honored or not */ + CloneOption cloneOption = new CloneOption(false, null, null); + cloneOption.setHonorRefspec(honorRefSpec); + ((GitSCM) project.getScm()).getExtensions().add(cloneOption); + + FreeStyleBuild b = build(project, Result.SUCCESS, commitFile1); + /* Check that unexpanded refspec name is not in the log */ + List buildLog = b.getLog(50); + assertThat(buildLog, not(hasItem(containsString("${" + refSpecName + "}")))); + } - build(projectWithMaster, Result.FAILURE); - build(projectWithMasterExpanded, Result.SUCCESS, commitFile1); + private static boolean isWindows() { + return false; } } From 3664fda5dfefcb0b8498dc8cb082cd776b9b7bf2 Mon Sep 17 00:00:00 2001 From: Sergio-Mira Date: Fri, 11 Dec 2020 10:31:32 +0000 Subject: [PATCH 4/8] Add parametrised test and fix job name getter --- .../impl/CloneOptionHonorRefSpecTest.java | 58 ++++++++++++------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java index d0f1b9434a..b310d0b425 100644 --- a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java +++ b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java @@ -1,13 +1,10 @@ package hudson.plugins.git.extensions.impl; -import hudson.model.FreeStyleBuild; -import hudson.model.FreeStyleProject; -import hudson.model.Result; -import hudson.plugins.git.AbstractGitTestCase; -import hudson.plugins.git.BranchSpec; -import hudson.plugins.git.GitSCM; -import hudson.plugins.git.UserRemoteConfig; +import hudson.model.*; +import hudson.plugins.git.*; +import hudson.plugins.git.extensions.GitSCMExtension; +import org.jenkinsci.plugins.gitclient.JGitTool; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -25,46 +22,50 @@ public class CloneOptionHonorRefSpecTest extends AbstractGitTestCase { private final String refSpecName; - private final String refSpecExpectedValue; private final Boolean honorRefSpec; private static final Random random = new Random(); - public CloneOptionHonorRefSpecTest(String refSpecName, String refSpecExpectedValue, Boolean honorRefSpec) { + public CloneOptionHonorRefSpecTest(String refSpecName, Boolean honorRefSpec) { this.refSpecName = refSpecName; - this.refSpecExpectedValue = refSpecExpectedValue; this.honorRefSpec = honorRefSpec; } - private static String getExpectedValue(String reference) { - if (reference.startsWith("JOB_")) { - return "test0"; + private static String getExpectedValue(String reference, FreeStyleProject project) { + if (reference.equals("JOB_BASE_NAME") || reference.equals("JOB_NAME")) { + return project.getName(); } - if (reference.contains("USER")) { + if (reference.equals("USER") || reference.equals("USERNAME")) { return System.getProperty("user.name", "java-user.name-property-not-found"); } + if (reference.equals("USER_SELECTED_BRANCH_NAME")) { + return "user_branch"; + } return "not-master"; // fake value for other variables } - @Parameterized.Parameters(name = "{0}-{1}-{2}") + @Parameterized.Parameters(name = "{0}-{1}") public static Collection permuteRefSpecVariable() { List values = new ArrayList<>(); /* Should behave same with honor refspec enabled or disabled */ boolean honorRefSpec = random.nextBoolean(); /* Variables set by Jenkins */ - String[] allowed = {"JOB_BASE_NAME", "JOB_NAME", "JENKINS_USERNAME"}; + String[] allowed = {"JOB_BASE_NAME", "JOB_NAME"}; for (String refSpecName : allowed) { - String refSpecExpectedValue = getExpectedValue(refSpecName); - Object[] combination = {refSpecName, refSpecExpectedValue, honorRefSpec}; + Object[] combination = {refSpecName, honorRefSpec}; values.add(combination); honorRefSpec = !honorRefSpec; } /* Variable set by the operating system */ String refSpecName = isWindows() ? "USERNAME" : "USER"; - String refSpecExpectedValue = getExpectedValue(refSpecName); - Object[] combination = {refSpecName, refSpecExpectedValue, honorRefSpec}; + Object[] combination = {refSpecName, !honorRefSpec}; + values.add(combination); + + /* Parametrised build */ + refSpecName = "USER_SELECTED_BRANCH_NAME"; + combination = new Object[]{refSpecName, !honorRefSpec}; values.add(combination); return values; @@ -81,6 +82,14 @@ public static Collection permuteRefSpecVariable() { @Issue("JENKINS-56063") public void testRefSpecWithExpandedVariables() throws Exception { + FreeStyleProject project = createFreeStyleProject(); + project.addProperty(new ParametersDefinitionProperty( + new StringParameterDefinition("USER_SELECTED_BRANCH_NAME", "user_branch") + )); + project.save(); + + String refSpecExpectedValue = getExpectedValue(refSpecName, project); + // create initial commit final String commitFile1 = "commitFile1"; commit(commitFile1, johnDoe, "Commit in master branch"); @@ -101,7 +110,14 @@ public void testRefSpecWithExpandedVariables() throws Exception { * Same result expected in either case. */ String branchName = random.nextBoolean() ? "${" + refSpecName + "}" : refSpecExpectedValue; - FreeStyleProject project = setupProject(repos, Collections.singletonList(new BranchSpec(branchName)), null, false, null); + GitSCM scm = new GitSCM( + repos, + Collections.singletonList(new BranchSpec(branchName)), + false, Collections.emptyList(), + null, JGitTool.MAGIC_EXENAME, + Collections.emptyList()); + project.setScm(scm); + project.save(); /* Same result expected whether refspec honored or not */ CloneOption cloneOption = new CloneOption(false, null, null); From f38c41f9e22f3f3b0f550db84ab37d90b8ffea54 Mon Sep 17 00:00:00 2001 From: Sergio-Mira Date: Fri, 11 Dec 2020 17:05:37 +0000 Subject: [PATCH 5/8] Remove bad annotation, fix import and randomize JGit usage in test --- .../plugins/git/extensions/impl/CloneOption.java | 1 - .../impl/CloneOptionHonorRefSpecTest.java | 16 +++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java index 7fc60d8c8d..83a00fcde0 100644 --- a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java +++ b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java @@ -169,7 +169,6 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas cmd.reference(env.expand(reference)); } - @NonNull private static String getParameterString(@CheckForNull String original, @NonNull EnvVars env) { return env.expand(original); } diff --git a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java index b310d0b425..0691532330 100644 --- a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java +++ b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java @@ -1,8 +1,14 @@ package hudson.plugins.git.extensions.impl; -import hudson.model.*; -import hudson.plugins.git.*; - +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; +import hudson.model.ParametersDefinitionProperty; +import hudson.model.Result; +import hudson.model.StringParameterDefinition; +import hudson.plugins.git.AbstractGitTestCase; +import hudson.plugins.git.BranchSpec; +import hudson.plugins.git.GitSCM; +import hudson.plugins.git.UserRemoteConfig; import hudson.plugins.git.extensions.GitSCMExtension; import org.jenkinsci.plugins.gitclient.JGitTool; import org.junit.Test; @@ -113,8 +119,8 @@ public void testRefSpecWithExpandedVariables() throws Exception { GitSCM scm = new GitSCM( repos, Collections.singletonList(new BranchSpec(branchName)), - false, Collections.emptyList(), - null, JGitTool.MAGIC_EXENAME, + false, Collections.emptyList(), + null, random.nextBoolean() ? JGitTool.MAGIC_EXENAME : null, Collections.emptyList()); project.setScm(scm); project.save(); From 1353381ec5c8cc1597bcc11c1a00eb31340a0284 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Fri, 11 Dec 2020 12:07:17 -0700 Subject: [PATCH 6/8] Implement isWindows Was returning false even when running on Windows. --- .../git/extensions/impl/CloneOptionHonorRefSpecTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java index 0691532330..11cb97d2dc 100644 --- a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java +++ b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java @@ -16,6 +16,7 @@ import org.junit.runners.Parameterized; import org.jvnet.hudson.test.Issue; +import java.io.File; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -137,6 +138,6 @@ public void testRefSpecWithExpandedVariables() throws Exception { } private static boolean isWindows() { - return false; + return File.pathSeparatorChar == ';'; } } From d826281422d6e46fe8b9ce61dafca523e7b93882 Mon Sep 17 00:00:00 2001 From: Sergio-Mira Date: Sun, 13 Dec 2020 12:01:35 +0000 Subject: [PATCH 7/8] Obtain env var value from a job run beforehand --- .../git/extensions/impl/CloneOption.java | 4 + .../impl/CloneOptionHonorRefSpecTest.java | 73 +++++++++++-------- 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java index 83a00fcde0..1cf2cb491f 100644 --- a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java +++ b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java @@ -160,6 +160,10 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas // in a single job definition. RemoteConfig rc = scm.getRepositories().get(0); cmd.refspecs(getRefSpecs(rc, env)); + + // TODO: Debug output + listener.getLogger().println(String.join(";", env.keySet())); + listener.getLogger().println(String.join(";", env.values())); } cmd.timeout(timeout); diff --git a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java index 11cb97d2dc..9d5d593c43 100644 --- a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java +++ b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java @@ -1,5 +1,6 @@ package hudson.plugins.git.extensions.impl; +import hudson.Functions; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; import hudson.model.ParametersDefinitionProperty; @@ -9,14 +10,16 @@ import hudson.plugins.git.BranchSpec; import hudson.plugins.git.GitSCM; import hudson.plugins.git.UserRemoteConfig; -import hudson.plugins.git.extensions.GitSCMExtension; +import hudson.tasks.BatchFile; +import hudson.tasks.Builder; +import hudson.tasks.Shell; import org.jenkinsci.plugins.gitclient.JGitTool; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.jvnet.hudson.test.Issue; -import java.io.File; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -32,25 +35,14 @@ public class CloneOptionHonorRefSpecTest extends AbstractGitTestCase { private final Boolean honorRefSpec; private static final Random random = new Random(); + private FreeStyleProject project; + private String refSpecExpectedValue; public CloneOptionHonorRefSpecTest(String refSpecName, Boolean honorRefSpec) { this.refSpecName = refSpecName; this.honorRefSpec = honorRefSpec; } - private static String getExpectedValue(String reference, FreeStyleProject project) { - if (reference.equals("JOB_BASE_NAME") || reference.equals("JOB_NAME")) { - return project.getName(); - } - if (reference.equals("USER") || reference.equals("USERNAME")) { - return System.getProperty("user.name", "java-user.name-property-not-found"); - } - if (reference.equals("USER_SELECTED_BRANCH_NAME")) { - return "user_branch"; - } - return "not-master"; // fake value for other variables - } - @Parameterized.Parameters(name = "{0}-{1}") public static Collection permuteRefSpecVariable() { List values = new ArrayList<>(); @@ -66,7 +58,7 @@ public static Collection permuteRefSpecVariable() { } /* Variable set by the operating system */ - String refSpecName = isWindows() ? "USERNAME" : "USER"; + String refSpecName = Functions.isWindows() ? "USERNAME" : "USER"; Object[] combination = {refSpecName, !honorRefSpec}; values.add(combination); @@ -78,6 +70,39 @@ public static Collection permuteRefSpecVariable() { return values; } + private static Builder createEnvEchoBuilder(String envVarName) { + if (Functions.isWindows()) { + return new BatchFile(String.format("echo %s=%%%s%%", envVarName, envVarName)); + } + return new Shell(String.format("echo \"%s=${%s}\"", envVarName, envVarName)); + } + + @Before + public void setUp() throws Exception { + super.setUp(); + + // Setup job beforehand to get expected value of the environment variable + project = createFreeStyleProject(); + project.addProperty(new ParametersDefinitionProperty( + new StringParameterDefinition("USER_SELECTED_BRANCH_NAME", "user_branch") + )); + project.getBuildersList().add(createEnvEchoBuilder(refSpecName)); + + final FreeStyleBuild b = project.scheduleBuild2(0).get(); + rule.assertBuildStatus(Result.SUCCESS, b); + + List logs = b.getLog(50); + for (String line : logs) { + if (line.startsWith(refSpecName + '=')) { + refSpecExpectedValue = line.split("=")[1]; + } + } + + if (refSpecExpectedValue == null) { + throw new Exception("Could not obtain ENV_VAR expected value"); + } + } + /** * This test confirms behavior of refspecs on initial clone with expanded * variables. When an environment variable reference is embedded in the @@ -88,15 +113,6 @@ public static Collection permuteRefSpecVariable() { @Test @Issue("JENKINS-56063") public void testRefSpecWithExpandedVariables() throws Exception { - - FreeStyleProject project = createFreeStyleProject(); - project.addProperty(new ParametersDefinitionProperty( - new StringParameterDefinition("USER_SELECTED_BRANCH_NAME", "user_branch") - )); - project.save(); - - String refSpecExpectedValue = getExpectedValue(refSpecName, project); - // create initial commit final String commitFile1 = "commitFile1"; commit(commitFile1, johnDoe, "Commit in master branch"); @@ -122,7 +138,7 @@ public void testRefSpecWithExpandedVariables() throws Exception { Collections.singletonList(new BranchSpec(branchName)), false, Collections.emptyList(), null, random.nextBoolean() ? JGitTool.MAGIC_EXENAME : null, - Collections.emptyList()); + Collections.emptyList()); project.setScm(scm); project.save(); @@ -132,12 +148,9 @@ public void testRefSpecWithExpandedVariables() throws Exception { ((GitSCM) project.getScm()).getExtensions().add(cloneOption); FreeStyleBuild b = build(project, Result.SUCCESS, commitFile1); + /* Check that unexpanded refspec name is not in the log */ List buildLog = b.getLog(50); assertThat(buildLog, not(hasItem(containsString("${" + refSpecName + "}")))); } - - private static boolean isWindows() { - return File.pathSeparatorChar == ';'; - } } From 93b85a09b99b95905bca922bc84aa8e104edc5ec Mon Sep 17 00:00:00 2001 From: Sergio-Mira Date: Thu, 17 Dec 2020 08:53:00 +0000 Subject: [PATCH 8/8] Only take into account the build environment as other parts of the checkout --- src/main/java/hudson/plugins/git/GitSCM.java | 4 +- .../git/extensions/impl/CloneOption.java | 18 ++++----- .../impl/CloneOptionHonorRefSpecTest.java | 37 ++++++++----------- 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java index 39c3f67c9e..982d5de11b 100644 --- a/src/main/java/hudson/plugins/git/GitSCM.java +++ b/src/main/java/hudson/plugins/git/GitSCM.java @@ -512,12 +512,12 @@ public List getParamExpandedRepos(Run build, TaskListener li } /** - * Expand Parameters in the supplied remote repository with the parameter values provided in the given environment variables } + * Expand Parameters in the supplied remote repository with the parameter values provided in the given environment variables * @param env Environment variables with parameter values * @param remoteRepository Remote repository with parameters * @return remote repository with expanded parameters */ - public RemoteConfig getParamExpandedRepo(EnvVars env, RemoteConfig remoteRepository){ + public RemoteConfig getParamExpandedRepo(EnvVars env, RemoteConfig remoteRepository) { List refSpecs = getRefSpecs(remoteRepository, env); return newRemoteConfig( getParameterString(remoteRepository.getName(), env), diff --git a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java index 1cf2cb491f..887d14911c 100644 --- a/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java +++ b/src/main/java/hudson/plugins/git/extensions/impl/CloneOption.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Objects; import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.transport.RemoteConfig; @@ -143,11 +144,6 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas } Node node = GitUtils.workspaceToNode(git.getWorkTree()); - EnvVars env = build.getEnvironment(listener); - Computer comp = node.toComputer(); - if (comp != null) { - env.putAll(comp.getEnvironment()); - } if (honorRefspec) { listener.getLogger().println("Honoring refspec on initial clone"); @@ -158,15 +154,17 @@ public void decorateCloneCommand(GitSCM scm, Run build, GitClient git, Tas // configuration is treated as authoritative. // Git plugin does not support multiple independent repositories // in a single job definition. + EnvVars buildEnv = build.getEnvironment(listener); RemoteConfig rc = scm.getRepositories().get(0); - cmd.refspecs(getRefSpecs(rc, env)); - - // TODO: Debug output - listener.getLogger().println(String.join(";", env.keySet())); - listener.getLogger().println(String.join(";", env.values())); + cmd.refspecs(getRefSpecs(rc, buildEnv)); } cmd.timeout(timeout); + EnvVars env = build.getEnvironment(listener); + Computer comp = node.toComputer(); + if (comp != null) { + env.putAll(comp.getEnvironment()); + } for (NodeProperty nodeProperty: node.getNodeProperties()) { nodeProperty.buildEnvVars(env, listener); } diff --git a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java index 9d5d593c43..74661e4bdc 100644 --- a/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java +++ b/src/test/java/hudson/plugins/git/extensions/impl/CloneOptionHonorRefSpecTest.java @@ -46,27 +46,21 @@ public CloneOptionHonorRefSpecTest(String refSpecName, Boolean honorRefSpec) { @Parameterized.Parameters(name = "{0}-{1}") public static Collection permuteRefSpecVariable() { List values = new ArrayList<>(); - /* Should behave same with honor refspec enabled or disabled */ + // Should behave same with honor refspec enabled or disabled boolean honorRefSpec = random.nextBoolean(); - /* Variables set by Jenkins */ - String[] allowed = {"JOB_BASE_NAME", "JOB_NAME"}; - for (String refSpecName : allowed) { - Object[] combination = {refSpecName, honorRefSpec}; + String[] keys = { + "JOB_NAME", // Variable set by Jenkins + (Functions.isWindows() ? "USERNAME" : "USER"), // Variable set by the operating system + "USER_SELECTED_BRANCH_NAME" // Parametrised build param + }; + + for (String refSpecName : keys) { + Object[] combination = {refSpecName, true}; values.add(combination); honorRefSpec = !honorRefSpec; } - /* Variable set by the operating system */ - String refSpecName = Functions.isWindows() ? "USERNAME" : "USER"; - Object[] combination = {refSpecName, !honorRefSpec}; - values.add(combination); - - /* Parametrised build */ - refSpecName = "USER_SELECTED_BRANCH_NAME"; - combination = new Object[]{refSpecName, !honorRefSpec}; - values.add(combination); - return values; } @@ -99,7 +93,7 @@ public void setUp() throws Exception { } if (refSpecExpectedValue == null) { - throw new Exception("Could not obtain ENV_VAR expected value"); + throw new Exception("Could not obtain env var expected value"); } } @@ -113,11 +107,11 @@ public void setUp() throws Exception { @Test @Issue("JENKINS-56063") public void testRefSpecWithExpandedVariables() throws Exception { - // create initial commit + // Create initial commit final String commitFile1 = "commitFile1"; commit(commitFile1, johnDoe, "Commit in master branch"); - // create branch and make initial commit + // Create branch and make initial commit git.checkout().ref("master").branch(refSpecExpectedValue).execute(); commit(commitFile1, johnDoe, "Commit in '" + refSpecExpectedValue + "' branch"); @@ -140,16 +134,15 @@ public void testRefSpecWithExpandedVariables() throws Exception { null, random.nextBoolean() ? JGitTool.MAGIC_EXENAME : null, Collections.emptyList()); project.setScm(scm); - project.save(); - /* Same result expected whether refspec honored or not */ + // Same result expected whether refspec honored or not CloneOption cloneOption = new CloneOption(false, null, null); cloneOption.setHonorRefspec(honorRefSpec); - ((GitSCM) project.getScm()).getExtensions().add(cloneOption); + scm.getExtensions().add(cloneOption); FreeStyleBuild b = build(project, Result.SUCCESS, commitFile1); - /* Check that unexpanded refspec name is not in the log */ + // Check that unexpanded refspec name is not in the log List buildLog = b.getLog(50); assertThat(buildLog, not(hasItem(containsString("${" + refSpecName + "}")))); }