Skip to content

Commit

Permalink
Show display form labels in java_* buildozer fixups
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fmeum authored and Kila2 committed May 13, 2024
1 parent f4f26b7 commit cfe9d03
Show file tree
Hide file tree
Showing 9 changed files with 207 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.google.devtools.build.lib.actions.PathMapper;
import com.google.devtools.build.lib.actions.SingleStringArgFormatter;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
Expand Down Expand Up @@ -580,9 +581,16 @@ static class PrefixArg implements ArgvFragment {

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

private static void push(List<Object> arguments, String before, Object arg) {
private static void push(
List<Object> arguments,
String before,
Object arg,
@Nullable RepositoryMapping mainRepoMapping) {
arguments.add(INSTANCE);
arguments.add(before);
if (mainRepoMapping != null) {
arguments.add(mainRepoMapping);
}
arguments.add(arg);
}

Expand All @@ -594,6 +602,9 @@ public int eval(
PathMapper pathMapper) {
String before = (String) arguments.get(argi++);
Object arg = arguments.get(argi++);
if (arg instanceof RepositoryMapping mainRepoMapping) {
arg = ((Label) arguments.get(argi++)).getDisplayForm(mainRepoMapping);
}
builder.add(before + CommandLineItem.expandToCommandLine(arg));
return argi;
}
Expand All @@ -606,7 +617,11 @@ public int addToFingerprint(
Fingerprint fingerprint) {
fingerprint.addUUID(PREFIX_UUID);
fingerprint.addString((String) arguments.get(argi++));
fingerprint.addString(CommandLineItem.expandToCommandLine(arguments.get(argi++)));
Object arg = arguments.get(argi++);
if (arg instanceof RepositoryMapping mainRepoMapping) {
arg = ((Label) arguments.get(argi++)).getDisplayForm(mainRepoMapping);
}
fingerprint.addString(CommandLineItem.expandToCommandLine(arg));
return argi;
}
}
Expand Down Expand Up @@ -752,6 +767,7 @@ public static final class Builder {
*
* <p>Prefer this over its dynamic cousin, as using static strings saves memory.
*/
@CanIgnoreReturnValue
public Builder add(@CompileTimeConstant String value) {
return addObjectInternal(value);
}
Expand All @@ -761,6 +777,7 @@ public Builder add(@CompileTimeConstant String value) {
*
* <p>If the value is null, neither the arg nor the value is added.
*/
@CanIgnoreReturnValue
public Builder add(@CompileTimeConstant String arg, @Nullable String value) {
return addObjectInternal(arg, value);
}
Expand Down Expand Up @@ -797,6 +814,7 @@ public Builder addObject(@Nullable Object value) {
* <p>There are many other ways you can try to avoid calling this. In general, try to use
* constants or objects that are already on the heap elsewhere.
*/
@CanIgnoreReturnValue
public Builder addDynamicString(@Nullable String value) {
return addObjectInternal(value);
}
Expand All @@ -807,6 +825,7 @@ public Builder addDynamicString(@Nullable String value) {
* <p>Prefer this over manually calling {@link Label#getCanonicalForm}, as it avoids a copy of
* the label value.
*/
@CanIgnoreReturnValue
public Builder addLabel(@Nullable Label value) {
return addObjectInternal(value);
}
Expand All @@ -819,6 +838,7 @@ public Builder addLabel(@Nullable Label value) {
*
* <p>If the value is null, neither the arg nor the value is added.
*/
@CanIgnoreReturnValue
public Builder addLabel(@CompileTimeConstant String arg, @Nullable Label value) {
return addObjectInternal(arg, value);
}
Expand All @@ -829,6 +849,7 @@ public Builder addLabel(@CompileTimeConstant String arg, @Nullable Label value)
* <p>Prefer this over manually calling {@link PathFragment#getPathString}, as it avoids storing
* a copy of the path string.
*/
@CanIgnoreReturnValue
public Builder addPath(@Nullable PathFragment value) {
return addObjectInternal(value);
}
Expand All @@ -841,6 +862,7 @@ public Builder addPath(@Nullable PathFragment value) {
*
* <p>If the value is null, neither the arg nor the value is added.
*/
@CanIgnoreReturnValue
public Builder addPath(@CompileTimeConstant String arg, @Nullable PathFragment value) {
return addObjectInternal(arg, value);
}
Expand All @@ -851,6 +873,7 @@ public Builder addPath(@CompileTimeConstant String arg, @Nullable PathFragment v
* <p>Prefer this over manually calling {@link Artifact#getExecPath}, as it avoids storing a
* copy of the artifact path string.
*/
@CanIgnoreReturnValue
public Builder addExecPath(@Nullable Artifact value) {
return addObjectInternal(value);
}
Expand All @@ -863,16 +886,19 @@ public Builder addExecPath(@Nullable Artifact value) {
*
* <p>If the value is null, neither the arg nor the value is added.
*/
@CanIgnoreReturnValue
public Builder addExecPath(@CompileTimeConstant String arg, @Nullable Artifact value) {
return addObjectInternal(arg, value);
}

/** Adds a lazily expanded string. */
@CanIgnoreReturnValue
public Builder addLazyString(@Nullable OnDemandString value) {
return addObjectInternal(value);
}

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

/** Concatenates the passed prefix string and the string. */
@CanIgnoreReturnValue
public Builder addPrefixed(@CompileTimeConstant String prefix, @Nullable String arg) {
return addPrefixedInternal(prefix, arg);
return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
}

/** Concatenates the passed prefix string and the label using {@link Label#getCanonicalForm}. */
public Builder addPrefixedLabel(@CompileTimeConstant String prefix, @Nullable Label arg) {
return addPrefixedInternal(prefix, arg);
/**
* Concatenates the passed prefix string and the label using {@link Label#getDisplayForm}, which
* is identical to {@link Label#getCanonicalForm()} for main repo labels.
*/
@CanIgnoreReturnValue
public Builder addPrefixedLabel(
@CompileTimeConstant String prefix,
@Nullable Label arg,
RepositoryMapping mainRepoMapping) {
return addPrefixedInternal(prefix, arg, mainRepoMapping);
}

/** Concatenates the passed prefix string and the path. */
@CanIgnoreReturnValue
public Builder addPrefixedPath(@CompileTimeConstant String prefix, @Nullable PathFragment arg) {
return addPrefixedInternal(prefix, arg);
return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
}

/** Concatenates the passed prefix string and the artifact's exec path. */
@CanIgnoreReturnValue
public Builder addPrefixedExecPath(@CompileTimeConstant String prefix, @Nullable Artifact arg) {
return addPrefixedInternal(prefix, arg);
return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
}

/**
Expand All @@ -912,6 +948,7 @@ public Builder addPrefixedExecPath(@CompileTimeConstant String prefix, @Nullable
* <p>If you are converting long lists or nested sets of a different type to string lists,
* please try to use a different method that supports what you are trying to do directly.
*/
@CanIgnoreReturnValue
public Builder addAll(@Nullable Collection<String> values) {
return addCollectionInternal(values);
}
Expand All @@ -922,6 +959,7 @@ public Builder addAll(@Nullable Collection<String> values) {
* <p>If you are converting long lists or nested sets of a different type to string lists,
* please try to use a different method that supports what you are trying to do directly.
*/
@CanIgnoreReturnValue
public Builder addAll(@Nullable NestedSet<String> values) {
return addNestedSetInternal(values);
}
Expand All @@ -934,6 +972,7 @@ public Builder addAll(@Nullable NestedSet<String> values) {
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addAll(@CompileTimeConstant String arg, @Nullable Collection<String> values) {
return addCollectionInternal(arg, values);
}
Expand All @@ -943,11 +982,13 @@ public Builder addAll(@CompileTimeConstant String arg, @Nullable Collection<Stri
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addAll(@CompileTimeConstant String arg, @Nullable NestedSet<String> values) {
return addNestedSetInternal(arg, values);
}

/** Adds the passed vector arg. See {@link VectorArg}. */
@CanIgnoreReturnValue
public Builder addAll(VectorArg<String> vectorArg) {
return addVectorArgInternal(vectorArg);
}
Expand All @@ -957,16 +998,19 @@ public Builder addAll(VectorArg<String> vectorArg) {
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addAll(@CompileTimeConstant String arg, VectorArg<String> vectorArg) {
return addVectorArgInternal(arg, vectorArg);
}

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

/** Adds the passed paths to the command line. */
@CanIgnoreReturnValue
public Builder addPaths(@Nullable NestedSet<PathFragment> values) {
return addNestedSetInternal(values);
}
Expand All @@ -976,6 +1020,7 @@ public Builder addPaths(@Nullable NestedSet<PathFragment> values) {
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addPaths(
@CompileTimeConstant String arg, @Nullable Collection<PathFragment> values) {
return addCollectionInternal(arg, values);
Expand All @@ -986,12 +1031,14 @@ public Builder addPaths(
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addPaths(
@CompileTimeConstant String arg, @Nullable NestedSet<PathFragment> values) {
return addNestedSetInternal(arg, values);
}

/** Adds the passed vector arg. See {@link VectorArg}. */
@CanIgnoreReturnValue
public Builder addPaths(VectorArg<PathFragment> vectorArg) {
return addVectorArgInternal(vectorArg);
}
Expand All @@ -1001,6 +1048,7 @@ public Builder addPaths(VectorArg<PathFragment> vectorArg) {
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addPaths(@CompileTimeConstant String arg, VectorArg<PathFragment> vectorArg) {
return addVectorArgInternal(arg, vectorArg);
}
Expand All @@ -1011,11 +1059,13 @@ public Builder addPaths(@CompileTimeConstant String arg, VectorArg<PathFragment>
* <p>Do not use this method if the list is derived from a flattened nested set. Instead, figure
* out how to avoid flattening the set and use {@link #addExecPaths(NestedSet)}.
*/
@CanIgnoreReturnValue
public Builder addExecPaths(@Nullable Collection<Artifact> values) {
return addCollectionInternal(values);
}

/** Adds the artifacts' exec paths to the command line. */
@CanIgnoreReturnValue
public Builder addExecPaths(@Nullable NestedSet<Artifact> values) {
return addNestedSetInternal(values);
}
Expand All @@ -1028,6 +1078,7 @@ public Builder addExecPaths(@Nullable NestedSet<Artifact> values) {
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addExecPaths(
@CompileTimeConstant String arg, @Nullable Collection<Artifact> values) {
return addCollectionInternal(arg, values);
Expand All @@ -1038,12 +1089,14 @@ public Builder addExecPaths(
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addExecPaths(
@CompileTimeConstant String arg, @Nullable NestedSet<Artifact> values) {
return addNestedSetInternal(arg, values);
}

/** Adds the passed vector arg. See {@link VectorArg}. */
@CanIgnoreReturnValue
public Builder addExecPaths(VectorArg<Artifact> vectorArg) {
return addVectorArgInternal(vectorArg);
}
Expand All @@ -1053,6 +1106,7 @@ public Builder addExecPaths(VectorArg<Artifact> vectorArg) {
*
* <p>If values is empty, the arg isn't added.
*/
@CanIgnoreReturnValue
public Builder addExecPaths(@CompileTimeConstant String arg, VectorArg<Artifact> vectorArg) {
return addVectorArgInternal(arg, vectorArg);
}
Expand Down Expand Up @@ -1129,10 +1183,11 @@ private Builder addObjectInternal(@CompileTimeConstant String arg, @Nullable Obj
}

@CanIgnoreReturnValue
private Builder addPrefixedInternal(String prefix, @Nullable Object arg) {
private Builder addPrefixedInternal(
String prefix, @Nullable Object arg, @Nullable RepositoryMapping mainRepoMapping) {
Preconditions.checkNotNull(prefix);
if (arg != null) {
PrefixArg.push(arguments, prefix, arg);
PrefixArg.push(arguments, prefix, arg, mainRepoMapping);
}
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ public JavaCompileOutputs<Artifact> createOutputs(JavaOutput output) {
return builder.build();
}

public void createCompileAction(JavaCompileOutputs<Artifact> outputs) throws RuleErrorException {
public void createCompileAction(JavaCompileOutputs<Artifact> outputs)
throws RuleErrorException, InterruptedException {
if (outputs.genClass() != null) {
createGenJarAction(
outputs.output(),
Expand Down Expand Up @@ -542,7 +543,7 @@ private Artifact turbineOutput(Artifact classJar, String newExtension) {
* @param headerDeps the .jdeps output of this java compilation
*/
public void createHeaderCompilationAction(Artifact headerJar, Artifact headerDeps)
throws RuleErrorException {
throws RuleErrorException, InterruptedException {

JavaTargetAttributes attributes = getAttributes();

Expand Down Expand Up @@ -687,7 +688,8 @@ public void createSourceJarAction(Artifact outputJar, @Nullable Artifact gensrcJ
* @return the header jar (if requested), or ijar (if requested), or else the class jar
*/
public Artifact createCompileTimeJarAction(
Artifact runtimeJar, JavaCompilationArtifacts.Builder builder) throws RuleErrorException {
Artifact runtimeJar, JavaCompilationArtifacts.Builder builder)
throws RuleErrorException, InterruptedException {
Artifact jar;
boolean isFullJar = false;
if (shouldUseHeaderCompilation()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public JavaCompileActionBuilder(
this.execGroup = execGroup;
}

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

Expand Down Expand Up @@ -271,7 +271,7 @@ private ImmutableSet<Artifact> allOutputs() {
}

private CustomCommandLine buildParamFileContents(ImmutableList<String> javacOpts)
throws RuleErrorException {
throws RuleErrorException, InterruptedException {

CustomCommandLine.Builder result = CustomCommandLine.builder();

Expand Down Expand Up @@ -304,7 +304,8 @@ private CustomCommandLine buildParamFileContents(ImmutableList<String> javacOpts
} else {
// @-prefixed strings will be assumed to be filenames and expanded by
// {@link JavaLibraryBuildRequest}, so add an extra &at; to escape it.
result.addPrefixedLabel("@", targetLabel);
result.addPrefixedLabel(
"@", targetLabel, ruleContext.getAnalysisEnvironment().getMainRepoMapping());
}
}
result.add("--injecting_rule_kind", injectingRuleKind);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,8 @@ public Builder enableDirectClasspath(boolean enableDirectClasspath) {
}

/** Builds and registers the action for a header compilation. */
public void build(JavaToolchainProvider javaToolchain) throws RuleErrorException {
public void build(JavaToolchainProvider javaToolchain)
throws RuleErrorException, InterruptedException {
checkNotNull(outputDepsProto, "outputDepsProto must not be null");
checkNotNull(sourceFiles, "sourceFiles must not be null");
checkNotNull(sourceJars, "sourceJars must not be null");
Expand Down Expand Up @@ -478,7 +479,8 @@ public void build(JavaToolchainProvider javaToolchain) throws RuleErrorException
} else {
// @-prefixed strings will be assumed to be params filenames and expanded,
// so add an extra @ to escape it.
commandLine.addPrefixedLabel("@", targetLabel);
commandLine.addPrefixedLabel(
"@", targetLabel, ruleContext.getAnalysisEnvironment().getMainRepoMapping());
}
}

Expand Down
Loading

0 comments on commit cfe9d03

Please sign in to comment.