Skip to content

Commit 83c42fd

Browse files
rpettirpetti
committed
Merge pull request #35 from synopsys-arc-oss/master
[FIXED JENKINS-19557] - Improved MacroStringHelper in order to avoid sub...
2 parents 5018efc + 06c933a commit 83c42fd

File tree

3 files changed

+78
-46
lines changed

3 files changed

+78
-46
lines changed

‎src/main/java/hudson/plugins/perforce/PerforceSCM.java‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,9 @@ protected Depot getDepot(Launcher launcher, FilePath workspace, AbstractProject
409409
Depot depot = new Depot(p4Factory);
410410

411411
if (build != null) {
412-
depot.setClient(MacroStringHelper.substituteParameters(p4Client, build));
413-
depot.setUser(MacroStringHelper.substituteParameters(p4User, build));
414-
depot.setPort(MacroStringHelper.substituteParameters(p4Port, build));
412+
depot.setClient(MacroStringHelper.substituteParameters(p4Client, build, null));
413+
depot.setUser(MacroStringHelper.substituteParameters(p4User, build, null));
414+
depot.setPort(MacroStringHelper.substituteParameters(p4Port, build, null));
415415
depot.setPassword(getDecryptedP4Passwd(build));
416416
} else if (project != null) {
417417
depot.setClient(MacroStringHelper.substituteParameters(p4Client, getDefaultSubstitutions(project)));
@@ -476,10 +476,10 @@ protected Depot getDepot(Launcher launcher, FilePath workspace, AbstractProject
476476
public void buildEnvVars(AbstractBuild build, Map<String, String> env) {
477477
super.buildEnvVars(build, env);
478478
try {
479-
env.put("P4PORT", MacroStringHelper.substituteParameters(p4Port, build));
480-
env.put("P4USER", MacroStringHelper.substituteParameters(p4User, build));
479+
env.put("P4PORT", MacroStringHelper.substituteParameters(p4Port, build, env));
480+
env.put("P4USER", MacroStringHelper.substituteParameters(p4User, build, env));
481481
} catch (ParameterSubstitutionException ex) {
482-
LOGGER.log(Level.SEVERE, "Can't substitute P4USER or P4PORT", ex);
482+
LOGGER.log(MacroStringHelper.SUBSTITUTION_ERROR_LEVEL, "Can't substitute P4USER or P4PORT", ex);
483483
//TODO: exit?
484484
}
485485

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

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

@@ -637,7 +637,7 @@ private String getEffectiveProjectPath(AbstractBuild build, AbstractProject proj
637637
if (useClientSpec) {
638638
projectPath = getEffectiveProjectPathFromFile(build, project, log, depot);
639639
} else if (build != null) {
640-
projectPath = MacroStringHelper.substituteParameters(this.projectPath, build);
640+
projectPath = MacroStringHelper.substituteParameters(this.projectPath, build, null);
641641
} else {
642642
projectPath = MacroStringHelper.substituteParameters(this.projectPath, getDefaultSubstitutions(project));
643643
}
@@ -647,15 +647,15 @@ private String getEffectiveProjectPath(AbstractBuild build, AbstractProject proj
647647
private String getEffectiveProjectPathFromFile(AbstractBuild build, AbstractProject project, PrintStream log, Depot depot) throws PerforceException, ParameterSubstitutionException {
648648
String clientSpec;
649649
if (build != null) {
650-
clientSpec = MacroStringHelper.substituteParameters(this.clientSpec, build);
650+
clientSpec = MacroStringHelper.substituteParameters(this.clientSpec, build, null);
651651
} else {
652652
clientSpec = MacroStringHelper.substituteParametersNoCheck(this.clientSpec, getDefaultSubstitutions(project));
653653
}
654654
log.println("Read ClientSpec from: " + clientSpec);
655655
com.tek42.perforce.parse.File f = depot.getFile(clientSpec);
656656
String projectPath = f.read();
657657
if (build != null) {
658-
projectPath = MacroStringHelper.substituteParameters(projectPath, build);
658+
projectPath = MacroStringHelper.substituteParameters(projectPath, build, null);
659659
} else {
660660
projectPath = MacroStringHelper.substituteParametersNoCheck(projectPath, getDefaultSubstitutions(project));
661661
}
@@ -803,13 +803,13 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
803803
PrintStream log = listener.getLogger();
804804
changelogFilename = changelogFile.getAbsolutePath();
805805
// HACK: Force build env vars to initialize
806-
MacroStringHelper.substituteParameters("", build);
806+
MacroStringHelper.substituteParameters("", build, null);
807807

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

814814
// Pull from optional named parameters
815815
boolean wipeBeforeBuild = overrideWithBooleanParameter(
@@ -849,7 +849,7 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
849849
String p4config;
850850
WipeWorkspaceExcludeFilter wipeFilter;
851851
try {
852-
p4config = MacroStringHelper.substituteParameters("${P4CONFIG}", build);
852+
p4config = MacroStringHelper.substituteParameters("${P4CONFIG}", build, null);
853853
wipeFilter = new WipeWorkspaceExcludeFilter(".p4config",p4config);
854854
} catch (ParameterSubstitutionException ex) {
855855
wipeFilter = new WipeWorkspaceExcludeFilter();
@@ -895,7 +895,7 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
895895
if (useStreamDepot) {
896896
if (dirtyWorkspace) {
897897
// Support for concurrent builds
898-
String p4Client = getConcurrentClientName(workspace, getEffectiveClientName(build));
898+
String p4Client = getConcurrentClientName(workspace, getEffectiveClientName(build, null));
899899
p4workspace = depot.getWorkspaces().getWorkspace(p4Client, p4Stream);
900900
}
901901
projectPath = p4workspace.getTrimmedViewsAsString();
@@ -938,7 +938,7 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
938938
if (p4Counter != null && !updateCounterValue) {
939939
//use a counter
940940
String counterName;
941-
counterName = MacroStringHelper.substituteParameters(this.p4Counter, build);
941+
counterName = MacroStringHelper.substituteParameters(this.p4Counter, build, null);
942942
Counter counter = depot.getCounters().getCounter(counterName);
943943
newestChange = counter.getValue();
944944
} else {
@@ -1067,14 +1067,14 @@ public boolean checkout(AbstractBuild build, Launcher launcher,
10671067
// Add tagging action that enables the user to create a label
10681068
// for this build.
10691069
build.addAction(new PerforceTagAction(
1070-
build, depot, newestChange, projectPath, MacroStringHelper.substituteParameters(p4User, build)));
1070+
build, depot, newestChange, projectPath, MacroStringHelper.substituteParameters(p4User, build, null)));
10711071

10721072
build.addAction(new PerforceSCMRevisionState(newestChange));
10731073

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

15171517
String p4Client;
15181518
if (build != null) {
1519-
p4Client = getEffectiveClientName(build);
1519+
p4Client = getEffectiveClientName(build, null);
15201520
} else {
15211521
p4Client = getDefaultEffectiveClientName(project, buildNode, workspace);
15221522
}
@@ -1539,7 +1539,7 @@ else if (dontRenameClient) {
15391539
}
15401540

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

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

1636-
private String getEffectiveClientName(AbstractBuild build) throws ParameterSubstitutionException {
1636+
private String getEffectiveClientName(AbstractBuild build, Map<String,String> env) throws ParameterSubstitutionException {
16371637
Node buildNode = build.getBuiltOn();
16381638
FilePath workspace = build.getWorkspace();
16391639
String p4Client = this.p4Client;
1640-
p4Client = MacroStringHelper.substituteParameters(p4Client, build);
1640+
p4Client = MacroStringHelper.substituteParameters(p4Client, build, env);
16411641
try {
16421642
p4Client = getEffectiveClientName(p4Client, buildNode);
16431643
} catch (Exception e) {
@@ -2532,7 +2532,7 @@ public String getDecryptedP4Passwd() {
25322532
}
25332533

25342534
public String getDecryptedP4Passwd(AbstractBuild build) throws ParameterSubstitutionException {
2535-
return MacroStringHelper.substituteParameters(getDecryptedP4Passwd(), build);
2535+
return MacroStringHelper.substituteParameters(getDecryptedP4Passwd(), build, null);
25362536
}
25372537

25382538
public String getDecryptedP4Passwd(AbstractProject project) {

‎src/main/java/hudson/plugins/perforce/PerforceTagNotifier.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,9 @@ public boolean perform(AbstractBuild<?,?> build, Launcher launcher, BuildListene
115115

116116
String labelName, labelDesc, labelOwner;
117117
try {
118-
labelName = MacroStringHelper.substituteParameters(rawLabelName, build);
119-
labelDesc = MacroStringHelper.substituteParameters(rawLabelDesc, build);
120-
labelOwner = MacroStringHelper.substituteParameters(rawLabelOwner, build);
118+
labelName = MacroStringHelper.substituteParameters(rawLabelName, build, null);
119+
labelDesc = MacroStringHelper.substituteParameters(rawLabelDesc, build, null);
120+
labelOwner = MacroStringHelper.substituteParameters(rawLabelOwner, build, null);
121121
} catch (ParameterSubstitutionException ex) {
122122
listener.getLogger().println("Parameter substitution error in label items. "+ex.getMessage());
123123
return false;

‎src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
*/
2525
package hudson.plugins.perforce.utils;
2626

27-
import hudson.AbortException;
2827
import hudson.EnvVars;
2928
import hudson.model.AbstractBuild;
3029
import hudson.model.AbstractProject;
@@ -36,6 +35,7 @@
3635
import java.io.IOException;
3736
import java.util.Hashtable;
3837
import java.util.Map;
38+
import java.util.TreeMap;
3939
import java.util.logging.Level;
4040
import java.util.logging.Logger;
4141

@@ -49,7 +49,7 @@
4949
* @since 1.3.25
5050
*/
5151
public class MacroStringHelper {
52-
52+
public static final Level SUBSTITUTION_ERROR_LEVEL = Level.WARNING;
5353
/**
5454
* Substitute parameters and validate contents of the resulting string
5555
* @param string Input string
@@ -69,13 +69,14 @@ public static String substituteParameters(String string, Map<String, String> sub
6969
* Substitute parameters and validate contents of the resulting string
7070
* @param string Input string
7171
* @param build Related build
72+
* @param env Additional variables to be substituted. Used as a workaround for build environment
7273
* @return Substituted string
7374
* @throws ParameterSubstitutionException Format error (unresolved variable, etc.)
7475
*/
75-
public static String substituteParameters(String string, AbstractBuild build)
76+
public static String substituteParameters(String string, AbstractBuild build, Map<String, String> env)
7677
throws ParameterSubstitutionException
7778
{
78-
String result = substituteParametersNoCheck(string, build);
79+
String result = substituteParametersNoCheck(string, build, env);
7980
checkString(result);
8081
return result;
8182
}
@@ -118,6 +119,10 @@ public static String substituteParametersNoCheck(String string, Map<String, Stri
118119
}
119120
return newString;
120121
}
122+
123+
public static boolean containsMacro(String str) {
124+
return str != null && str.contains("${");
125+
}
121126

122127
/**
123128
* Substitute parameters and validate contents of the resulting string
@@ -126,8 +131,39 @@ public static String substituteParametersNoCheck(String string, Map<String, Stri
126131
* @return Substituted string
127132
* @deprecated Use checked methods instead
128133
*/
129-
public static String substituteParametersNoCheck(String string, AbstractBuild build) {
130-
Hashtable<String, String> subst = new Hashtable<String, String>();
134+
public static String substituteParametersNoCheck(String inputString, AbstractBuild build, Map<String, String> env) {
135+
if (!containsMacro(inputString)) {
136+
return inputString;
137+
}
138+
139+
// Substitute environment if possible
140+
String string = inputString;
141+
if (env != null && !env.isEmpty()) {
142+
string = substituteParametersNoCheck(string, env);
143+
144+
//exit if no macros left
145+
if (!containsMacro(inputString)) {
146+
return string;
147+
}
148+
}
149+
150+
// Try to substitute via node and global environment
151+
for (NodeProperty nodeProperty : Hudson.getInstance().getGlobalNodeProperties()) {
152+
if (nodeProperty instanceof EnvironmentVariablesNodeProperty) {
153+
string = ((EnvironmentVariablesNodeProperty) nodeProperty).getEnvVars().expand(string);
154+
}
155+
}
156+
for (NodeProperty nodeProperty : build.getBuiltOn().getNodeProperties()) {
157+
if (nodeProperty instanceof EnvironmentVariablesNodeProperty) {
158+
string = ((EnvironmentVariablesNodeProperty) nodeProperty).getEnvVars().expand(string);
159+
}
160+
}
161+
if (!containsMacro(string)) {
162+
return string;
163+
}
164+
165+
// The last attempts: Try to build the full environment
166+
Map<String, String> subst = new TreeMap<String, String>();
131167
boolean useEnvironment = true;
132168
for (StackTraceElement ste : (new Throwable()).getStackTrace()) {
133169
if (ste.getMethodName().equals("buildEnvVars") && ste.getClassName().equals(PerforceSCM.class.getName())) {
@@ -144,26 +180,22 @@ public static String substituteParametersNoCheck(String string, AbstractBuild bu
144180
Logger.getLogger(PerforceSCM.class.getName()).log(Level.SEVERE, null, ex);
145181
}
146182
}
183+
if (!containsMacro(string)) {
184+
return string;
185+
}
186+
187+
//TODO: remove?
147188
subst.put("JOB_NAME", getSafeJobName(build));
148189
String hudsonName = Hudson.getInstance().getDisplayName().toLowerCase();
149190
subst.put("BUILD_TAG", hudsonName + "-" + build.getProject().getName() + "-" + String.valueOf(build.getNumber()));
150191
subst.put("BUILD_ID", build.getId());
151192
subst.put("BUILD_NUMBER", String.valueOf(build.getNumber()));
152-
for (NodeProperty nodeProperty : Hudson.getInstance().getGlobalNodeProperties()) {
153-
if (nodeProperty instanceof EnvironmentVariablesNodeProperty) {
154-
subst.putAll(((EnvironmentVariablesNodeProperty) nodeProperty).getEnvVars());
155-
}
156-
}
157-
for (NodeProperty nodeProperty : build.getBuiltOn().getNodeProperties()) {
158-
if (nodeProperty instanceof EnvironmentVariablesNodeProperty) {
159-
subst.putAll(((EnvironmentVariablesNodeProperty) nodeProperty).getEnvVars());
160-
}
161-
}
193+
162194
String result = MacroStringHelper.substituteParametersNoCheck(string, subst);
163195
result = MacroStringHelper.substituteParametersNoCheck(result, build.getBuildVariables());
164196
return result;
165197
}
166-
198+
167199
public static String getSafeJobName(AbstractBuild build) {
168200
return getSafeJobName(build.getProject());
169201
}

0 commit comments

Comments
 (0)