Skip to content

[FIXED JENKINS-19557] - Improved MacroStringHelper in order to avoid sub... #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rpetti merged 1 commit into from
Sep 13, 2013
Merged

[FIXED JENKINS-19557] - Improved MacroStringHelper in order to avoid sub... #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 26 additions & 26 deletions src/main/java/hudson/plugins/perforce/PerforceSCM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,9 @@ protected Depot getDepot(Launcher launcher, FilePath workspace, AbstractProject
Depot depot = new Depot(p4Factory);

if (build != null) {
depot.setClient(MacroStringHelper.substituteParameters(p4Client, build));
depot.setUser(MacroStringHelper.substituteParameters(p4User, build));
depot.setPort(MacroStringHelper.substituteParameters(p4Port, build));
depot.setClient(MacroStringHelper.substituteParameters(p4Client, build, null));
depot.setUser(MacroStringHelper.substituteParameters(p4User, build, null));
depot.setPort(MacroStringHelper.substituteParameters(p4Port, build, null));
depot.setPassword(getDecryptedP4Passwd(build));
} else if (project != null) {
depot.setClient(MacroStringHelper.substituteParameters(p4Client, getDefaultSubstitutions(project)));
Expand Down Expand Up @@ -476,10 +476,10 @@ protected Depot getDepot(Launcher launcher, FilePath workspace, AbstractProject
public void buildEnvVars(AbstractBuild build, Map<String, String> env) {
super.buildEnvVars(build, env);
try {
env.put("P4PORT", MacroStringHelper.substituteParameters(p4Port, build));
env.put("P4USER", MacroStringHelper.substituteParameters(p4User, build));
env.put("P4PORT", MacroStringHelper.substituteParameters(p4Port, build, env));
env.put("P4USER", MacroStringHelper.substituteParameters(p4User, build, env));
} catch (ParameterSubstitutionException ex) {
LOGGER.log(Level.SEVERE, "Can't substitute P4USER or P4PORT", ex);
LOGGER.log(MacroStringHelper.SUBSTITUTION_ERROR_LEVEL, "Can't substitute P4USER or P4PORT", ex);
//TODO: exit?
}

Expand All @@ -495,9 +495,9 @@ public void buildEnvVars(AbstractBuild build, Map<String, String> env) {
}

try {
env.put("P4CLIENT", getConcurrentClientName(build.getWorkspace(), getEffectiveClientName(build)));
env.put("P4CLIENT", getConcurrentClientName(build.getWorkspace(), getEffectiveClientName(build, env)));
} catch(ParameterSubstitutionException ex) {
LOGGER.log(Level.SEVERE, "Can't substitute P$CLIENT",ex);
LOGGER.log(MacroStringHelper.SUBSTITUTION_ERROR_LEVEL, "Can't substitute P4CLIENT",ex);
//TODO: exit?
}

Expand Down Expand Up @@ -637,7 +637,7 @@ private String getEffectiveProjectPath(AbstractBuild build, AbstractProject proj
if (useClientSpec) {
projectPath = getEffectiveProjectPathFromFile(build, project, log, depot);
} else if (build != null) {
projectPath = MacroStringHelper.substituteParameters(this.projectPath, build);
projectPath = MacroStringHelper.substituteParameters(this.projectPath, build, null);
} else {
projectPath = MacroStringHelper.substituteParameters(this.projectPath, getDefaultSubstitutions(project));
}
Expand All @@ -647,15 +647,15 @@ private String getEffectiveProjectPath(AbstractBuild build, AbstractProject proj
private String getEffectiveProjectPathFromFile(AbstractBuild build, AbstractProject project, PrintStream log, Depot depot) throws PerforceException, ParameterSubstitutionException {
String clientSpec;
if (build != null) {
clientSpec = MacroStringHelper.substituteParameters(this.clientSpec, build);
clientSpec = MacroStringHelper.substituteParameters(this.clientSpec, build, null);
} else {
clientSpec = MacroStringHelper.substituteParametersNoCheck(this.clientSpec, getDefaultSubstitutions(project));
}
log.println("Read ClientSpec from: " + clientSpec);
com.tek42.perforce.parse.File f = depot.getFile(clientSpec);
String projectPath = f.read();
if (build != null) {
projectPath = MacroStringHelper.substituteParameters(projectPath, build);
projectPath = MacroStringHelper.substituteParameters(projectPath, build, null);
} else {
projectPath = MacroStringHelper.substituteParametersNoCheck(projectPath, getDefaultSubstitutions(project));
}
Expand Down Expand Up @@ -803,13 +803,13 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
PrintStream log = listener.getLogger();
changelogFilename = changelogFile.getAbsolutePath();
// HACK: Force build env vars to initialize
MacroStringHelper.substituteParameters("", build);
MacroStringHelper.substituteParameters("", build, null);

// Use local variables so that substitutions are not saved
String p4Label = MacroStringHelper.substituteParameters(this.p4Label, build);
String viewMask = MacroStringHelper.substituteParameters(this.viewMask, build);
String p4Label = MacroStringHelper.substituteParameters(this.p4Label, build, null);
String viewMask = MacroStringHelper.substituteParameters(this.viewMask, build, null);
Depot depot = getDepot(launcher,workspace, build.getProject(), build, build.getBuiltOn());
String p4Stream = MacroStringHelper.substituteParameters(this.p4Stream, build);
String p4Stream = MacroStringHelper.substituteParameters(this.p4Stream, build, null);

// Pull from optional named parameters
boolean wipeBeforeBuild = overrideWithBooleanParameter(
Expand Down Expand Up @@ -849,7 +849,7 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
String p4config;
WipeWorkspaceExcludeFilter wipeFilter;
try {
p4config = MacroStringHelper.substituteParameters("${P4CONFIG}", build);
p4config = MacroStringHelper.substituteParameters("${P4CONFIG}", build, null);
wipeFilter = new WipeWorkspaceExcludeFilter(".p4config",p4config);
} catch (ParameterSubstitutionException ex) {
wipeFilter = new WipeWorkspaceExcludeFilter();
Expand Down Expand Up @@ -895,7 +895,7 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
if (useStreamDepot) {
if (dirtyWorkspace) {
// Support for concurrent builds
String p4Client = getConcurrentClientName(workspace, getEffectiveClientName(build));
String p4Client = getConcurrentClientName(workspace, getEffectiveClientName(build, null));
p4workspace = depot.getWorkspaces().getWorkspace(p4Client, p4Stream);
}
projectPath = p4workspace.getTrimmedViewsAsString();
Expand Down Expand Up @@ -938,7 +938,7 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
if (p4Counter != null && !updateCounterValue) {
//use a counter
String counterName;
counterName = MacroStringHelper.substituteParameters(this.p4Counter, build);
counterName = MacroStringHelper.substituteParameters(this.p4Counter, build, null);
Counter counter = depot.getCounters().getCounter(counterName);
newestChange = counter.getValue();
} else {
Expand Down Expand Up @@ -1067,14 +1067,14 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
// Add tagging action that enables the user to create a label
// for this build.
build.addAction(new PerforceTagAction(
build, depot, newestChange, projectPath, MacroStringHelper.substituteParameters(p4User, build)));
build, depot, newestChange, projectPath, MacroStringHelper.substituteParameters(p4User, build, null)));

build.addAction(new PerforceSCMRevisionState(newestChange));

if (p4Counter != null && updateCounterValue) {
// Set or create a counter to mark this change
Counter counter = new Counter();
String counterName = MacroStringHelper.substituteParameters(this.p4Counter, build);
String counterName = MacroStringHelper.substituteParameters(this.p4Counter, build, null);
counter.setName(counterName);
counter.setValue(newestChange);
log.println("Updating counter " + counterName + " to " + newestChange);
Expand Down Expand Up @@ -1133,7 +1133,7 @@ private synchronized int getOrSetMatrixChangeSet(AbstractBuild build, Depot depo
// no changeset on parent, set it for other
// matrixruns to use
log.println("No change number has been set by parent/siblings. Using latest.");
parentBuild.addAction(new PerforceTagAction(build, depot, newestChange, projectPath, MacroStringHelper.substituteParameters(p4User,build)));
parentBuild.addAction(new PerforceTagAction(build, depot, newestChange, projectPath, MacroStringHelper.substituteParameters(p4User,build,null)));
}
}
}
Expand Down Expand Up @@ -1516,7 +1516,7 @@ private Workspace getPerforceWorkspace(AbstractProject project, String projectPa

String p4Client;
if (build != null) {
p4Client = getEffectiveClientName(build);
p4Client = getEffectiveClientName(build, null);
} else {
p4Client = getDefaultEffectiveClientName(project, buildNode, workspace);
}
Expand All @@ -1539,7 +1539,7 @@ else if (dontRenameClient) {
}

depot.setClient(p4Client);
String p4Stream = (build == null ? MacroStringHelper.substituteParameters(this.p4Stream, getDefaultSubstitutions(project)) : MacroStringHelper.substituteParameters(this.p4Stream, build));
String p4Stream = (build == null ? MacroStringHelper.substituteParameters(this.p4Stream, getDefaultSubstitutions(project)) : MacroStringHelper.substituteParameters(this.p4Stream, build, null));

// Get the clientspec (workspace) from perforce
Workspace p4workspace = depot.getWorkspaces().getWorkspace(p4Client, p4Stream);
Expand Down Expand Up @@ -1633,11 +1633,11 @@ else if (localPath.trim().equals(""))
return p4workspace;
}

private String getEffectiveClientName(AbstractBuild build) throws ParameterSubstitutionException {
private String getEffectiveClientName(AbstractBuild build, Map<String,String> env) throws ParameterSubstitutionException {
Node buildNode = build.getBuiltOn();
FilePath workspace = build.getWorkspace();
String p4Client = this.p4Client;
p4Client = MacroStringHelper.substituteParameters(p4Client, build);
p4Client = MacroStringHelper.substituteParameters(p4Client, build, env);
try {
p4Client = getEffectiveClientName(p4Client, buildNode);
} catch (Exception e) {
Expand Down Expand Up @@ -2532,7 +2532,7 @@ public String getDecryptedP4Passwd() {
}

public String getDecryptedP4Passwd(AbstractBuild build) throws ParameterSubstitutionException {
return MacroStringHelper.substituteParameters(getDecryptedP4Passwd(), build);
return MacroStringHelper.substituteParameters(getDecryptedP4Passwd(), build, null);
}

public String getDecryptedP4Passwd(AbstractProject project) {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/hudson/plugins/perforce/PerforceTagNotifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ public boolean perform(AbstractBuild<?,?> build, Launcher launcher, BuildListene

String labelName, labelDesc, labelOwner;
try {
labelName = MacroStringHelper.substituteParameters(rawLabelName, build);
labelDesc = MacroStringHelper.substituteParameters(rawLabelDesc, build);
labelOwner = MacroStringHelper.substituteParameters(rawLabelOwner, build);
labelName = MacroStringHelper.substituteParameters(rawLabelName, build, null);
labelDesc = MacroStringHelper.substituteParameters(rawLabelDesc, build, null);
labelOwner = MacroStringHelper.substituteParameters(rawLabelOwner, build, null);
} catch (ParameterSubstitutionException ex) {
listener.getLogger().println("Parameter substitution error in label items. "+ex.getMessage());
return false;
Expand Down
66 changes: 49 additions & 17 deletions src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
*/
package hudson.plugins.perforce.utils;

import hudson.AbortException;
import hudson.EnvVars;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
Expand All @@ -36,6 +35,7 @@
import java.io.IOException;
import java.util.Hashtable;
import java.util.Map;
import java.util.TreeMap;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand All @@ -49,7 +49,7 @@
* @since 1.3.25
*/
public class MacroStringHelper {

public static final Level SUBSTITUTION_ERROR_LEVEL = Level.WARNING;
/**
* Substitute parameters and validate contents of the resulting string
* @param string Input string
Expand All @@ -69,13 +69,14 @@ public static String substituteParameters(String string, Map<String, String> sub
* Substitute parameters and validate contents of the resulting string
* @param string Input string
* @param build Related build
* @param env Additional variables to be substituted. Used as a workaround for build environment
* @return Substituted string
* @throws ParameterSubstitutionException Format error (unresolved variable, etc.)
*/
public static String substituteParameters(String string, AbstractBuild build)
public static String substituteParameters(String string, AbstractBuild build, Map<String, String> env)
throws ParameterSubstitutionException
{
String result = substituteParametersNoCheck(string, build);
String result = substituteParametersNoCheck(string, build, env);
checkString(result);
return result;
}
Expand Down Expand Up @@ -118,6 +119,10 @@ public static String substituteParametersNoCheck(String string, Map<String, Stri
}
return newString;
}

public static boolean containsMacro(String str) {
return str != null && str.contains("${");
}

/**
* Substitute parameters and validate contents of the resulting string
Expand All @@ -126,8 +131,39 @@ public static String substituteParametersNoCheck(String string, Map<String, Stri
* @return Substituted string
* @deprecated Use checked methods instead
*/
public static String substituteParametersNoCheck(String string, AbstractBuild build) {
Hashtable<String, String> subst = new Hashtable<String, String>();
public static String substituteParametersNoCheck(String inputString, AbstractBuild build, Map<String, String> env) {
if (!containsMacro(inputString)) {
return inputString;
}

// Substitute environment if possible
String string = inputString;
if (env != null && !env.isEmpty()) {
string = substituteParametersNoCheck(string, env);

//exit if no macros left
if (!containsMacro(inputString)) {
return string;
}
}

// Try to substitute via node and global environment
for (NodeProperty nodeProperty : Hudson.getInstance().getGlobalNodeProperties()) {
if (nodeProperty instanceof EnvironmentVariablesNodeProperty) {
string = ((EnvironmentVariablesNodeProperty) nodeProperty).getEnvVars().expand(string);
}
}
for (NodeProperty nodeProperty : build.getBuiltOn().getNodeProperties()) {
if (nodeProperty instanceof EnvironmentVariablesNodeProperty) {
string = ((EnvironmentVariablesNodeProperty) nodeProperty).getEnvVars().expand(string);
}
}
if (!containsMacro(string)) {
return string;
}

// The last attempts: Try to build the full environment
Map<String, String> subst = new TreeMap<String, String>();
boolean useEnvironment = true;
for (StackTraceElement ste : (new Throwable()).getStackTrace()) {
if (ste.getMethodName().equals("buildEnvVars") && ste.getClassName().equals(PerforceSCM.class.getName())) {
Expand All @@ -144,26 +180,22 @@ public static String substituteParametersNoCheck(String string, AbstractBuild bu
Logger.getLogger(PerforceSCM.class.getName()).log(Level.SEVERE, null, ex);
}
}
if (!containsMacro(string)) {
return string;
}

//TODO: remove?
subst.put("JOB_NAME", getSafeJobName(build));
String hudsonName = Hudson.getInstance().getDisplayName().toLowerCase();
subst.put("BUILD_TAG", hudsonName + "-" + build.getProject().getName() + "-" + String.valueOf(build.getNumber()));
subst.put("BUILD_ID", build.getId());
subst.put("BUILD_NUMBER", String.valueOf(build.getNumber()));
for (NodeProperty nodeProperty : Hudson.getInstance().getGlobalNodeProperties()) {
if (nodeProperty instanceof EnvironmentVariablesNodeProperty) {
subst.putAll(((EnvironmentVariablesNodeProperty) nodeProperty).getEnvVars());
}
}
for (NodeProperty nodeProperty : build.getBuiltOn().getNodeProperties()) {
if (nodeProperty instanceof EnvironmentVariablesNodeProperty) {
subst.putAll(((EnvironmentVariablesNodeProperty) nodeProperty).getEnvVars());
}
}

String result = MacroStringHelper.substituteParametersNoCheck(string, subst);
result = MacroStringHelper.substituteParametersNoCheck(result, build.getBuildVariables());
return result;
}

public static String getSafeJobName(AbstractBuild build) {
return getSafeJobName(build.getProject());
}
Expand Down