Skip to content
Permalink
Browse files

[FIXED JENKINS-7411]

Environment variables are in fact not used at all during expansions.
This commit fixes that. As discussed in the ticket, and as done
elsewhere, the standard convention is to use previous build's
environment to expand polling.

Also see JENKINS-19042
  • Loading branch information...
kohsuke committed Aug 1, 2013
1 parent e7a66a5 commit c5b127f98ebc652eead68b39aa7f1c93094cbcc4
Showing with 54 additions and 23 deletions.
  1. +21 −23 src/main/java/hudson/plugins/git/GitSCM.java
  2. +33 −0 src/test/java/hudson/plugins/git/GitSCMTest.java
@@ -18,6 +18,7 @@
import hudson.scm.*;
import hudson.triggers.SCMTrigger;
import hudson.util.FormValidation;
import hudson.util.IOException2;
import hudson.util.IOUtils;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
@@ -520,13 +521,15 @@ public void setBuildChooser(BuildChooser buildChooser) {
*
* @return can be empty but never null.
*/
public List<RemoteConfig> getParamExpandedRepos(AbstractBuild<?, ?> build) {
public List<RemoteConfig> getParamExpandedRepos(AbstractBuild<?, ?> build) throws IOException, InterruptedException {
List<RemoteConfig> expandedRepos = new ArrayList<RemoteConfig>();

EnvVars env = build.getEnvironment();

for (RemoteConfig oldRepo : Util.fixNull(remoteRepositories)) {
expandedRepos.add(newRemoteConfig(getParameterString(oldRepo.getName(), build),
getParameterString(oldRepo.getURIs().get(0).toPrivateString(), build),
new RefSpec(getRefSpec(oldRepo, build))));
expandedRepos.add(newRemoteConfig(getParameterString(oldRepo.getName(), env),
getParameterString(oldRepo.getURIs().get(0).toPrivateString(), env),
new RefSpec(getRefSpec(oldRepo, env))));
}

return expandedRepos;
@@ -560,19 +563,14 @@ public String getGitTool() {
return gitTool;
}

private String getParameterString(String original, AbstractBuild<?, ?> build) {
ParametersAction parameters = build.getAction(ParametersAction.class);
if (parameters != null) {
original = parameters.substitute(build, original);
}

return original;
private String getParameterString(String original, EnvVars env) {
return env.expand(original);
}

private String getRefSpec(RemoteConfig repo, AbstractBuild<?, ?> build) {
private String getRefSpec(RemoteConfig repo, EnvVars env) {
String refSpec = repo.getFetchRefSpecs().get(0).toString();

return getParameterString(refSpec, build);
return getParameterString(refSpec, env);
}

/**
@@ -581,7 +579,7 @@ private String getRefSpec(RemoteConfig repo, AbstractBuild<?, ?> build) {
*
* Otherwise return null.
*/
private String getSingleBranch(AbstractBuild<?, ?> build) {
private String getSingleBranch(EnvVars env) {
// if we have multiple branches skip to advanced usecase
if (getBranches().size() != 1 || getRepositories().size() != 1) {
return null;
@@ -602,7 +600,7 @@ private String getSingleBranch(AbstractBuild<?, ?> build) {
}

// substitute build parameters if available
branch = getParameterString(branch, build);
branch = getParameterString(branch, env);

// Check for empty string - replace with "**" when seen.
if (branch.equals("")) {
@@ -627,7 +625,7 @@ protected PollingResult compareRemoteRevisionWith(AbstractProject<?, ?> project,
try {
return compareRemoteRevisionWithImpl( project, launcher, workspace, listener, baseline);
} catch (GitException e){
throw new IOException(e);
throw new IOException2(e);
}
}

@@ -648,7 +646,7 @@ private PollingResult compareRemoteRevisionWithImpl(AbstractProject<?, ?> projec
listener.getLogger().println("[poll] Last Built Revision: " + buildData.lastBuild.revision);
}

final String singleBranch = getSingleBranch(lastBuild);
final String singleBranch = getSingleBranch(lastBuild.getEnvironment());

// fast remote polling needs a single branch and an existing last build
if (this.remotePoll && singleBranch != null && buildData.lastBuild != null && buildData.lastBuild.getRevision() != null) {
@@ -933,7 +931,7 @@ private Revision determineRevisionToBuild(final AbstractBuild build,

final Revision parentLastBuiltRev = tempParentLastBuiltRev;

final String singleBranch = environment.expand( getSingleBranch(build) );
final String singleBranch = environment.expand( getSingleBranch(environment) );

final RevisionParameterAction rpa = build.getAction(RevisionParameterAction.class);
final BuildChooserContext context = new BuildChooserContextImpl(build.getProject(), build);
@@ -1129,8 +1127,8 @@ public boolean checkout(final AbstractBuild build, Launcher launcher,

final BuildChooserContext context = new BuildChooserContextImpl(build.getProject(),build);

final String remoteBranchName = getParameterString(mergeOptions.getRemoteBranchName(), build);
final String mergeTarget = getParameterString(mergeOptions.getMergeTarget(), build);
final String remoteBranchName = getParameterString(mergeOptions.getRemoteBranchName(), environment);
final String mergeTarget = getParameterString(mergeOptions.getMergeTarget(), environment);

Build returnedBuildData;
if (mergeOptions.doMerge() && !revToBuild.containsBranchName(remoteBranchName)) {
@@ -1342,7 +1340,7 @@ private void computeMergeChangeLog(GitClient git, Revision revToBuild, String re
public void buildEnvVars(AbstractBuild<?, ?> build, java.util.Map<String, String> env) {
super.buildEnvVars(build, env);
Revision rev = fixNull(getBuildData(build, false)).getLastBuiltRevision();
String singleBranch = getSingleBranch(build);
String singleBranch = getSingleBranch(new EnvVars(env));
if (singleBranch != null){
env.put(GIT_BRANCH, singleBranch);
} else if (rev != null) {
@@ -1767,10 +1765,10 @@ public String getLocalBranch() {
return Util.fixEmpty(localBranch);
}

public String getParamLocalBranch(AbstractBuild<?, ?> build) {
public String getParamLocalBranch(AbstractBuild<?, ?> build) throws IOException, InterruptedException {
String branch = getLocalBranch();
// substitute build parameters if available
return getParameterString(branch, build);
return getParameterString(branch, build.getEnvironment());
}

public String getRelativeTargetDir() {
@@ -5,11 +5,14 @@
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
import hudson.model.Cause;
import hudson.model.EnvironmentContributor;
import hudson.model.FreeStyleBuild;
import hudson.model.FreeStyleProject;
import hudson.model.Hudson;
import hudson.model.Node;
import hudson.model.Result;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.model.User;
import hudson.plugins.git.GitSCM.BuildChooserContextImpl;
import hudson.plugins.git.util.BuildChooserContext;
@@ -20,6 +23,7 @@
import hudson.remoting.Callable;
import hudson.remoting.Channel;
import hudson.remoting.VirtualChannel;
import hudson.scm.PollingResult;
import hudson.slaves.DumbSlave;
import hudson.slaves.EnvironmentVariablesNodeProperty;
import hudson.slaves.EnvironmentVariablesNodeProperty.Entry;
@@ -30,13 +34,15 @@
import com.google.common.base.Function;
import com.google.common.collect.Collections2;

import hudson.util.StreamTaskListener;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.PersonIdent;

import org.jenkinsci.plugins.gitclient.Git;
import org.jenkinsci.plugins.gitclient.GitClient;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.CaptureEnvironmentBuilder;
import org.jvnet.hudson.test.TestExtension;

import java.io.File;
import java.io.IOException;
@@ -915,4 +921,31 @@ public void testMergeFailedWithSlave() throws Exception {
assertFalse("scm polling should not detect any more changes after build", project.poll(listener).hasChanges());
}

public void testEnvironmentVariableExpansion() throws Exception {
FreeStyleProject project = createFreeStyleProject();
project.setScm(new GitSCM("${CAT}"+testRepo.gitDir.getPath()));

// create initial commit and then run the build against it:
commit("a.txt", johnDoe, "Initial Commit");

build(project, Result.SUCCESS, "a.txt");

PollingResult r = project.poll(StreamTaskListener.fromStdout());
assertFalse(r.hasChanges());

commit("b.txt", johnDoe, "Another commit");

r = project.poll(StreamTaskListener.fromStdout());
assertTrue(r.hasChanges());

build(project, Result.SUCCESS, "b.txt");
}

@TestExtension("testEnvironmentVariableExpansion")
public static class SupplySomeEnvVars extends EnvironmentContributor {
@Override
public void buildEnvironmentFor(Run r, EnvVars envs, TaskListener listener) throws IOException, InterruptedException {
envs.put("CAT","");
}
}
}

0 comments on commit c5b127f

Please sign in to comment.
You can’t perform that action at this time.