Skip to content

Commit

Permalink
[FIXED JENKINS-7411]
Browse files Browse the repository at this point in the history 
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
kohsuke committed Aug 1, 2013
1 parent e7a66a5 commit c5b127f
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 23 deletions.
44 changes: 21 additions & 23 deletions src/main/java/hudson/plugins/git/GitSCM.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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;
Expand All @@ -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("")) {
Expand All @@ -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);
}
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down
33 changes: 33 additions & 0 deletions src/test/java/hudson/plugins/git/GitSCMTest.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -915,4 +921,31 @@ false, new DefaultBuildChooser(), null, null, true, null, null,
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.