Skip to content

Commit bfa3643

Browse files
comiuscopybara-github
authored andcommitted
Implement substitution in SpawnAction progress message.
The native SpawnAction provides formatter to improve performance (formatted string are not retained), while this was not available in Starlark. The cl exposes formatting with default placeholders to Starlark and as well to native implementation. I marked old native api with formatting as deprecated as only the string with default placeholders can be used. PiperOrigin-RevId: 385753127
1 parent f880948 commit bfa3643

File tree

5 files changed

+69
-19
lines changed

5 files changed

+69
-19
lines changed

src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,25 @@ public final String getMnemonic() {
470470
@Override
471471
protected String getRawProgressMessage() {
472472
if (progressMessage != null) {
473-
return progressMessage.toString();
473+
if (progressMessage instanceof String) {
474+
String progressMessageStr = (String) progressMessage;
475+
if (progressMessageStr.contains("%{label}") && getOwner().getLabel() != null) {
476+
progressMessageStr =
477+
progressMessageStr.replace("%{label}", getOwner().getLabel().toString());
478+
}
479+
if (progressMessageStr.contains("%{output}") && getPrimaryOutput() != null) {
480+
progressMessageStr =
481+
progressMessageStr.replace("%{output}", getPrimaryOutput().getExecPathString());
482+
}
483+
if (progressMessageStr.contains("%{input}") && getPrimaryInput() != null) {
484+
progressMessageStr =
485+
progressMessageStr.replace("%{input}", getPrimaryInput().getExecPathString());
486+
}
487+
return progressMessageStr;
488+
} else {
489+
// handles OnDemandString
490+
return progressMessage.toString();
491+
}
474492
}
475493
return super.getRawProgressMessage();
476494
}
@@ -1172,14 +1190,9 @@ public Builder addCommandLine(CommandLine commandLine, @Nullable ParamFileInfo p
11721190
/**
11731191
* Sets the progress message.
11741192
*
1175-
* <p>If you are formatting the string in any way, prefer one of the overloads that do the
1176-
* formatting lazily. This helps save memory by delaying the construction of the progress
1177-
* message string.
1178-
*
1179-
* <p>If you cannot use simple formatting, try {@link
1180-
* Builder#setProgressMessage(OnDemandString)}.
1181-
*
1182-
* <p>If you must eagerly compute the string, use {@link Builder#setProgressMessageNonLazy}.
1193+
* <p>The message may contain <code>%{label}</code>, <code>%{input}</code> or <code>%{output}
1194+
* </code> patterns, which are substituted with label string, first input or output's path,
1195+
* respectively.
11831196
*/
11841197
public Builder setProgressMessage(@CompileTimeConstant String progressMessage) {
11851198
this.progressMessage = progressMessage;
@@ -1191,8 +1204,10 @@ public Builder setProgressMessage(@CompileTimeConstant String progressMessage) {
11911204
*
11921205
* @param progressMessage The message to display
11931206
* @param subject Passed to {@link String#format}
1207+
* @deprecated Use {@link #setProgressMessage(String)} with provided patterns.
11941208
*/
11951209
@FormatMethod
1210+
@Deprecated
11961211
public Builder setProgressMessage(@FormatString String progressMessage, Object subject) {
11971212
return setProgressMessage(
11981213
new OnDemandString() {
@@ -1209,8 +1224,10 @@ public String toString() {
12091224
* @param progressMessage The message to display
12101225
* @param subject0 Passed to {@link String#format}
12111226
* @param subject1 Passed to {@link String#format}
1227+
* @deprecated Use {@link #setProgressMessage(String)} with provided patterns.
12121228
*/
12131229
@FormatMethod
1230+
@Deprecated
12141231
public Builder setProgressMessage(
12151232
@FormatString String progressMessage, Object subject0, Object subject1) {
12161233
return setProgressMessage(
@@ -1229,8 +1246,10 @@ public String toString() {
12291246
* @param subject0 Passed to {@link String#format}
12301247
* @param subject1 Passed to {@link String#format}
12311248
* @param subject2 Passed to {@link String#format}
1249+
* @deprecated Use {@link #setProgressMessage(String)} with provided patterns.
12321250
*/
12331251
@FormatMethod
1252+
@Deprecated
12341253
public Builder setProgressMessage(
12351254
@FormatString String progressMessage, Object subject0, Object subject1, Object subject2) {
12361255
return setProgressMessage(
@@ -1250,8 +1269,10 @@ public String toString() {
12501269
* @param subject1 Passed to {@link String#format}
12511270
* @param subject2 Passed to {@link String#format}
12521271
* @param subject3 Passed to {@link String#format}
1272+
* @deprecated Use {@link #setProgressMessage(String)} with provided patterns.
12531273
*/
12541274
@FormatMethod
1275+
@Deprecated
12551276
public Builder setProgressMessage(
12561277
@FormatString String progressMessage,
12571278
Object subject0,
@@ -1273,17 +1294,18 @@ public String toString() {
12731294
* <p>When possible, prefer use of one of the overloads that use {@link String#format}. If you
12741295
* do use this overload, take care not to capture anything expensive.
12751296
*/
1276-
public Builder setProgressMessage(OnDemandString progressMessage) {
1297+
private Builder setProgressMessage(OnDemandString progressMessage) {
12771298
this.progressMessage = progressMessage;
12781299
return this;
12791300
}
12801301

12811302
/**
1282-
* Sets an eagerly computed progress message.
1303+
* Sets the progress message.
12831304
*
1284-
* <p>Prefer one of the lazy overloads whenever possible, as it will generally save memory.
1305+
* <p>Same as {@link #setProgressMessage(String)}, except that it may be used with non compile
1306+
* time constants (needed for Starlark literals).
12851307
*/
1286-
public Builder setProgressMessageNonLazy(String progressMessage) {
1308+
public Builder setProgressMessageFromStarlark(String progressMessage) {
12871309
this.progressMessage = progressMessage;
12881310
return this;
12891311
}

src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public final class StarlarkAction extends SpawnAction implements ActionCacheAwar
7878
* modified
7979
* @param outputs the set of all files written by this action; must not be subsequently modified.
8080
* @param primaryOutput the primary output of this action
81-
* @param resourceSet the resources consumed by executing this Action
81+
* @param resourceSetOrBuilder the resources consumed by executing this Action
8282
* @param commandLines the command lines to execute. This includes the main argv vector and any
8383
* param file-backed command lines.
8484
* @param commandLineLimits the command line limits, from the build configuration

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ private void registerStarlarkAction(
599599
ImmutableMap.copyOf(Dict.cast(envUnchecked, String.class, String.class, "env")));
600600
}
601601
if (progressMessage != Starlark.NONE) {
602-
builder.setProgressMessageNonLazy((String) progressMessage);
602+
builder.setProgressMessageFromStarlark((String) progressMessage);
603603
}
604604
if (Starlark.truth(useDefaultShellEnv)) {
605605
builder.useDefaultShellEnvironment();

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,12 @@ void symlink(
360360
named = true,
361361
positional = false,
362362
doc =
363-
"Progress message to show to the user during the build, "
364-
+ "for example, \"Compiling foo.cc to create foo.o\"."),
363+
"Progress message to show to the user during the build, for example, \"Compiling"
364+
+ " foo.cc to create foo.o\". The message may contain <code>%{label}</code>,"
365+
+ " <code>%{input}</code>, or <code>%{output}</code> patterns, which are"
366+
+ " substituted with label string, first input, or output's path,"
367+
+ " respectively. Prefer to use patterns instead of static strings, because"
368+
+ " the former are more efficient."),
365369
@Param(
366370
name = "use_default_shell_env",
367371
defaultValue = "False",
@@ -561,8 +565,12 @@ void run(
561565
named = true,
562566
positional = false,
563567
doc =
564-
"Progress message to show to the user during the build, "
565-
+ "for example, \"Compiling foo.cc to create foo.o\"."),
568+
"Progress message to show to the user during the build, for example, \"Compiling"
569+
+ " foo.cc to create foo.o\". The message may contain <code>%{label}</code>,"
570+
+ " <code>%{input}</code>, or <code>%{output}</code> patterns, which are"
571+
+ " substituted with label string, first input, or output's path,"
572+
+ " respectively. Prefer to use patterns instead of static strings, because"
573+
+ " the former are more efficient."),
566574
@Param(
567575
name = "use_default_shell_env",
568576
defaultValue = "False",

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,26 @@ public void testCreateSpawnActionArgumentsWithExecutable() throws Exception {
310310
MoreAsserts.assertContainsSublist(action.getArguments(), "foo/t.exe", "--a", "--b");
311311
}
312312

313+
@Test
314+
public void createSpawnAction_progressMessageWithSubstitutions() throws Exception {
315+
StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
316+
setRuleContext(ruleContext);
317+
ev.exec(
318+
"ruleContext.actions.run(",
319+
" inputs = ruleContext.files.srcs,",
320+
" outputs = ruleContext.files.srcs[1:],",
321+
" executable = ruleContext.files.tools[0],",
322+
" mnemonic = 'DummyMnemonic',",
323+
" progress_message = 'message %{label} %{input} %{output}')");
324+
325+
SpawnAction action =
326+
(SpawnAction)
327+
Iterables.getOnlyElement(
328+
ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
329+
330+
assertThat(action.getProgressMessage()).isEqualTo("message //foo:foo foo/a.txt foo/b.img");
331+
}
332+
313333
@Test
314334
public void testCreateActionWithDepsetInput() throws Exception {
315335
// Same test as above, with depset as inputs.

0 commit comments

Comments
 (0)