From 58a2922c5e03caeccd0b317344e344041a59996f Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 1 Apr 2024 12:49:22 -0700 Subject: [PATCH] Stringify `Label`s in display form in `Args` `Label`s added to `Args` are now formatted in display form, which means that they will be repo-relative if referring to the main repo and use an apparent repository name if possible. Previously, labels were formatted in repo-relative form if referring to the main repo and using canonical repository names exclusively for external repos. At the same time, `Args` docs explicitly stated that the exact stringification of any type other than `File` is unspecified. The previous behavior was problematic since it neither always used canonical labels (which could be useful for writing tests that check these labels against an allowlist) nor provided labels that could be included in BUILD files (canonical names are explicitly unstable). Furthermore, whether a label resulted in a string prefixed with a single or two `@`s already dependended on a user choice, namely the value of `--enable_bzlmod`. Thus, this change is not considered to be breaking. It makes it so that existing rulesets relying on default stringification to return a BUILD-file compatible label in WORKSPACE continue to do so with Bzlmod with no code changes. This change aims to be as memory efficient as possible: * Single labels or sequences of labels that reference targets in the main repo incur no memory overhead. * Labels referring to external repos as well as `NestedSet`s of labels result in an additional Skyframe dependency for each target using the command line as well as one additional reference (4 bytes) stored in the command line's `arguments`. Work towards #20486 Closes #21702. PiperOrigin-RevId: 620925978 Change-Id: I54aa807c41bf783aee223482d2309f5cee2726b5 --- .../lib/analysis/AnalysisEnvironment.java | 6 + .../google/devtools/build/lib/analysis/BUILD | 4 + .../analysis/CachingAnalysisEnvironment.java | 16 ++ .../build/lib/analysis/starlark/Args.java | 59 +++++-- .../starlark/StarlarkActionFactory.java | 28 ++- .../starlark/StarlarkCustomCommandLine.java | 90 ++++++++-- .../build/lib/cmdline/RepositoryMapping.java | 4 + .../starlarkbuildapi/CommandLineArgsApi.java | 8 + .../StarlarkActionFactoryApi.java | 7 +- .../analysis/starlark/ArgsParamFileTest.java | 6 +- .../analysis/starlark/FlagPerLineTest.java | 6 +- .../StarlarkCustomCommandLineTest.java | 3 +- .../lib/analysis/util/AnalysisTestUtil.java | 11 ++ .../lib/analysis/util/BuildViewTestCase.java | 6 + .../google/devtools/build/lib/starlark/BUILD | 1 + ...arlarkRuleImplementationFunctionsTest.java | 162 +++++++++++++++++- src/test/shell/bazel/BUILD | 13 +- src/test/shell/bazel/bazel_java_test.sh | 40 +++++ 18 files changed, 415 insertions(+), 55 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java index 12bab8df8168a8..7fba9c3c74720c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.actions.MiddlemanFactory; import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoKey; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; @@ -176,4 +177,9 @@ ImmutableList getBuildInfo( ImmutableSet getTreeArtifactsConflictingWithFiles(); ActionKeyContext getActionKeyContext(); + + /** + * Returns and registers a Skyframe dependency on the {@link RepositoryMapping} of the main repo. + */ + RepositoryMapping getMainRepoMapping() throws InterruptedException; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 837c287f8d3cfa..3a6be604719a0e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -414,6 +414,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data", "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key", "//src/main/java/com/google/devtools/build/lib/skyframe:configured_value_creation_exception", + "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", "//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value", "//src/main/java/com/google/devtools/build/lib/skyframe:workspace_status_value", "//src/main/java/com/google/devtools/build/lib/skyframe/config", @@ -424,6 +425,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test", + "//src/main/java/com/google/devtools/build/lib/supplier", "//src/main/java/com/google/devtools/build/lib/unsafe:string", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", @@ -2420,9 +2422,11 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi", + "//src/main/java/com/google/devtools/build/lib/supplier", "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java index d9c9d5d171ffbb..6e50ea6fb989f8 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java @@ -30,10 +30,13 @@ import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoCollection; import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoKey; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue; +import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsValue; import com.google.devtools.build.lib.skyframe.WorkspaceStatusValue; import com.google.devtools.build.lib.util.Pair; @@ -385,6 +388,19 @@ public ImmutableList getBuildInfo( return stamp ? collection.getStampedBuildInfo() : collection.getRedactedBuildInfo(); } + @Override + public RepositoryMapping getMainRepoMapping() throws InterruptedException { + var mainRepoMapping = + (RepositoryMappingValue) + skyframeEnv.getValue(RepositoryMappingValue.key(RepositoryName.MAIN)); + if (mainRepoMapping == null) { + // This isn't expected to happen since the main repository mapping is computed before the + // analysis phase. + throw new MissingDepException("Restart due to missing main repository mapping"); + } + return mainRepoMapping.getRepositoryMapping(); + } + @Override public ActionLookupKey getOwner() { return owner; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java index a063d8381a6ab9..f85578805dd14b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java @@ -24,10 +24,13 @@ import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.SingleStringArgFormatter; import com.google.devtools.build.lib.analysis.starlark.StarlarkCustomCommandLine.ScalarArg; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.collect.nestedset.Depset; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.starlarkbuildapi.CommandLineArgsApi; +import com.google.devtools.build.lib.supplier.InterruptibleSupplier; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -71,7 +74,8 @@ public void repr(Printer printer) { @Override public void debugPrint(Printer printer, StarlarkSemantics semantics) { try { - printer.append(Joiner.on(" ").join(build().arguments())); + printer.append( + Joiner.on(" ").join(build(/* mainRepoMappingSupplier= */ () -> null).arguments())); } catch (CommandLineExpansionException e) { printer.append("Cannot expand command line: " + e.getMessage()); } catch (InterruptedException e) { @@ -103,7 +107,8 @@ public void debugPrint(Printer printer, StarlarkSemantics semantics) { public abstract ImmutableSet getDirectoryArtifacts(); /** Returns the command line built by this {@link Args} object. */ - public abstract CommandLine build(); + public abstract CommandLine build( + InterruptibleSupplier mainRepoMappingSupplier) throws InterruptedException; /** * Returns a frozen {@link Args} representation corresponding to an already-registered action. @@ -158,7 +163,7 @@ public ImmutableSet getDirectoryArtifacts() { } @Override - public CommandLine build() { + public CommandLine build(InterruptibleSupplier mainRepoMappingSupplier) { return commandLine; } @@ -260,6 +265,12 @@ private static class MutableArgs extends Args implements StarlarkValue, Mutabili */ private boolean flagPerLine = false; + /** + * True if the command line needs to stringify any {@link Label}s without an explicit 'map_each' + * function. + */ + private boolean mayStringifyExternalLabel = false; + // May be set explicitly once -- if unset defaults to ParameterFileType.SHELL_QUOTED. private ParameterFileType parameterFileType = null; private String flagFormatString; @@ -308,6 +319,9 @@ public CommandLineArgsApi addArgument( "Args.add() doesn't accept vectorized arguments. Please use Args.add_all() or" + " Args.add_joined() instead."); } + if (value instanceof Label label && !label.getRepository().isMain()) { + mayStringifyExternalLabel = true; + } addScalarArg(value, format != Starlark.NONE ? (String) format : null); return this; } @@ -437,8 +451,12 @@ private void addVectorArg( validateFormatString("format_each", formatEach); validateFormatString("format_joined", formatJoined); StarlarkCustomCommandLine.VectorArg.Builder vectorArg; - if (value instanceof Depset) { - Depset starlarkNestedSet = (Depset) value; + if (value instanceof Depset starlarkNestedSet) { + if (mapEach == null && Label.class.equals(starlarkNestedSet.getElementClass())) { + // We don't want to eagerly check whether all labels reference targets in the main repo, + // so just assume they might not. Nested sets of labels should be rare. + mayStringifyExternalLabel = true; + } NestedSet nestedSet = starlarkNestedSet.getSet(); if (nestedSet.isEmpty() && omitIfEmpty) { return; @@ -452,8 +470,16 @@ private void addVectorArg( if (starlarkList.isEmpty() && omitIfEmpty) { return; } - if (expandDirectories) { - scanForDirectories(starlarkList); + for (Object object : starlarkList) { + if (expandDirectories && isDirectory(object)) { + directoryArtifacts.add((Artifact) object); + } + // Labels referencing targets in the main repo are stringified as //pkg:name and thus + // don't require a RepositoryMapping. If a map_each function is provided, default + // stringification via Label#toString() is not used. + if (mapEach == null && object instanceof Label label && !label.getRepository().isMain()) { + mayStringifyExternalLabel = true; + } } vectorArg = new StarlarkCustomCommandLine.VectorArg.Builder(starlarkList); } @@ -575,8 +601,10 @@ private MutableArgs(@Nullable Mutability mutability, StarlarkSemantics starlarkS } @Override - public CommandLine build() { - return commandLine.build(flagPerLine); + public CommandLine build(InterruptibleSupplier mainRepoMappingSupplier) + throws InterruptedException { + return commandLine.build( + flagPerLine, mayStringifyExternalLabel ? mainRepoMappingSupplier.get() : null); } @Override @@ -587,18 +615,15 @@ public Mutability mutability() { @Override public ImmutableSet getDirectoryArtifacts() { for (NestedSet collection : potentialDirectoryArtifacts) { - scanForDirectories(collection.toList()); + for (Object object : collection.toList()) { + if (isDirectory(object)) { + directoryArtifacts.add((Artifact) object); + } + } } potentialDirectoryArtifacts.clear(); return ImmutableSet.copyOf(directoryArtifacts); } - private void scanForDirectories(Iterable objects) { - for (Object object : objects) { - if (isDirectory(object)) { - directoryArtifacts.add((Artifact) object); - } - } - } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java index d00394c5a815f4..192dc70c83fedf 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java @@ -55,6 +55,7 @@ import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.collect.nestedset.Depset; import com.google.devtools.build.lib.collect.nestedset.Depset.TypeException; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -72,6 +73,7 @@ import com.google.devtools.build.lib.starlarkbuildapi.FileApi; import com.google.devtools.build.lib.starlarkbuildapi.StarlarkActionFactoryApi; import com.google.devtools.build.lib.starlarkbuildapi.TemplateDictApi; +import com.google.devtools.build.lib.supplier.InterruptibleSupplier; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.GeneratedMessage; @@ -362,7 +364,8 @@ public void symlink( } @Override - public void write(FileApi output, Object content, Boolean isExecutable) throws EvalException { + public void write(FileApi output, Object content, Boolean isExecutable) + throws EvalException, InterruptedException { context.checkMutable("actions.write"); RuleContext ruleContext = getRuleContext(); @@ -377,7 +380,7 @@ public void write(FileApi output, Object content, Boolean isExecutable) throws E ruleContext.getActionOwner(), NestedSetBuilder.wrap(Order.STABLE_ORDER, args.getDirectoryArtifacts()), (Artifact) output, - args.build(), + args.build(getMainRepoMappingSupplier()), args.getParameterFileType()); } else { throw new AssertionError("Unexpected type: " + content.getClass().getSimpleName()); @@ -403,7 +406,7 @@ public void run( Object shadowedActionUnchecked, Object resourceSetUnchecked, Object toolchainUnchecked) - throws EvalException { + throws EvalException, InterruptedException { context.checkMutable("actions.run"); execGroupUnchecked = context.maybeOverrideExecGroup(execGroupUnchecked); toolchainUnchecked = context.maybeOverrideToolchain(toolchainUnchecked); @@ -412,7 +415,7 @@ public void run( boolean useAutoExecGroups = ruleContext.useAutoExecGroups(); StarlarkAction.Builder builder = new StarlarkAction.Builder(); - buildCommandLine(builder, arguments); + buildCommandLine(builder, arguments, getMainRepoMappingSupplier()); if (executableUnchecked instanceof Artifact) { Artifact executable = (Artifact) executableUnchecked; FilesToRunProvider provider = context.getExecutableRunfiles(executable, "executable"); @@ -599,7 +602,7 @@ public void runShell( Object shadowedActionUnchecked, Object resourceSetUnchecked, Object toolchainUnchecked) - throws EvalException { + throws EvalException, InterruptedException { context.checkMutable("actions.run_shell"); execGroupUnchecked = context.maybeOverrideExecGroup(execGroupUnchecked); toolchainUnchecked = context.maybeOverrideToolchain(toolchainUnchecked); @@ -607,7 +610,7 @@ public void runShell( RuleContext ruleContext = getRuleContext(); StarlarkAction.Builder builder = new StarlarkAction.Builder(); - buildCommandLine(builder, arguments); + buildCommandLine(builder, arguments, getMainRepoMappingSupplier()); // When we use a shell command, add an empty argument before other arguments. // e.g. bash -c "cmd" '' 'arg1' 'arg2' @@ -671,8 +674,11 @@ public void runShell( builder); } - private static void buildCommandLine(SpawnAction.Builder builder, Sequence argumentsList) - throws EvalException { + private static void buildCommandLine( + SpawnAction.Builder builder, + Sequence argumentsList, + InterruptibleSupplier repoMappingSupplier) + throws EvalException, InterruptedException { List stringArgs = new ArrayList<>(); for (Object value : argumentsList) { if (value instanceof String) { @@ -684,7 +690,7 @@ private static void buildCommandLine(SpawnAction.Builder builder, Sequence ar } Args args = (Args) value; ParamFileInfo paramFileInfo = args.getParamFileInfo(); - builder.addCommandLine(args.build(), paramFileInfo); + builder.addCommandLine(args.build(repoMappingSupplier), paramFileInfo); } else { throw Starlark.errorf( "expected list of strings or ctx.actions.args() for arguments instead of %s", @@ -1094,6 +1100,10 @@ public void repr(Printer printer) { context.repr(printer); } + private InterruptibleSupplier getMainRepoMappingSupplier() { + return context.getRuleContext().getAnalysisEnvironment()::getMainRepoMapping; + } + /** The analysis context for {@code Starlark} actions */ // For now, this contains methods necessary for SubruleContext to begin using // StarlarkActionFactory without any invasive changes to the latter. It will be improved once the diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java index 2ae61bdb1293b5..1da07e10e35d07 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLine.java @@ -18,6 +18,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Interner; +import com.google.common.collect.Iterables; +import com.google.common.collect.Iterators; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.Artifact; @@ -33,6 +36,8 @@ 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.cmdline.RepositoryName; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization; @@ -181,7 +186,8 @@ private int eval( int argi, List builder, @Nullable ArtifactExpander artifactExpander, - PathMapper pathMapper) + PathMapper pathMapper, + @Nullable RepositoryMapping mainRepoMapping) throws CommandLineExpansionException, InterruptedException { final Location location = ((features & HAS_LOCATION) != 0) ? (Location) arguments.get(argi++) : null; @@ -215,7 +221,7 @@ private int eval( int count = expandedValues.size(); stringValues = new ArrayList<>(expandedValues.size()); for (int i = 0; i < count; ++i) { - stringValues.add(expandToCommandLine(expandedValues.get(i), pathMapper)); + stringValues.add(expandToCommandLine(expandedValues.get(i), pathMapper, mainRepoMapping)); } } // It's safe to uniquify at this stage, any transformations after this @@ -614,9 +620,14 @@ private static void push(List arguments, Builder arg) { } private int eval( - List arguments, int argi, List builder, PathMapper pathMapper) { + List arguments, + int argi, + List builder, + PathMapper pathMapper, + @Nullable RepositoryMapping mainRepoMapping) { Object object = arguments.get(argi++); - String stringValue = StarlarkCustomCommandLine.expandToCommandLine(object, pathMapper); + String stringValue = + StarlarkCustomCommandLine.expandToCommandLine(object, pathMapper, mainRepoMapping); if (hasFormat) { String formatStr = (String) arguments.get(argi++); stringValue = SingleStringArgFormatter.format(formatStr, stringValue); @@ -704,8 +715,15 @@ Builder add(ScalarArg.Builder scalarArg) { return this; } - StarlarkCustomCommandLine build(boolean flagPerLine) { - Object[] args = arguments.toArray(); + StarlarkCustomCommandLine build( + boolean flagPerLine, @Nullable RepositoryMapping mainRepoMapping) { + Object[] args; + if (mainRepoMapping != null) { + args = arguments.toArray(new Object[arguments.size() + 1]); + args[arguments.size()] = mainRepoMapping; + } else { + args = arguments.toArray(); + } return flagPerLine ? new StarlarkCustomCommandLineWithIndexes(args, argStartIndexes.build()) : new StarlarkCustomCommandLine(args); @@ -739,21 +757,40 @@ public Iterable arguments( List result = new ArrayList<>(); List arguments = rawArgsAsList(); - for (int argi = 0; argi < arguments.size(); ) { + RepositoryMapping mainRepoMapping; + int size; + // Added in #build() if any labels in the command line require this to be formatted with an + // apparent repository name. + if (arguments.getLast() instanceof RepositoryMapping) { + mainRepoMapping = (RepositoryMapping) arguments.getLast(); + size = arguments.size() - 1; + } else { + mainRepoMapping = null; + size = arguments.size(); + } + + for (int argi = 0; argi < size; ) { Object arg = arguments.get(argi++); if (arg instanceof VectorArg) { - argi = ((VectorArg) arg).eval(arguments, argi, result, artifactExpander, pathMapper); + argi = + ((VectorArg) arg) + .eval(arguments, argi, result, artifactExpander, pathMapper, mainRepoMapping); } else if (arg instanceof ScalarArg) { - argi = ((ScalarArg) arg).eval(arguments, argi, result, pathMapper); + argi = ((ScalarArg) arg).eval(arguments, argi, result, pathMapper, mainRepoMapping); } else { - result.add(expandToCommandLine(arg, pathMapper)); + result.add(expandToCommandLine(arg, pathMapper, mainRepoMapping)); } } return ImmutableList.copyOf(pathMapper.mapCustomStarlarkArgs(result)); } - private static String expandToCommandLine(Object object, PathMapper pathMapper) { + private static String expandToCommandLine( + Object object, PathMapper pathMapper, @Nullable RepositoryMapping mainRepoMapping) { + if (mainRepoMapping != null && object instanceof Label label) { + return label.getDisplayForm(mainRepoMapping); + } + // It'd be nice to build this into DerivedArtifact's CommandLine interface so we don't have // to explicitly check if an object is a DerivedArtifact. Unfortunately that would require // a lot more dependencies on the Java library DerivedArtifact is built into. @@ -792,7 +829,17 @@ public Iterable arguments( Iterator startIndexIterator = argStartIndexes.iterator(); int nextStartIndex = startIndexIterator.hasNext() ? startIndexIterator.next() : -1; - for (int argi = 0; argi < arguments.size(); ) { + RepositoryMapping mainRepoMapping; + int size; + if (arguments.getLast() instanceof RepositoryMapping) { + mainRepoMapping = (RepositoryMapping) arguments.getLast(); + size = arguments.size() - 1; + } else { + mainRepoMapping = null; + size = arguments.size(); + } + + for (int argi = 0; argi < size; ) { // If we're grouping arguments, record the actual beginning of each group if (argi == nextStartIndex) { @@ -802,11 +849,14 @@ public Iterable arguments( Object arg = arguments.get(argi++); if (arg instanceof VectorArg) { - argi = ((VectorArg) arg).eval(arguments, argi, result, artifactExpander, pathMapper); + argi = + ((VectorArg) arg) + .eval(arguments, argi, result, artifactExpander, pathMapper, mainRepoMapping); } else if (arg instanceof ScalarArg) { - argi = ((ScalarArg) arg).eval(arguments, argi, result, pathMapper); + argi = ((ScalarArg) arg).eval(arguments, argi, result, pathMapper, mainRepoMapping); } else { - result.add(StarlarkCustomCommandLine.expandToCommandLine(arg, pathMapper)); + result.add( + StarlarkCustomCommandLine.expandToCommandLine(arg, pathMapper, mainRepoMapping)); } } @@ -842,7 +892,15 @@ public void addToFingerprint( Fingerprint fingerprint) throws CommandLineExpansionException, InterruptedException { List arguments = rawArgsAsList(); - for (int argi = 0; argi < arguments.size(); ) { + int size; + if (arguments.getLast() instanceof RepositoryMapping mainRepoMapping) { + fingerprint.addStringMap( + Maps.transformValues(mainRepoMapping.entries(), RepositoryName::getName)); + size = arguments.size() - 1; + } else { + size = arguments.size(); + } + for (int argi = 0; argi < size; ) { Object arg = arguments.get(argi++); if (arg instanceof VectorArg) { argi = diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java index 6bffe81b4b1efd..478f0d24f4c8a9 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryMapping.java @@ -31,6 +31,10 @@ *

For repositories from the WORKSPACE file, if the requested repo doesn't exist in the mapping, * we fallback to the requested name. For repositories from Bzlmod, we return null to let the caller * decide what to do. + * + *

This class must not implement {@link net.starlark.java.eval.StarlarkValue} since instances of + * this class are used as markers by {@link + * com.google.devtools.build.lib.analysis.starlark.StarlarkCustomCommandLine}. */ public class RepositoryMapping { /* A repo mapping that always falls back to using the apparent name as the canonical name. */ diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/CommandLineArgsApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/CommandLineArgsApi.java index e5272bd39eeb04..b46b8e89ae3cfa 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/CommandLineArgsApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/CommandLineArgsApi.java @@ -66,6 +66,14 @@ + "

  • Values that are already strings are left as-is." // + "
  • File objects are turned into" + " their File.path values." // + + "
  • Label objects are turned into" + + " a string representation that resolves back to the same object when resolved in the" + + " context of the main repository. If possible, the string representation uses the" + + " apparent name of a repository in favor of the repository's canonical name, which" + + " makes this representation suited for use in BUILD files. While the exact form of" + + " the representation is not guaranteed, typical examples are" + + " //foo:bar, @repo//foo:bar and" + + " @@canonical_name~//foo:bar.bzl." + "
  • All other types are turned into strings in an unspecified manner. For " + " this reason, you should avoid passing values that are not of string or " + " File type to add(), and if you pass them to " diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java index f697aa45630654..60e53fc27c43ea 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java @@ -302,7 +302,8 @@ void symlink( doc = "Whether the output file should be executable.", named = true) }) - void write(FileApi output, Object content, Boolean isExecutable) throws EvalException; + void write(FileApi output, Object content, Boolean isExecutable) + throws EvalException, InterruptedException; @StarlarkMethod( name = "run", @@ -554,7 +555,7 @@ void run( Object shadowedAction, Object resourceSetUnchecked, Object toolchainUnchecked) - throws EvalException; + throws EvalException, InterruptedException; @StarlarkMethod( name = "run_shell", @@ -807,7 +808,7 @@ void runShell( Object shadowedAction, Object resourceSetUnchecked, Object toolchainUnchecked) - throws EvalException; + throws EvalException, InterruptedException; @StarlarkMethod( name = "expand_template", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/starlark/ArgsParamFileTest.java b/src/test/java/com/google/devtools/build/lib/analysis/starlark/ArgsParamFileTest.java index 258c052f8682e3..ed374fbdc52099 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/starlark/ArgsParamFileTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/starlark/ArgsParamFileTest.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.util.ShellEscaper; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -143,7 +144,10 @@ private static ImmutableList toParamFile(Args args) throws Exception { byte[] bytes; try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { ParameterFile.writeParameterFile( - outputStream, args.build().arguments(), args.getParameterFileType(), UTF_8); + outputStream, + args.build(() -> RepositoryMapping.ALWAYS_FALLBACK).arguments(), + args.getParameterFileType(), + UTF_8); bytes = outputStream.toByteArray(); } try (ByteArrayInputStream inputStream = new ByteArrayInputStream(bytes); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/starlark/FlagPerLineTest.java b/src/test/java/com/google/devtools/build/lib/analysis/starlark/FlagPerLineTest.java index eb5bc915bbcdc5..686f4e7ea51c0f 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/starlark/FlagPerLineTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/starlark/FlagPerLineTest.java @@ -21,6 +21,7 @@ import com.google.common.io.CharStreams; import com.google.devtools.build.lib.actions.ParameterFile; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.InputStreamReader; @@ -166,7 +167,10 @@ private static ImmutableList toParamFile(Args args) throws Exception { byte[] bytes; try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { ParameterFile.writeParameterFile( - outputStream, args.build().arguments(), args.getParameterFileType(), UTF_8); + outputStream, + args.build(() -> RepositoryMapping.ALWAYS_FALLBACK).arguments(), + args.getParameterFileType(), + UTF_8); bytes = outputStream.toByteArray(); } try (ByteArrayInputStream inputStream = new ByteArrayInputStream(bytes); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java index e53c7bc4e5b0e2..f515027753c836 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkCustomCommandLineTest.java @@ -35,6 +35,7 @@ import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.starlark.StarlarkCustomCommandLine.VectorArg; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.Path; @@ -218,7 +219,7 @@ private static StarlarkCustomCommandLine createCustomCommandLine( VectorArg.Builder vectorArgBuilder) { return new StarlarkCustomCommandLine.Builder(StarlarkSemantics.DEFAULT) .add(vectorArgBuilder) - .build(/*flagPerLine=*/ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); } private static ArtifactExpander createArtifactExpander( diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java index a892023d0a4e31..3d96e98208a175 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java @@ -50,6 +50,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptionsView; import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -233,6 +234,11 @@ public ImmutableSet getTreeArtifactsConflictingWithFiles() { public ActionKeyContext getActionKeyContext() { return original.getActionKeyContext(); } + + @Override + public RepositoryMapping getMainRepoMapping() throws InterruptedException { + return original.getMainRepoMapping(); + } } /** A dummy WorkspaceStatusAction. */ @@ -469,6 +475,11 @@ public ImmutableSet getTreeArtifactsConflictingWithFiles() { public ActionKeyContext getActionKeyContext() { return null; } + + @Override + public RepositoryMapping getMainRepoMapping() { + return RepositoryMapping.ALWAYS_FALLBACK; + } } /** Matches the output path prefix contributed by a C++ configuration fragment. */ diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index f3d92c2f087341..0c7fec28d120c4 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -114,6 +114,7 @@ import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -2326,6 +2327,11 @@ public ImmutableSet getTreeArtifactsConflictingWithFiles() { public ActionKeyContext getActionKeyContext() { return actionKeyContext; } + + @Override + public RepositoryMapping getMainRepoMapping() { + throw new UnsupportedOperationException(); + } } protected Iterable baselineCoverageArtifactBasenames(ConfiguredTarget target) diff --git a/src/test/java/com/google/devtools/build/lib/starlark/BUILD b/src/test/java/com/google/devtools/build/lib/starlark/BUILD index 7a887cabfdd05f..ffa40d40d9c13a 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/BUILD +++ b/src/test/java/com/google/devtools/build/lib/starlark/BUILD @@ -83,6 +83,7 @@ java_test( "//src/test/java/com/google/devtools/build/lib/actions/util", "//src/test/java/com/google/devtools/build/lib/analysis/testing", "//src/test/java/com/google/devtools/build/lib/analysis/util", + "//src/test/java/com/google/devtools/build/lib/bazel/bzlmod:util", "//src/test/java/com/google/devtools/build/lib/exec/util", "//src/test/java/com/google/devtools/build/lib/starlark/util", "//src/test/java/com/google/devtools/build/lib/testutil", diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java index 647342aedf3657..32063ff0284167 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java @@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.devtools.build.lib.bazel.bzlmod.BzlmodTestUtil.createModuleKey; import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; @@ -53,6 +54,7 @@ import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.collect.nestedset.Depset; import com.google.devtools.build.lib.events.Event; @@ -2574,6 +2576,156 @@ public void testLazyArgsObjectImmutability() throws Exception { assertThat(e).hasMessageThat().contains("trying to mutate a frozen Args value"); } + @Test + public void testArgsMainRepoLabel() throws Exception { + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + ev.exec( + "actions = ruleContext.actions", + "a = []", + "a.append(actions.args().add(Label('//bar')))", + "a.append(actions.args().add('-flag', Label('//bar')))", + "a.append(actions.args().add('-flag', Label('//bar'), format = '_%s_'))", + "a.append(actions.args().add_all(['foo', Label('//bar')]))", + "a.append(actions.args().add_all(depset([Label('//foo'), Label('//bar')])))", + "ruleContext.actions.run(", + " inputs = depset(ruleContext.files.srcs),", + " outputs = ruleContext.files.srcs,", + " arguments = a,", + " executable = ruleContext.files.tools[0],", + " toolchain = None", + ")"); + SpawnAction action = + (SpawnAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + assertThat(action.getArguments()) + .containsExactly( + "foo/t.exe", + "//bar:bar", + "-flag", + "//bar:bar", + "-flag", + "_//bar:bar_", + "foo", + "//bar:bar", + "//foo:foo", + "//bar:bar") + .inOrder(); + } + + @Test + public void testArgsCanonicalRepoLabel() throws Exception { + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + ev.exec( + "actions = ruleContext.actions", + "a = []", + "a.append(actions.args().add(Label('@@repo~//:foo')))", + "a.append(actions.args().add('-flag', Label('@@repo~//:foo')))", + "a.append(actions.args().add('-flag', Label('@@repo~//:foo'), format = '_%s_'))", + "a.append(actions.args().add_all(['foo', Label('@@repo~//:foo')]))", + "a.append(actions.args().add_all(depset([Label('@@other_repo~//:foo')," + + " Label('@@repo~//:foo')])))", + "ruleContext.actions.run(", + " inputs = depset(ruleContext.files.srcs),", + " outputs = ruleContext.files.srcs,", + " arguments = a,", + " executable = ruleContext.files.tools[0],", + " toolchain = None", + ")"); + SpawnAction action = + (SpawnAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + assertThat(action.getArguments()) + .containsExactly( + "foo/t.exe", + "@@repo~//:foo", + "-flag", + "@@repo~//:foo", + "-flag", + "_@@repo~//:foo_", + "foo", + "@@repo~//:foo", + "@@other_repo~//:foo", + "@@repo~//:foo") + .inOrder(); + } + + @Test + public void testArgsApparentRepoLabel() throws Exception { + scratch.overwriteFile("MODULE.bazel", "bazel_dep(name = 'foo', version = '1.0')"); + registry.addModule(createModuleKey("foo", "1.0"), "module(name='foo', version='1.0')"); + invalidatePackages(); + + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + ev.exec( + "actions = ruleContext.actions", + "a = []", + "a.append(actions.args().add(Label('@@foo~//:foo')))", + "a.append(actions.args().add('-flag', Label('@@foo~//:foo')))", + "a.append(actions.args().add('-flag', Label('@@foo~//:foo'), format = '_%s_'))", + "a.append(actions.args().add_all(['foo', Label('@@foo~//:foo')]))", + "a.append(actions.args().add_all(depset([Label('@@repo~//:foo'), Label('@@foo~//:foo')])))", + "ruleContext.actions.run(", + " inputs = depset(ruleContext.files.srcs),", + " outputs = ruleContext.files.srcs,", + " arguments = a,", + " executable = ruleContext.files.tools[0],", + " toolchain = None", + ")"); + SpawnAction action = + (SpawnAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + assertThat(action.getArguments()) + .containsExactly( + "foo/t.exe", + "@foo//:foo", + "-flag", + "@foo//:foo", + "-flag", + "_@foo//:foo_", + "foo", + "@foo//:foo", + "@@repo~//:foo", + "@foo//:foo") + .inOrder(); + } + + @Test + public void testArgsBuiltTwiceWithExternalLabel() throws Exception { + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + ev.exec( + "args = ruleContext.actions.args()", + "args.add(Label('@@foo'))", + "ruleContext.actions.run(", + " inputs = depset(ruleContext.files.srcs),", + " outputs = ruleContext.files.srcs,", + " arguments = [args],", + " executable = ruleContext.files.tools[0],", + " toolchain = None", + ")", + "ruleContext.actions.run(", + " inputs = depset(ruleContext.files.srcs),", + " outputs = ruleContext.files.srcs,", + " arguments = [args],", + " executable = ruleContext.files.tools[0],", + " toolchain = None", + ")"); + List actions = + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions().stream() + .map(SpawnAction.class::cast) + .toList(); + assertThat(actions).hasSize(2); + assertThat(actions.getFirst().getArguments()) + .containsExactlyElementsIn(actions.getLast().getArguments()) + .inOrder(); + } + @Test public void testConfigurationField_starlarkSplitTransitionProhibited() throws Exception { scratch.overwriteFile( @@ -3197,7 +3349,7 @@ private String getDigest(CommandLine commandLine, ArtifactExpander artifactExpan private CommandLine getCommandLine(String... lines) throws Exception { ev.exec(lines); - return ((Args) ev.eval("args")).build(); + return ((Args) ev.eval("args")).build(() -> RepositoryMapping.ALWAYS_FALLBACK); } @Test @@ -3220,7 +3372,7 @@ public void testDirectoryInArgs() throws Exception { Sequence result = (Sequence) ev.eval("args, directory"); Args args = (Args) result.get(0); Artifact directory = (Artifact) result.get(1); - CommandLine commandLine = args.build(); + CommandLine commandLine = args.build(() -> RepositoryMapping.ALWAYS_FALLBACK); // When asking for arguments without an artifact expander we just return the directory assertThat(commandLine.arguments()).containsExactly("foo/dir"); @@ -3246,7 +3398,7 @@ public void testDirectoryInArgsExpandDirectories() throws Exception { Sequence result = (Sequence) ev.eval("args, directory"); Args args = (Args) result.get(0); Artifact directory = (Artifact) result.get(1); - CommandLine commandLine = args.build(); + CommandLine commandLine = args.build(() -> RepositoryMapping.ALWAYS_FALLBACK); Artifact file1 = getBinArtifactWithNoOwner("foo/dir/file1"); Artifact file2 = getBinArtifactWithNoOwner("foo/dir/file2"); @@ -3297,7 +3449,7 @@ public void testDirectoryExpansionInArgs() throws Exception { "args.add_all([directory, file3], map_each=_expand_dirs)"); Args args = (Args) ev.eval("args"); Artifact directory = (Artifact) ev.eval("directory"); - CommandLine commandLine = args.build(); + CommandLine commandLine = args.build(() -> RepositoryMapping.ALWAYS_FALLBACK); Artifact file1 = getBinArtifactWithNoOwner("foo/dir/file1"); Artifact file2 = getBinArtifactWithNoOwner("foo/dir/file2"); @@ -3317,7 +3469,7 @@ public void testCallDirectoryExpanderWithWrongType() throws Exception { " return dir_expander.expand('oh no a string')", "args.add_all([f], map_each=_expand_dirs)"); Args args = (Args) ev.eval("args"); - CommandLine commandLine = args.build(); + CommandLine commandLine = args.build(() -> RepositoryMapping.ALWAYS_FALLBACK); assertThrows(CommandLineExpansionException.class, commandLine::arguments); } diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index fef7cccad28b87..7517f98b2d7d79 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -263,6 +263,9 @@ sh_test( "@bazel_tools//tools/bash/runfiles", ], exec_compatible_with = ["//:highcpu_machine"], + tags = [ + "requires-network", # For Bzlmod + ], ) JAVA_VERSIONS = ("11", "17", "21") @@ -292,6 +295,9 @@ JAVA_VERSIONS_COVERAGE = ("11", "17", "21") "@bazel_tools//tools/bash/runfiles", ], exec_compatible_with = ["//:highcpu_machine"], + tags = [ + "requires-network", # For Bzlmod + ], ) for java_version in JAVA_VERSIONS ] @@ -316,8 +322,11 @@ JAVA_VERSIONS_COVERAGE = ("11", "17", "21") ":test-deps", "@bazel_tools//tools/bash/runfiles", ], - # This test is only run by the java_tools binaries pipeline. - tags = ["manual"], + tags = [ + # This test is only run by the java_tools binaries pipeline. + "manual", + "requires-network", # For Bzlmod + ], ) for java_version in JAVA_VERSIONS ] diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh index 92db25e7c9fa02..6d5a1e339d0b1a 100755 --- a/src/test/shell/bazel/bazel_java_test.sh +++ b/src/test/shell/bazel/bazel_java_test.sh @@ -1981,4 +1981,44 @@ EOF >& $TEST_log || fail "build failed" } +function test_strict_deps_error_external_repo_starlark_action() { + cat << 'EOF' > MODULE.bazel +bazel_dep( + name = "lib_c", + repo_name = "c", +) +local_path_override( + module_name = "lib_c", + path = "lib_c", +) +EOF + + mkdir -p pkg + cat << 'EOF' > pkg/BUILD +java_library(name = "a", srcs = ["A.java"], deps = [":b"]) +java_library(name = "b", srcs = ["B.java"], deps = ["@c"]) +EOF + cat << 'EOF' > pkg/A.java +public class A extends B implements C {} +EOF + cat << 'EOF' > pkg/B.java +public class B implements C {} +EOF + + mkdir -p lib_c + cat << 'EOF' > lib_c/MODULE.bazel +module(name = "lib_c") +EOF + cat << 'EOF' > lib_c/BUILD +java_library(name = "c_pregen", srcs = ["C.java"]) +java_import(name = "c", jars = ["libc_pregen.jar"], visibility = ["//visibility:public"]) +EOF + cat << 'EOF' > lib_c/C.java +public interface C {} +EOF + + bazel build //pkg:a >& $TEST_log && fail "build should fail" + expect_log "buildozer 'add deps @c//:c' //pkg:a" +} + run_suite "Java integration tests"