Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #13 from synopsys-arc-oss/JENKINS_20560_FIX
[FIXED JENKINS-20560] - Custom-tools should not override global paths in...
  • Loading branch information
oleg-nenashev committed Nov 25, 2013
2 parents 5d9a150 + 84cac80 commit 6c81ebb
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 16 deletions.
Expand Up @@ -214,19 +214,27 @@ public Proc launch(ProcStarter starter) throws IOException {
vars = new EnvVars();
}

// HACK: Avoids issue with invalid separators in EnvVars::override in case of different master/slave
String overridenPaths = vars.get("PATH");
overridenPaths += paths.toListString();
vars.override("PATH", overridenPaths);
// Inject paths
final String injectedPaths = paths.toListString();
if (injectedPaths != null) {
vars.override("PATH+", injectedPaths);
}

// Inject additional variables
vars.putAll(homes);
vars.putAll(versions);

// Inject additional variables
for (EnvVariablesInjector injector : additionalVarInjectors) {
injector.Inject(vars);
}

// Override paths to prevent JENKINS-20560
if (vars.containsKey("PATH")) {
final String overallPaths=vars.get("PATH");
vars.remove("PATH");
vars.put("PATH+", overallPaths);
}

return getInner().launch(starter.envs(Util.mapToEnv(vars)));
return getInner().launch(starter.envs(vars));
}

private EnvVars toEnvVars(String[] envs) {
Expand Down
Expand Up @@ -68,32 +68,38 @@ public boolean add(String path) {
/**
* Adds PathsList and overrides null variables.
* @param pathsList PathsList to be added
* @return True if
* @return True if the paths list has been modified after the tool installation
*/
//TODO: Is it a bug?
public boolean add(PathsList pathsList) {
if (pathSeparator == null) {
pathSeparator = pathsList.pathSeparator;
}
if (separator == null) {
separator = pathsList.separator;
}
// Add homeDir as well (legacy behavior)
if (pathsList.homeDir != null) {
this.paths.add(pathsList.homeDir);
}

return this.paths.addAll(pathsList.paths);
}

/**
* Gets the list of installed tools
* @return A list with valid delimiters or null if paths is empty
*/
public String toListString() {
StringBuilder builder = new StringBuilder(paths.size()*2);
for ( String path : paths) {
builder.append(pathSeparator);
builder.append(path);
}
if (paths.isEmpty()) {
return null;
}

// Add homeDir as well (legacy behavior)
builder.append(pathSeparator);

StringBuilder builder = new StringBuilder();
for ( String path : paths) {
builder.append(path);
builder.append(pathSeparator);
}
return builder.toString();
}
}
Expand Up @@ -58,6 +58,8 @@ public void testNestedWrapperReverse() throws Exception {
nestedWrapperTestImpl(wrappers, true);
}



/**
* Tests custom tools with wrapper, which calls wrapper without
* specifying of envs.
Expand All @@ -71,6 +73,14 @@ public void testNestedLauncherCalls() throws Exception {
nestedWrapperTestImpl(wrappers, false);
}

//@Bug(20560)
public void testEmptyToolsList() throws Exception {
List<BuildWrapper> wrappers = new ArrayList<BuildWrapper>(0);
wrappers.add(new CommandCallerInstaller());
wrappers.add(new CustomToolInstallWrapper(null, MulticonfigWrapperOptions.DEFAULT, false));
nestedWrapperTestImpl(wrappers, false);
}

/**
* Implements tests for nested wrappers.
* The test checks that environment variables have been set correctly.
Expand Down

0 comments on commit 6c81ebb

Please sign in to comment.