Skip to content

Commit

Permalink
[FIXED JENKINS-19557] - Improved MacroStringHelper in order to avoid …
Browse files Browse the repository at this point in the history 
…substitution errors and improve performance.

Resolves https://issues.jenkins-ci.org/browse/JENKINS-19557

Signed-off-by: Oleg Nenashev <nenashev@synopsys.com>
  • Loading branch information
@oleg-nenashev
oleg-nenashev committed Sep 13, 2013
1 parent 7c67b46 commit 06c933a
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 46 deletions.
52 changes: 26 additions & 26 deletions src/main/java/hudson/plugins/perforce/PerforceSCM.java
View file
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
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
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

0 comments on commit 06c933a

Please sign in to comment.