Skip to content

Commit 850da07

Browse files
committed
Show display form labels in java_* buildozer fixups
The change in bazelbuild#21702 only fixed the buildozer fixup messages for `java_*` actions entirely defined in Starlark. This commit makes the analogous change to the remaining native actions. Related to bazelbuild#20486 and bazelbuild#21702 Closes bazelbuild#21827. PiperOrigin-RevId: 623253302 Change-Id: Iadc2b0d8ce64f359b921ad7a7c489111a3a29997
1 parent 58a2922 commit 850da07

File tree

9 files changed

+207
-25
lines changed

9 files changed

+207
-25
lines changed

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

+65-10
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.devtools.build.lib.actions.PathMapper;
3535
import com.google.devtools.build.lib.actions.SingleStringArgFormatter;
3636
import com.google.devtools.build.lib.cmdline.Label;
37+
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
3738
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
3839
import com.google.devtools.build.lib.concurrent.BlazeInterners;
3940
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -580,9 +581,16 @@ static class PrefixArg implements ArgvFragment {
580581

581582
private static final UUID PREFIX_UUID = UUID.fromString("a95eccdf-4f54-46fc-b925-c8c7e1f50c95");
582583

583-
private static void push(List<Object> arguments, String before, Object arg) {
584+
private static void push(
585+
List<Object> arguments,
586+
String before,
587+
Object arg,
588+
@Nullable RepositoryMapping mainRepoMapping) {
584589
arguments.add(INSTANCE);
585590
arguments.add(before);
591+
if (mainRepoMapping != null) {
592+
arguments.add(mainRepoMapping);
593+
}
586594
arguments.add(arg);
587595
}
588596

@@ -594,6 +602,9 @@ public int eval(
594602
PathMapper pathMapper) {
595603
String before = (String) arguments.get(argi++);
596604
Object arg = arguments.get(argi++);
605+
if (arg instanceof RepositoryMapping mainRepoMapping) {
606+
arg = ((Label) arguments.get(argi++)).getDisplayForm(mainRepoMapping);
607+
}
597608
builder.add(before + CommandLineItem.expandToCommandLine(arg));
598609
return argi;
599610
}
@@ -606,7 +617,11 @@ public int addToFingerprint(
606617
Fingerprint fingerprint) {
607618
fingerprint.addUUID(PREFIX_UUID);
608619
fingerprint.addString((String) arguments.get(argi++));
609-
fingerprint.addString(CommandLineItem.expandToCommandLine(arguments.get(argi++)));
620+
Object arg = arguments.get(argi++);
621+
if (arg instanceof RepositoryMapping mainRepoMapping) {
622+
arg = ((Label) arguments.get(argi++)).getDisplayForm(mainRepoMapping);
623+
}
624+
fingerprint.addString(CommandLineItem.expandToCommandLine(arg));
610625
return argi;
611626
}
612627
}
@@ -752,6 +767,7 @@ public static final class Builder {
752767
*
753768
* <p>Prefer this over its dynamic cousin, as using static strings saves memory.
754769
*/
770+
@CanIgnoreReturnValue
755771
public Builder add(@CompileTimeConstant String value) {
756772
return addObjectInternal(value);
757773
}
@@ -761,6 +777,7 @@ public Builder add(@CompileTimeConstant String value) {
761777
*
762778
* <p>If the value is null, neither the arg nor the value is added.
763779
*/
780+
@CanIgnoreReturnValue
764781
public Builder add(@CompileTimeConstant String arg, @Nullable String value) {
765782
return addObjectInternal(arg, value);
766783
}
@@ -797,6 +814,7 @@ public Builder addObject(@Nullable Object value) {
797814
* <p>There are many other ways you can try to avoid calling this. In general, try to use
798815
* constants or objects that are already on the heap elsewhere.
799816
*/
817+
@CanIgnoreReturnValue
800818
public Builder addDynamicString(@Nullable String value) {
801819
return addObjectInternal(value);
802820
}
@@ -807,6 +825,7 @@ public Builder addDynamicString(@Nullable String value) {
807825
* <p>Prefer this over manually calling {@link Label#getCanonicalForm}, as it avoids a copy of
808826
* the label value.
809827
*/
828+
@CanIgnoreReturnValue
810829
public Builder addLabel(@Nullable Label value) {
811830
return addObjectInternal(value);
812831
}
@@ -819,6 +838,7 @@ public Builder addLabel(@Nullable Label value) {
819838
*
820839
* <p>If the value is null, neither the arg nor the value is added.
821840
*/
841+
@CanIgnoreReturnValue
822842
public Builder addLabel(@CompileTimeConstant String arg, @Nullable Label value) {
823843
return addObjectInternal(arg, value);
824844
}
@@ -829,6 +849,7 @@ public Builder addLabel(@CompileTimeConstant String arg, @Nullable Label value)
829849
* <p>Prefer this over manually calling {@link PathFragment#getPathString}, as it avoids storing
830850
* a copy of the path string.
831851
*/
852+
@CanIgnoreReturnValue
832853
public Builder addPath(@Nullable PathFragment value) {
833854
return addObjectInternal(value);
834855
}
@@ -841,6 +862,7 @@ public Builder addPath(@Nullable PathFragment value) {
841862
*
842863
* <p>If the value is null, neither the arg nor the value is added.
843864
*/
865+
@CanIgnoreReturnValue
844866
public Builder addPath(@CompileTimeConstant String arg, @Nullable PathFragment value) {
845867
return addObjectInternal(arg, value);
846868
}
@@ -851,6 +873,7 @@ public Builder addPath(@CompileTimeConstant String arg, @Nullable PathFragment v
851873
* <p>Prefer this over manually calling {@link Artifact#getExecPath}, as it avoids storing a
852874
* copy of the artifact path string.
853875
*/
876+
@CanIgnoreReturnValue
854877
public Builder addExecPath(@Nullable Artifact value) {
855878
return addObjectInternal(value);
856879
}
@@ -863,16 +886,19 @@ public Builder addExecPath(@Nullable Artifact value) {
863886
*
864887
* <p>If the value is null, neither the arg nor the value is added.
865888
*/
889+
@CanIgnoreReturnValue
866890
public Builder addExecPath(@CompileTimeConstant String arg, @Nullable Artifact value) {
867891
return addObjectInternal(arg, value);
868892
}
869893

870894
/** Adds a lazily expanded string. */
895+
@CanIgnoreReturnValue
871896
public Builder addLazyString(@Nullable OnDemandString value) {
872897
return addObjectInternal(value);
873898
}
874899

875900
/** Adds a lazily expanded string. */
901+
@CanIgnoreReturnValue
876902
public Builder addLazyString(@CompileTimeConstant String arg, @Nullable OnDemandString value) {
877903
return addObjectInternal(arg, value);
878904
}
@@ -887,23 +913,33 @@ public Builder addFormatted(@FormatString String formatStr, Object... args) {
887913
}
888914

889915
/** Concatenates the passed prefix string and the string. */
916+
@CanIgnoreReturnValue
890917
public Builder addPrefixed(@CompileTimeConstant String prefix, @Nullable String arg) {
891-
return addPrefixedInternal(prefix, arg);
918+
return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
892919
}
893920

894-
/** Concatenates the passed prefix string and the label using {@link Label#getCanonicalForm}. */
895-
public Builder addPrefixedLabel(@CompileTimeConstant String prefix, @Nullable Label arg) {
896-
return addPrefixedInternal(prefix, arg);
921+
/**
922+
* Concatenates the passed prefix string and the label using {@link Label#getDisplayForm}, which
923+
* is identical to {@link Label#getCanonicalForm()} for main repo labels.
924+
*/
925+
@CanIgnoreReturnValue
926+
public Builder addPrefixedLabel(
927+
@CompileTimeConstant String prefix,
928+
@Nullable Label arg,
929+
RepositoryMapping mainRepoMapping) {
930+
return addPrefixedInternal(prefix, arg, mainRepoMapping);
897931
}
898932

899933
/** Concatenates the passed prefix string and the path. */
934+
@CanIgnoreReturnValue
900935
public Builder addPrefixedPath(@CompileTimeConstant String prefix, @Nullable PathFragment arg) {
901-
return addPrefixedInternal(prefix, arg);
936+
return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
902937
}
903938

904939
/** Concatenates the passed prefix string and the artifact's exec path. */
940+
@CanIgnoreReturnValue
905941
public Builder addPrefixedExecPath(@CompileTimeConstant String prefix, @Nullable Artifact arg) {
906-
return addPrefixedInternal(prefix, arg);
942+
return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
907943
}
908944

909945
/**
@@ -912,6 +948,7 @@ public Builder addPrefixedExecPath(@CompileTimeConstant String prefix, @Nullable
912948
* <p>If you are converting long lists or nested sets of a different type to string lists,
913949
* please try to use a different method that supports what you are trying to do directly.
914950
*/
951+
@CanIgnoreReturnValue
915952
public Builder addAll(@Nullable Collection<String> values) {
916953
return addCollectionInternal(values);
917954
}
@@ -922,6 +959,7 @@ public Builder addAll(@Nullable Collection<String> values) {
922959
* <p>If you are converting long lists or nested sets of a different type to string lists,
923960
* please try to use a different method that supports what you are trying to do directly.
924961
*/
962+
@CanIgnoreReturnValue
925963
public Builder addAll(@Nullable NestedSet<String> values) {
926964
return addNestedSetInternal(values);
927965
}
@@ -934,6 +972,7 @@ public Builder addAll(@Nullable NestedSet<String> values) {
934972
*
935973
* <p>If values is empty, the arg isn't added.
936974
*/
975+
@CanIgnoreReturnValue
937976
public Builder addAll(@CompileTimeConstant String arg, @Nullable Collection<String> values) {
938977
return addCollectionInternal(arg, values);
939978
}
@@ -943,11 +982,13 @@ public Builder addAll(@CompileTimeConstant String arg, @Nullable Collection<Stri
943982
*
944983
* <p>If values is empty, the arg isn't added.
945984
*/
985+
@CanIgnoreReturnValue
946986
public Builder addAll(@CompileTimeConstant String arg, @Nullable NestedSet<String> values) {
947987
return addNestedSetInternal(arg, values);
948988
}
949989

950990
/** Adds the passed vector arg. See {@link VectorArg}. */
991+
@CanIgnoreReturnValue
951992
public Builder addAll(VectorArg<String> vectorArg) {
952993
return addVectorArgInternal(vectorArg);
953994
}
@@ -957,16 +998,19 @@ public Builder addAll(VectorArg<String> vectorArg) {
957998
*
958999
* <p>If values is empty, the arg isn't added.
9591000
*/
1001+
@CanIgnoreReturnValue
9601002
public Builder addAll(@CompileTimeConstant String arg, VectorArg<String> vectorArg) {
9611003
return addVectorArgInternal(arg, vectorArg);
9621004
}
9631005

9641006
/** Adds the passed paths to the command line. */
1007+
@CanIgnoreReturnValue
9651008
public Builder addPaths(@Nullable Collection<PathFragment> values) {
9661009
return addCollectionInternal(values);
9671010
}
9681011

9691012
/** Adds the passed paths to the command line. */
1013+
@CanIgnoreReturnValue
9701014
public Builder addPaths(@Nullable NestedSet<PathFragment> values) {
9711015
return addNestedSetInternal(values);
9721016
}
@@ -976,6 +1020,7 @@ public Builder addPaths(@Nullable NestedSet<PathFragment> values) {
9761020
*
9771021
* <p>If values is empty, the arg isn't added.
9781022
*/
1023+
@CanIgnoreReturnValue
9791024
public Builder addPaths(
9801025
@CompileTimeConstant String arg, @Nullable Collection<PathFragment> values) {
9811026
return addCollectionInternal(arg, values);
@@ -986,12 +1031,14 @@ public Builder addPaths(
9861031
*
9871032
* <p>If values is empty, the arg isn't added.
9881033
*/
1034+
@CanIgnoreReturnValue
9891035
public Builder addPaths(
9901036
@CompileTimeConstant String arg, @Nullable NestedSet<PathFragment> values) {
9911037
return addNestedSetInternal(arg, values);
9921038
}
9931039

9941040
/** Adds the passed vector arg. See {@link VectorArg}. */
1041+
@CanIgnoreReturnValue
9951042
public Builder addPaths(VectorArg<PathFragment> vectorArg) {
9961043
return addVectorArgInternal(vectorArg);
9971044
}
@@ -1001,6 +1048,7 @@ public Builder addPaths(VectorArg<PathFragment> vectorArg) {
10011048
*
10021049
* <p>If values is empty, the arg isn't added.
10031050
*/
1051+
@CanIgnoreReturnValue
10041052
public Builder addPaths(@CompileTimeConstant String arg, VectorArg<PathFragment> vectorArg) {
10051053
return addVectorArgInternal(arg, vectorArg);
10061054
}
@@ -1011,11 +1059,13 @@ public Builder addPaths(@CompileTimeConstant String arg, VectorArg<PathFragment>
10111059
* <p>Do not use this method if the list is derived from a flattened nested set. Instead, figure
10121060
* out how to avoid flattening the set and use {@link #addExecPaths(NestedSet)}.
10131061
*/
1062+
@CanIgnoreReturnValue
10141063
public Builder addExecPaths(@Nullable Collection<Artifact> values) {
10151064
return addCollectionInternal(values);
10161065
}
10171066

10181067
/** Adds the artifacts' exec paths to the command line. */
1068+
@CanIgnoreReturnValue
10191069
public Builder addExecPaths(@Nullable NestedSet<Artifact> values) {
10201070
return addNestedSetInternal(values);
10211071
}
@@ -1028,6 +1078,7 @@ public Builder addExecPaths(@Nullable NestedSet<Artifact> values) {
10281078
*
10291079
* <p>If values is empty, the arg isn't added.
10301080
*/
1081+
@CanIgnoreReturnValue
10311082
public Builder addExecPaths(
10321083
@CompileTimeConstant String arg, @Nullable Collection<Artifact> values) {
10331084
return addCollectionInternal(arg, values);
@@ -1038,12 +1089,14 @@ public Builder addExecPaths(
10381089
*
10391090
* <p>If values is empty, the arg isn't added.
10401091
*/
1092+
@CanIgnoreReturnValue
10411093
public Builder addExecPaths(
10421094
@CompileTimeConstant String arg, @Nullable NestedSet<Artifact> values) {
10431095
return addNestedSetInternal(arg, values);
10441096
}
10451097

10461098
/** Adds the passed vector arg. See {@link VectorArg}. */
1099+
@CanIgnoreReturnValue
10471100
public Builder addExecPaths(VectorArg<Artifact> vectorArg) {
10481101
return addVectorArgInternal(vectorArg);
10491102
}
@@ -1053,6 +1106,7 @@ public Builder addExecPaths(VectorArg<Artifact> vectorArg) {
10531106
*
10541107
* <p>If values is empty, the arg isn't added.
10551108
*/
1109+
@CanIgnoreReturnValue
10561110
public Builder addExecPaths(@CompileTimeConstant String arg, VectorArg<Artifact> vectorArg) {
10571111
return addVectorArgInternal(arg, vectorArg);
10581112
}
@@ -1129,10 +1183,11 @@ private Builder addObjectInternal(@CompileTimeConstant String arg, @Nullable Obj
11291183
}
11301184

11311185
@CanIgnoreReturnValue
1132-
private Builder addPrefixedInternal(String prefix, @Nullable Object arg) {
1186+
private Builder addPrefixedInternal(
1187+
String prefix, @Nullable Object arg, @Nullable RepositoryMapping mainRepoMapping) {
11331188
Preconditions.checkNotNull(prefix);
11341189
if (arg != null) {
1135-
PrefixArg.push(arguments, prefix, arg);
1190+
PrefixArg.push(arguments, prefix, arg, mainRepoMapping);
11361191
}
11371192
return this;
11381193
}

src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,8 @@ public JavaCompileOutputs<Artifact> createOutputs(JavaOutput output) {
211211
return builder.build();
212212
}
213213

214-
public void createCompileAction(JavaCompileOutputs<Artifact> outputs) throws RuleErrorException {
214+
public void createCompileAction(JavaCompileOutputs<Artifact> outputs)
215+
throws RuleErrorException, InterruptedException {
215216
if (outputs.genClass() != null) {
216217
createGenJarAction(
217218
outputs.output(),
@@ -544,7 +545,7 @@ private Artifact turbineOutput(Artifact classJar, String newExtension) {
544545
* @param headerDeps the .jdeps output of this java compilation
545546
*/
546547
public void createHeaderCompilationAction(Artifact headerJar, Artifact headerDeps)
547-
throws RuleErrorException {
548+
throws RuleErrorException, InterruptedException {
548549

549550
JavaTargetAttributes attributes = getAttributes();
550551

@@ -689,7 +690,8 @@ public void createSourceJarAction(Artifact outputJar, @Nullable Artifact gensrcJ
689690
* @return the header jar (if requested), or ijar (if requested), or else the class jar
690691
*/
691692
public Artifact createCompileTimeJarAction(
692-
Artifact runtimeJar, JavaCompilationArtifacts.Builder builder) throws RuleErrorException {
693+
Artifact runtimeJar, JavaCompilationArtifacts.Builder builder)
694+
throws RuleErrorException, InterruptedException {
693695
Artifact jar;
694696
boolean isFullJar = false;
695697
if (shouldUseHeaderCompilation()) {

src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public JavaCompileActionBuilder(
171171
this.execGroup = execGroup;
172172
}
173173

174-
public JavaCompileAction build() throws RuleErrorException {
174+
public JavaCompileAction build() throws RuleErrorException, InterruptedException {
175175
// TODO(bazel-team): all the params should be calculated before getting here, and the various
176176
// aggregation code below should go away.
177177

@@ -271,7 +271,7 @@ private ImmutableSet<Artifact> allOutputs() {
271271
}
272272

273273
private CustomCommandLine buildParamFileContents(ImmutableList<String> javacOpts)
274-
throws RuleErrorException {
274+
throws RuleErrorException, InterruptedException {
275275

276276
CustomCommandLine.Builder result = CustomCommandLine.builder();
277277

@@ -304,7 +304,8 @@ private CustomCommandLine buildParamFileContents(ImmutableList<String> javacOpts
304304
} else {
305305
// @-prefixed strings will be assumed to be filenames and expanded by
306306
// {@link JavaLibraryBuildRequest}, so add an extra &at; to escape it.
307-
result.addPrefixedLabel("@", targetLabel);
307+
result.addPrefixedLabel(
308+
"@", targetLabel, ruleContext.getAnalysisEnvironment().getMainRepoMapping());
308309
}
309310
}
310311
result.add("--injecting_rule_kind", injectingRuleKind);

src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,8 @@ public Builder enableDirectClasspath(boolean enableDirectClasspath) {
372372
}
373373

374374
/** Builds and registers the action for a header compilation. */
375-
public void build(JavaToolchainProvider javaToolchain) throws RuleErrorException {
375+
public void build(JavaToolchainProvider javaToolchain)
376+
throws RuleErrorException, InterruptedException {
376377
checkNotNull(outputDepsProto, "outputDepsProto must not be null");
377378
checkNotNull(sourceFiles, "sourceFiles must not be null");
378379
checkNotNull(sourceJars, "sourceJars must not be null");
@@ -482,7 +483,8 @@ public void build(JavaToolchainProvider javaToolchain) throws RuleErrorException
482483
} else {
483484
// @-prefixed strings will be assumed to be params filenames and expanded,
484485
// so add an extra @ to escape it.
485-
commandLine.addPrefixedLabel("@", targetLabel);
486+
commandLine.addPrefixedLabel(
487+
"@", targetLabel, ruleContext.getAnalysisEnvironment().getMainRepoMapping());
486488
}
487489
}
488490

0 commit comments

Comments
 (0)