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 d4b6ec7ff51dc9..4f1ed9dec21cf2 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 @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.MiddlemanFactory; 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; @@ -166,4 +167,9 @@ Artifact.DerivedArtifact getDerivedArtifact( 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 06eec0debc63d1..ed76de6b3acb11 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -411,6 +411,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", @@ -421,6 +422,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", @@ -2450,9 +2452,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 0ee01e8cf7b47d..069019856446c3 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 @@ -27,9 +27,12 @@ import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.MiddlemanFactory; +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.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; @@ -368,6 +371,19 @@ private WorkspaceStatusValue getWorkspaceStatusValue() throws InterruptedExcepti return workspaceStatusValue; } + @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 8c133dc8712369..77a19edf02c2fd 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 @@ -23,10 +23,13 @@ import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; 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.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; @@ -70,7 +73,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) { @@ -102,7 +106,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. @@ -157,7 +162,7 @@ public ImmutableSet getDirectoryArtifacts() { } @Override - public CommandLine build() { + public CommandLine build(InterruptibleSupplier mainRepoMappingSupplier) { return commandLine; } @@ -259,6 +264,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; @@ -307,6 +318,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; + } addSingleArg(value, format != Starlark.NONE ? (String) format : null); return this; } @@ -436,8 +450,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; @@ -451,8 +469,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); } @@ -573,8 +599,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 @@ -585,18 +613,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 9091c80ff8dc03..19f75a9cf303a0 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 @@ -51,6 +51,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; @@ -68,6 +69,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; @@ -332,7 +334,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(); @@ -347,7 +350,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()); @@ -373,7 +376,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); @@ -382,7 +385,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"); @@ -560,7 +563,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); @@ -568,7 +571,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' @@ -631,8 +634,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 { ImmutableList.Builder stringArgs = null; for (Object value : argumentsList) { if (value instanceof String) { @@ -647,7 +653,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", @@ -1046,6 +1052,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 5562e5026bbb00..bfb2f15e69b063 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 @@ -22,6 +22,7 @@ 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.common.collect.UnmodifiableIterator; import com.google.devtools.build.lib.actions.ActionKeyContext; @@ -41,6 +42,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; @@ -256,7 +259,8 @@ private int preprocess( int argi, PreprocessedCommandLine.Builder builder, @Nullable ArtifactExpander artifactExpander, - PathMapper pathMapper) + PathMapper pathMapper, + @Nullable RepositoryMapping mainRepoMapping) throws CommandLineExpansionException, InterruptedException { StarlarkCallable mapEach = null; StarlarkSemantics starlarkSemantics = null; @@ -293,7 +297,7 @@ private int preprocess( 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 @@ -690,6 +694,7 @@ static void push(List arguments, Object object, String format) { * @param builder the {@link PreprocessedCommandLine.Builder} in which to add a preprocessed * representation of this arg * @param pathMapper mapper for exec paths + * @param mainRepoMapping the repository mapping to use for formatting labels if needed * @return index in {@code arguments} where the next arg begins, or {@code arguments.size()} if * there are no more arguments */ @@ -697,10 +702,12 @@ static int preprocess( List arguments, int argi, PreprocessedCommandLine.Builder builder, - PathMapper pathMapper) { + PathMapper pathMapper, + @Nullable RepositoryMapping mainRepoMapping) { Object object = arguments.get(argi++); String formatStr = (String) arguments.get(argi++); - String stringValue = StarlarkCustomCommandLine.expandToCommandLine(object, pathMapper); + String stringValue = + StarlarkCustomCommandLine.expandToCommandLine(object, pathMapper, mainRepoMapping); builder.addPreprocessedArg(new PreprocessedSingleFormattedArg(formatStr, stringValue)); return argi; } @@ -754,11 +761,17 @@ Builder addFormatted(Object object, String format) { return this; } - CommandLine build(boolean flagPerLine) { + CommandLine build(boolean flagPerLine, @Nullable RepositoryMapping mainRepoMapping) { if (arguments.isEmpty()) { return CommandLine.empty(); } - Object[] args = arguments.toArray(); + 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); @@ -791,14 +804,29 @@ public ArgChunk expand(@Nullable ArtifactExpander artifactExpander, PathMapper p PreprocessedCommandLine.Builder builder = new PreprocessedCommandLine.Builder(); 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).preprocess(arguments, argi, builder, artifactExpander, pathMapper); + argi = + ((VectorArg) arg) + .preprocess( + arguments, argi, builder, artifactExpander, pathMapper, mainRepoMapping); } else if (arg == SingleFormattedArg.MARKER) { - argi = SingleFormattedArg.preprocess(arguments, argi, builder, pathMapper); + argi = SingleFormattedArg.preprocess(arguments, argi, builder, pathMapper, mainRepoMapping); } else { - builder.addString(expandToCommandLine(arg, pathMapper)); + builder.addString(expandToCommandLine(arg, pathMapper, mainRepoMapping)); } } return pathMapper.mapCustomStarlarkArgs(builder.build()); @@ -816,7 +844,12 @@ public final Iterable arguments(ArtifactExpander artifactExpander, PathM return expand(artifactExpander, pathMapper).arguments(); } - 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. @@ -848,20 +881,32 @@ public ArgChunk expand(@Nullable ArtifactExpander artifactExpander, PathMapper p List arguments = ((StarlarkCustomCommandLine) this).rawArgsAsList(); Iterator startIndexIterator = argStartIndexes.iterator(); - for (int argi = 0; argi < arguments.size(); ) { - int nextStartIndex = - startIndexIterator.hasNext() ? startIndexIterator.next() : 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; ) { + int nextStartIndex = startIndexIterator.hasNext() ? startIndexIterator.next() : size; PreprocessedCommandLine.Builder line = new PreprocessedCommandLine.Builder(); while (argi < nextStartIndex) { Object arg = arguments.get(argi++); if (arg instanceof VectorArg) { argi = - ((VectorArg) arg).preprocess(arguments, argi, line, artifactExpander, pathMapper); + ((VectorArg) arg) + .preprocess( + arguments, argi, line, artifactExpander, pathMapper, mainRepoMapping); } else if (arg == SingleFormattedArg.MARKER) { - argi = SingleFormattedArg.preprocess(arguments, argi, line, pathMapper); + argi = + SingleFormattedArg.preprocess(arguments, argi, line, pathMapper, mainRepoMapping); } else { - line.addString(expandToCommandLine(arg, pathMapper)); + line.addString(expandToCommandLine(arg, pathMapper, mainRepoMapping)); } } @@ -879,7 +924,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 e302b1481d260a..0e061301e88ee6 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", @@ -553,7 +554,7 @@ void run( Object shadowedAction, Object resourceSetUnchecked, Object toolchainUnchecked) - throws EvalException; + throws EvalException, InterruptedException; @StarlarkMethod( name = "run_shell", @@ -804,7 +805,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 7b0c6922520a20..772d33d70d60c2 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 @@ -38,6 +38,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; @@ -74,7 +75,11 @@ public void createArtifactRoot() throws IOException { @Test public void add() throws Exception { CommandLine commandLine = - builder.add("one").add("two").add("three").build(/* flagPerLine= */ false); + builder + .add("one") + .add("two") + .add("three") + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "one", "two", "three"); } @@ -85,7 +90,7 @@ public void addFormatted() throws Exception { .addFormatted("one", "--arg1=%s") .addFormatted("two", "--arg2=%s") .addFormatted("three", "--arg3=%s") - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "--arg1=one", "--arg2=two", "--arg3=three"); } @@ -95,7 +100,7 @@ public void argName() throws Exception { builder .add(vectorArg("one", "two", "three").setArgName("--arg")) .add(vectorArg("four").setArgName("--other_arg")) - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "--arg", "one", "two", "three", "--other_arg", "four"); } @@ -105,7 +110,7 @@ public void terminateWith() throws Exception { builder .add(vectorArg("one", "two", "three").setTerminateWith("end1")) .add(vectorArg("four").setTerminateWith("end2")) - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "one", "two", "three", "end1", "four", "end2"); } @@ -115,7 +120,7 @@ public void formatEach() throws Exception { builder .add(vectorArg("one", "two", "three").setFormatEach("--arg=%s")) .add(vectorArg("four").setFormatEach("--other_arg=%s")) - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "--arg=one", "--arg=two", "--arg=three", "--other_arg=four"); } @@ -125,7 +130,7 @@ public void beforeEach() throws Exception { builder .add(vectorArg("one", "two", "three").setBeforeEach("b4")) .add(vectorArg("four").setBeforeEach("and")) - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "b4", "one", "b4", "two", "b4", "three", "and", "four"); } @@ -135,7 +140,7 @@ public void joinWith() throws Exception { builder .add(vectorArg("one", "two", "three").setJoinWith("...")) .add(vectorArg("four").setJoinWith("n/a")) - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "one...two...three", "four"); } @@ -145,7 +150,7 @@ public void formatJoined() throws Exception { builder .add(vectorArg("one", "two", "three").setJoinWith("...").setFormatJoined("--arg=%s")) .add(vectorArg("four").setJoinWith("n/a").setFormatJoined("--other_arg=%s")) - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "--arg=one...two...three", "--other_arg=four"); } @@ -156,7 +161,7 @@ public void emptyVectorArg_omit() throws Exception { .add("before") .add(vectorArg().omitIfEmpty(true).setJoinWith(",").setFormatJoined("--empty=%s")) .add("after") - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "before", "after"); } @@ -167,7 +172,7 @@ public void emptyVectorArg_noOmit() throws Exception { .add("before") .add(vectorArg().omitIfEmpty(false).setJoinWith(",").setFormatJoined("--empty=%s")) .add("after") - .build(/* flagPerLine= */ false); + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine(commandLine, "before", "--empty=", "after"); } @@ -184,7 +189,7 @@ public void flagPerLine() throws Exception { .add("single_arg") .recordArgStart() .add(vectorArg("", "line", "four", "has", "no").setTerminateWith("flag")) - .build(/* flagPerLine= */ true); + .build(/* flagPerLine= */ true, RepositoryMapping.ALWAYS_FALLBACK); verifyCommandLine( commandLine, "--this=is line one", @@ -198,7 +203,9 @@ public void vectorArgAddToFingerprint_treeArtifactMissingExpansion_returnsDigest throws Exception { SpecialArtifact tree = createTreeArtifact("tree"); CommandLine commandLine = - builder.add(vectorArg(tree).setExpandDirectories(true)).build(/* flagPerLine= */ false); + builder + .add(vectorArg(tree).setExpandDirectories(true)) + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); ActionKeyContext actionKeyContext = new ActionKeyContext(); Fingerprint fingerprint = new Fingerprint(); @@ -212,7 +219,9 @@ public void vectorArgAddToFingerprint_treeArtifactMissingExpansion_returnsDigest public void vectorArgAddToFingerprint_expandFileset_includesInDigest() throws Exception { SpecialArtifact fileset = createFileset("fileset"); CommandLine commandLine = - builder.add(vectorArg(fileset).setExpandDirectories(true)).build(/* flagPerLine= */ false); + builder + .add(vectorArg(fileset).setExpandDirectories(true)) + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); FilesetOutputSymlink symlink1 = createFilesetSymlink("file1"); FilesetOutputSymlink symlink2 = createFilesetSymlink("file2"); ActionKeyContext actionKeyContext = new ActionKeyContext(); @@ -231,7 +240,9 @@ public void vectorArgAddToFingerprint_expandFileset_includesInDigest() throws Ex public void vectorArgAddToFingerprint_expandTreeArtifact_includesInDigest() throws Exception { SpecialArtifact tree = createTreeArtifact("tree"); CommandLine commandLine = - builder.add(vectorArg(tree).setExpandDirectories(true)).build(/* flagPerLine= */ false); + builder + .add(vectorArg(tree).setExpandDirectories(true)) + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); TreeFileArtifact child = TreeFileArtifact.createTreeOutput(tree, "child"); ActionKeyContext actionKeyContext = new ActionKeyContext(); Fingerprint fingerprint = new Fingerprint(); @@ -249,7 +260,9 @@ public void vectorArgAddToFingerprint_expandTreeArtifact_includesInDigest() thro public void vectorArgAddToFingerprint_expandFilesetMissingExpansion_fails() { SpecialArtifact fileset = createFileset("fileset"); CommandLine commandLine = - builder.add(vectorArg(fileset).setExpandDirectories(true)).build(/* flagPerLine= */ false); + builder + .add(vectorArg(fileset).setExpandDirectories(true)) + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); ActionKeyContext actionKeyContext = new ActionKeyContext(); Fingerprint fingerprint = new Fingerprint(); @@ -262,7 +275,9 @@ public void vectorArgAddToFingerprint_expandFilesetMissingExpansion_fails() { public void vectorArgArguments_expandsTreeArtifact() throws Exception { SpecialArtifact tree = createTreeArtifact("tree"); CommandLine commandLine = - builder.add(vectorArg(tree).setExpandDirectories(true)).build(/* flagPerLine= */ false); + builder + .add(vectorArg(tree).setExpandDirectories(true)) + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); TreeFileArtifact child1 = TreeFileArtifact.createTreeOutput(tree, "child1"); TreeFileArtifact child2 = TreeFileArtifact.createTreeOutput(tree, "child2"); ArtifactExpander artifactExpander = @@ -279,7 +294,9 @@ public void vectorArgArguments_expandsTreeArtifact() throws Exception { public void vectorArgArguments_expandsFileset() throws Exception { SpecialArtifact fileset = createFileset("fileset"); CommandLine commandLine = - builder.add(vectorArg(fileset).setExpandDirectories(true)).build(/* flagPerLine= */ false); + builder + .add(vectorArg(fileset).setExpandDirectories(true)) + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); FilesetOutputSymlink symlink1 = createFilesetSymlink("file1"); FilesetOutputSymlink symlink2 = createFilesetSymlink("file2"); ArtifactExpander artifactExpander = @@ -296,7 +313,9 @@ public void vectorArgArguments_expandsFileset() throws Exception { public void vectorArgArguments_treeArtifactMissingExpansion_returnsEmptyList() throws Exception { SpecialArtifact tree = createTreeArtifact("tree"); CommandLine commandLine = - builder.add(vectorArg(tree).setExpandDirectories(true)).build(/* flagPerLine= */ false); + builder + .add(vectorArg(tree).setExpandDirectories(true)) + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); // TODO(b/167696101): Fail arguments computation when we are missing the directory from inputs. assertThat(commandLine.arguments(EMPTY_EXPANDER, PathMapper.NOOP)).isEmpty(); @@ -306,7 +325,9 @@ public void vectorArgArguments_treeArtifactMissingExpansion_returnsEmptyList() t public void vectorArgArguments_filesetMissingExpansion_fails() { SpecialArtifact fileset = createFileset("fileset"); CommandLine commandLine = - builder.add(vectorArg(fileset).setExpandDirectories(true)).build(/* flagPerLine= */ false); + builder + .add(vectorArg(fileset).setExpandDirectories(true)) + .build(/* flagPerLine= */ false, RepositoryMapping.ALWAYS_FALLBACK); assertThrows( CommandLineExpansionException.class, 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 42ed8ad774cbbf..f8391718f26b4b 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 @@ -49,6 +49,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; @@ -225,6 +226,11 @@ public ImmutableSet getTreeArtifactsConflictingWithFiles() { public ActionKeyContext getActionKeyContext() { return original.getActionKeyContext(); } + + @Override + public RepositoryMapping getMainRepoMapping() throws InterruptedException { + return original.getMainRepoMapping(); + } } /** A dummy WorkspaceStatusAction. */ @@ -455,6 +461,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 a4b40895c560a2..79cb6503d6ee99 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 @@ -111,6 +111,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; @@ -2141,6 +2142,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 fa34fb594d8cd2..3b8307d09c1e9a 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/BUILD +++ b/src/test/java/com/google/devtools/build/lib/starlark/BUILD @@ -82,6 +82,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/rules/python:PythonTestUtils", "//src/test/java/com/google/devtools/build/lib/starlark/util", 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 94fc1fbe80b162..932805604df9da 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; @@ -51,6 +52,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; @@ -2690,6 +2692,156 @@ def _dep_impl(ctx): 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( @@ -3365,7 +3517,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 @@ -3388,7 +3540,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"); @@ -3414,7 +3566,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"); @@ -3465,7 +3617,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"); @@ -3485,7 +3637,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 b0ec4eb348f54a..ce24e5e875f7f0 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -273,6 +273,9 @@ sh_test( "@bazel_tools//tools/bash/runfiles", ], exec_compatible_with = ["//:highcpu_machine"], + tags = [ + "requires-network", # For Bzlmod + ], ) JAVA_VERSIONS = ("11", "17", "21") @@ -302,6 +305,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 ] @@ -326,8 +332,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 192bdb212df1ef..a77f00874b2411 100755 --- a/src/test/shell/bazel/bazel_java_test.sh +++ b/src/test/shell/bazel/bazel_java_test.sh @@ -1958,4 +1958,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"