From 527308cd3d965855b5a37ef8c0131165f0b10df5 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 25 Oct 2022 11:44:33 -0700 Subject: [PATCH] Write a repo mapping manifest in the runfiles directory To ensure we can use repo mappings in the runfiles library, this change writes an extra file "my_binary_target.repo_mapping", which contains a bunch of (base_repo_canonical_name, apparent_repo_name, canonical_repo_name) triples. See https://github.com/bazelbuild/proposals/blob/main/designs/2022-07-21-locating-runfiles-with-bzlmod.md for more information. The extra file is written using a new action "RepoMappingManifestAction", and it's only executed if Bzlmod is enabled. This avoids generating a lot of extra actions that are essentially useless for monorepo setups such as Google's. Work towards https://github.com/bazelbuild/bazel/issues/16124 PiperOrigin-RevId: 483735699 Change-Id: I885b4df093bd2c783c57d19f995f420b9b29b53c --- .../google/devtools/build/lib/analysis/BUILD | 20 ++ .../analysis/RepoMappingManifestAction.java | 126 ++++++++++ .../build/lib/analysis/RunfilesSupport.java | 103 +++++++- .../build/lib/cmdline/RepositoryMapping.java | 11 +- .../google/devtools/build/lib/analysis/BUILD | 24 ++ .../RunfilesRepoMappingManifestTest.java | 228 ++++++++++++++++++ .../lib/analysis/util/BuildViewTestCase.java | 2 +- src/test/py/bazel/bzlmod/bazel_module_test.py | 111 ++++++++- src/test/py/bazel/bzlmod/test_utils.py | 14 +- 9 files changed, 616 insertions(+), 23 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/analysis/RepoMappingManifestAction.java create mode 100644 src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java 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 4f5328526e0724..d42c34e2c4b0a1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -336,6 +336,7 @@ java_library( ":package_specification_provider", ":platform_options", ":provider_collection", + ":repo_mapping_manifest_action", ":required_config_fragments_provider", ":resolved_toolchain_context", ":rule_configured_object_value", @@ -982,6 +983,25 @@ java_library( ], ) +java_library( + name = "repo_mapping_manifest_action", + srcs = ["RepoMappingManifestAction.java"], + deps = [ + ":actions/abstract_file_write_action", + ":actions/deterministic_writer", + "//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/util", + "//src/main/java/net/starlark/java/eval", + "//third_party:auto_value", + "//third_party:guava", + "//third_party:jsr305", + ], +) + java_library( name = "required_config_fragments_provider", srcs = ["RequiredConfigFragmentsProvider.java"], diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RepoMappingManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/RepoMappingManifestAction.java new file mode 100644 index 00000000000000..11f5fe82022ebf --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/RepoMappingManifestAction.java @@ -0,0 +1,126 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.analysis; + +import static java.nio.charset.StandardCharsets.ISO_8859_1; +import static java.util.Comparator.comparing; + +import com.google.auto.value.AutoValue; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionKeyContext; +import com.google.devtools.build.lib.actions.ActionOwner; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +import com.google.devtools.build.lib.analysis.actions.DeterministicWriter; +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; +import com.google.devtools.build.lib.util.Fingerprint; +import java.io.PrintWriter; +import java.util.List; +import java.util.UUID; +import javax.annotation.Nullable; +import net.starlark.java.eval.EvalException; + +/** Creates a manifest file describing the repos and mappings relevant for a runfile tree. */ +public class RepoMappingManifestAction extends AbstractFileWriteAction { + private static final UUID MY_UUID = UUID.fromString("458e351c-4d30-433d-b927-da6cddd4737f"); + + private final ImmutableList entries; + private final String workspaceName; + + /** An entry in the repo mapping manifest file. */ + @AutoValue + public abstract static class Entry { + public static Entry of( + RepositoryName sourceRepo, String targetRepoApparentName, RepositoryName targetRepo) { + return new AutoValue_RepoMappingManifestAction_Entry( + sourceRepo, targetRepoApparentName, targetRepo); + } + + public abstract RepositoryName sourceRepo(); + + public abstract String targetRepoApparentName(); + + public abstract RepositoryName targetRepo(); + } + + public RepoMappingManifestAction( + ActionOwner owner, Artifact output, List entries, String workspaceName) { + super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, /*makeExecutable=*/ false); + this.entries = + ImmutableList.sortedCopyOf( + comparing((Entry e) -> e.sourceRepo().getName()) + .thenComparing(Entry::targetRepoApparentName), + entries); + this.workspaceName = workspaceName; + } + + @Override + public String getMnemonic() { + return "RepoMappingManifest"; + } + + @Override + protected String getRawProgressMessage() { + return "writing repo mapping manifest for " + getOwner().getLabel(); + } + + @Override + protected void computeKey( + ActionKeyContext actionKeyContext, + @Nullable ArtifactExpander artifactExpander, + Fingerprint fp) + throws CommandLineExpansionException, EvalException, InterruptedException { + fp.addUUID(MY_UUID); + fp.addString(workspaceName); + for (Entry entry : entries) { + fp.addString(entry.sourceRepo().getName()); + fp.addString(entry.targetRepoApparentName()); + fp.addString(entry.targetRepo().getName()); + } + } + + @Override + public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) + throws InterruptedException, ExecException { + return out -> { + PrintWriter writer = new PrintWriter(out, /*autoFlush=*/ false, ISO_8859_1); + for (Entry entry : entries) { + if (entry.targetRepoApparentName().isEmpty()) { + // The apparent repo name can only be empty for the main repo. We skip this line as + // Rlocation paths can't reference an empty apparent name anyway. + Preconditions.checkArgument( + entry.sourceRepo().isMain(), + "only the main repo mapping can contain an entry with an empty apparent name"); + continue; + } + // The canonical name of the main repo is the empty string, which is not a valid name for a + // directory, so the "workspace name" is used the name of the directory under the runfiles + // tree for it. + String targetRepoDirectoryName = + entry.targetRepo().isMain() ? workspaceName : entry.targetRepo().getName(); + writer.format( + "%s,%s,%s\n", + entry.sourceRepo().getName(), entry.targetRepoApparentName(), targetRepoDirectoryName); + } + writer.flush(); + }; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index ef668a7974a8be..189cb882fe15d7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -14,26 +14,34 @@ package com.google.devtools.build.lib.analysis; +import static com.google.common.collect.ImmutableSet.toImmutableSet; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.ActionEnvironment; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLine; +import com.google.devtools.build.lib.analysis.RepoMappingManifestAction.Entry; import com.google.devtools.build.lib.analysis.SourceManifestAction.ManifestType; import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; import com.google.devtools.build.lib.analysis.actions.SymlinkTreeAction; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.RunUnder; +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; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.packages.Type; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -76,11 +84,13 @@ public final class RunfilesSupport { private static final String RUNFILES_DIR_EXT = ".runfiles"; private static final String INPUT_MANIFEST_EXT = ".runfiles_manifest"; private static final String OUTPUT_MANIFEST_BASENAME = "MANIFEST"; + private static final String REPO_MAPPING_MANIFEST_EXT = ".repo_mapping"; private final Runfiles runfiles; private final Artifact runfilesInputManifest; private final Artifact runfilesManifest; + private final Artifact repoMappingManifest; private final Artifact runfilesMiddleman; private final Artifact owningExecutable; private final boolean buildRunfileLinks; @@ -132,8 +142,11 @@ private static RunfilesSupport create( runfilesInputManifest = null; runfilesManifest = null; } + Artifact repoMappingManifest = + createRepoMappingManifestAction(ruleContext, runfiles, owningExecutable); Artifact runfilesMiddleman = - createRunfilesMiddleman(ruleContext, owningExecutable, runfiles, runfilesManifest); + createRunfilesMiddleman( + ruleContext, owningExecutable, runfiles, runfilesManifest, repoMappingManifest); boolean runfilesEnabled = ruleContext.getConfiguration().runfilesEnabled(); @@ -141,6 +154,7 @@ private static RunfilesSupport create( runfiles, runfilesInputManifest, runfilesManifest, + repoMappingManifest, runfilesMiddleman, owningExecutable, buildRunfileLinks, @@ -153,6 +167,7 @@ private RunfilesSupport( Runfiles runfiles, Artifact runfilesInputManifest, Artifact runfilesManifest, + Artifact repoMappingManifest, Artifact runfilesMiddleman, Artifact owningExecutable, boolean buildRunfileLinks, @@ -162,6 +177,7 @@ private RunfilesSupport( this.runfiles = runfiles; this.runfilesInputManifest = runfilesInputManifest; this.runfilesManifest = runfilesManifest; + this.repoMappingManifest = repoMappingManifest; this.runfilesMiddleman = runfilesMiddleman; this.owningExecutable = owningExecutable; this.buildRunfileLinks = buildRunfileLinks; @@ -268,6 +284,17 @@ public Artifact getRunfilesManifest() { return runfilesManifest; } + /** + * Returns the foo.repo_mapping file if Bazel is run with transitive package tracking turned on + * (see {@code SkyframeExecutor#getForcedSingleSourceRootIfNoExecrootSymlinkCreation}) and any of + * the transitive packages come from a repository with strict deps (see {@code + * #collectRepoMappings}). Otherwise, returns null. + */ + @Nullable + public Artifact getRepoMappingManifest() { + return repoMappingManifest; + } + /** Returns the root directory of the runfiles symlink farm; otherwise, returns null. */ @Nullable public Path getRunfilesDirectory() { @@ -327,12 +354,16 @@ private static Artifact createRunfilesMiddleman( ActionConstructionContext context, Artifact owningExecutable, Runfiles runfiles, - @Nullable Artifact runfilesManifest) { + @Nullable Artifact runfilesManifest, + Artifact repoMappingManifest) { NestedSetBuilder deps = NestedSetBuilder.stableOrder(); deps.addTransitive(runfiles.getAllArtifacts()); if (runfilesManifest != null) { deps.add(runfilesManifest); } + if (repoMappingManifest != null) { + deps.add(repoMappingManifest); + } return context .getAnalysisEnvironment() .getMiddlemanFactory() @@ -495,4 +526,72 @@ public static Path inputManifestPath(Path runfilesDir) { public static Path outputManifestPath(Path runfilesDir) { return runfilesDir.getRelative(OUTPUT_MANIFEST_BASENAME); } + + @Nullable + private static Artifact createRepoMappingManifestAction( + RuleContext ruleContext, Runfiles runfiles, Artifact owningExecutable) { + if (!ruleContext + .getAnalysisEnvironment() + .getStarlarkSemantics() + .getBool(BuildLanguageOptions.ENABLE_BZLMOD)) { + // If Bzlmod is not enabled, we don't need a repo mapping manifest. + return null; + } + + PathFragment executablePath = + (owningExecutable != null) + ? owningExecutable.getOutputDirRelativePath( + ruleContext.getConfiguration().isSiblingRepositoryLayout()) + : ruleContext.getPackageDirectory().getRelative(ruleContext.getLabel().getName()); + Artifact repoMappingManifest = + ruleContext.getDerivedArtifact( + executablePath.replaceName(executablePath.getBaseName() + REPO_MAPPING_MANIFEST_EXT), + ruleContext.getBinDirectory()); + ruleContext + .getAnalysisEnvironment() + .registerAction( + new RepoMappingManifestAction( + ruleContext.getActionOwner(), + repoMappingManifest, + collectRepoMappings( + Preconditions.checkNotNull( + ruleContext.getTransitivePackagesForRunfileRepoMappingManifest()), + runfiles), + ruleContext.getWorkspaceName())); + return repoMappingManifest; + } + + /** Returns the list of entries (unsorted) that should appear in the repo mapping manifest. */ + private static ImmutableList collectRepoMappings( + NestedSet transitivePackages, Runfiles runfiles) { + // NOTE: It might appear that the flattening of `transitivePackages` is better suited to the + // execution phase rather than here in the analysis phase, but we can't do that since it would + // necessitate storing `transitivePackages` in an action, which breaks skyframe serialization + // since packages cannot be serialized here. + + ImmutableSet reposContributingRunfiles = + runfiles.getAllArtifacts().toList().stream() + .filter(a -> a.getOwner() != null) + .map(a -> a.getOwner().getRepository()) + .collect(toImmutableSet()); + Set seenRepos = new HashSet<>(); + ImmutableList.Builder entries = ImmutableList.builder(); + for (Package pkg : transitivePackages.toList()) { + if (!seenRepos.add(pkg.getPackageIdentifier().getRepository())) { + // Any package from the same repo would have the same repo mapping. + continue; + } + for (Map.Entry repoMappingEntry : + pkg.getRepositoryMapping().entries().entrySet()) { + if (reposContributingRunfiles.contains(repoMappingEntry.getValue())) { + entries.add( + Entry.of( + pkg.getPackageIdentifier().getRepository(), + repoMappingEntry.getKey(), + repoMappingEntry.getValue())); + } + } + } + return entries.build(); + } } 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 3a2889c324236a..a1280ed3fb2dea 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 @@ -37,7 +37,8 @@ public abstract class RepositoryMapping { // Always fallback to the requested name public static final RepositoryMapping ALWAYS_FALLBACK = createAllowingFallback(ImmutableMap.of()); - abstract ImmutableMap repositoryMapping(); + /** Returns all the entries in this repo mapping. */ + public abstract ImmutableMap entries(); /** * The owner repo of this repository mapping. It is for providing useful debug information when @@ -66,7 +67,7 @@ public static RepositoryMapping createAllowingFallback( */ public RepositoryMapping withAdditionalMappings(Map additionalMappings) { HashMap allMappings = new HashMap<>(additionalMappings); - allMappings.putAll(repositoryMapping()); + allMappings.putAll(entries()); return new AutoValue_RepositoryMapping(ImmutableMap.copyOf(allMappings), ownerRepo()); } @@ -76,7 +77,7 @@ public RepositoryMapping withAdditionalMappings(Map addi * repo of the given additional mappings is ignored. */ public RepositoryMapping withAdditionalMappings(RepositoryMapping additionalMappings) { - return withAdditionalMappings(additionalMappings.repositoryMapping()); + return withAdditionalMappings(additionalMappings.entries()); } /** @@ -84,7 +85,7 @@ public RepositoryMapping withAdditionalMappings(RepositoryMapping additionalMapp * provided apparent repo name is assumed to be valid. */ public RepositoryName get(String preMappingName) { - RepositoryName canonicalRepoName = repositoryMapping().get(preMappingName); + RepositoryName canonicalRepoName = entries().get(preMappingName); if (canonicalRepoName != null) { return canonicalRepoName; } @@ -108,7 +109,7 @@ public boolean usesStrictDeps() { * Returns the first apparent name in this mapping that maps to the given canonical name, if any. */ public Optional getInverse(RepositoryName postMappingName) { - return repositoryMapping().entrySet().stream() + return entries().entrySet().stream() .filter(e -> e.getValue().equals(postMappingName)) .map(Entry::getKey) .findFirst(); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD index da2be4dce0d157..d89744dfd73bd8 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD @@ -35,6 +35,7 @@ java_library( "JDKJavaLauncherRunfilesSupportTest.java", "PackageGroupBuildViewTest.java", "RuleConfiguredTargetTest.java", + "RunfilesRepoMappingManifestTest.java", "SourceManifestActionTest.java", "AnalysisFailureInfoTest.java", ], @@ -355,6 +356,29 @@ java_test( ], ) +java_test( + name = "RunfilesRepoMappingManifestTest", + srcs = ["RunfilesRepoMappingManifestTest.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", + "//src/main/java/com/google/devtools/build/lib/analysis:repo_mapping_manifest_action", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl", + "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", + "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/skyframe", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/test/java/com/google/devtools/build/lib/analysis/util", + "//src/test/java/com/google/devtools/build/lib/bazel/bzlmod:util", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) + java_test( name = "TransitiveValidationPropagationTest", srcs = ["TransitiveValidationPropagationTest.java"], diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java new file mode 100644 index 00000000000000..4c27d2bd2a4b44 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java @@ -0,0 +1,228 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.analysis; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.bazel.bzlmod.BzlmodTestUtil.createModuleKey; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction; +import com.google.devtools.build.lib.bazel.bzlmod.FakeRegistry; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction; +import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode; +import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; +import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected; +import com.google.devtools.build.lib.vfs.Path; +import java.util.Map.Entry; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests that the repo mapping manifest file is properly generated for runfiles. */ +@RunWith(JUnit4.class) +public class RunfilesRepoMappingManifestTest extends BuildViewTestCase { + private Path moduleRoot; + private FakeRegistry registry; + + @Override + protected ImmutableList extraPrecomputedValues() throws Exception { + moduleRoot = scratch.dir("modules"); + registry = FakeRegistry.DEFAULT_FACTORY.newFakeRegistry(moduleRoot.getPathString()); + return ImmutableList.of( + PrecomputedValue.injected( + ModuleFileFunction.REGISTRIES, ImmutableList.of(registry.getUrl())), + PrecomputedValue.injected(ModuleFileFunction.IGNORE_DEV_DEPS, false), + PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, ImmutableMap.of()), + PrecomputedValue.injected( + BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES, CheckDirectDepsMode.WARNING), + PrecomputedValue.injected( + BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, BazelCompatibilityMode.ERROR), + PrecomputedValue.injected( + BazelModuleResolutionFunction.ALLOWED_YANKED_VERSIONS, ImmutableList.of())); + } + + @Before + public void enableBzlmod() throws Exception { + setBuildLanguageOptions("--enable_bzlmod"); + } + + /** + * Sets up a Bazel module bare_rule@1.0, which provides a bare_binary rule that passes along + * runfiles in the data attribute, and does nothing else. + */ + @Before + public void setupBareBinaryRule() throws Exception { + registry.addModule( + createModuleKey("bare_rule", "1.0"), "module(name='bare_rule',version='1.0')"); + scratch.overwriteFile(moduleRoot.getRelative("bare_rule~1.0/WORKSPACE").getPathString()); + scratch.overwriteFile( + moduleRoot.getRelative("bare_rule~1.0/defs.bzl").getPathString(), + "def _bare_binary_impl(ctx):", + " exe = ctx.actions.declare_file(ctx.label.name)", + " ctx.actions.write(exe, 'i got nothing', True)", + " runfiles = ctx.runfiles(files=ctx.files.data)", + " for data in ctx.attr.data:", + " runfiles = runfiles.merge(data[DefaultInfo].default_runfiles)", + " return DefaultInfo(files=depset(direct=[exe]), executable=exe, runfiles=runfiles)", + "bare_binary=rule(", + " implementation=_bare_binary_impl,", + " attrs={'data':attr.label_list(allow_files=True)},", + " executable=True,", + ")"); + scratch.overwriteFile( + moduleRoot.getRelative("bare_rule~1.0/BUILD").getPathString(), + "load('//:defs.bzl', 'bare_binary')", + "bare_binary(name='bare_binary')"); + } + + private ImmutableList getRepoMappingManifestForTarget(String label) throws Exception { + Action action = getGeneratingAction(getRunfilesSupport(label).getRepoMappingManifest()); + assertThat(action).isInstanceOf(RepoMappingManifestAction.class); + return ((RepoMappingManifestAction) action) + .newDeterministicWriter(null) + .getBytes() + .toStringUtf8() + .lines() + .collect(toImmutableList()); + } + + @Test + public void diamond() throws Exception { + rewriteWorkspace("workspace(name='aaa_ws')"); + scratch.overwriteFile( + "MODULE.bazel", + "module(name='aaa',version='1.0')", + "bazel_dep(name='bbb',version='1.0')", + "bazel_dep(name='ccc',version='2.0')", + "bazel_dep(name='bare_rule',version='1.0')"); + registry.addModule( + createModuleKey("bbb", "1.0"), + "module(name='bbb',version='1.0')", + "bazel_dep(name='ddd',version='1.0')", + "bazel_dep(name='bare_rule',version='1.0')"); + registry.addModule( + createModuleKey("ccc", "2.0"), + "module(name='ccc',version='2.0')", + "bazel_dep(name='ddd',version='2.0')", + "bazel_dep(name='bare_rule',version='1.0')"); + registry.addModule( + createModuleKey("ddd", "1.0"), + "module(name='ddd',version='1.0')", + "bazel_dep(name='bare_rule',version='1.0')"); + registry.addModule( + createModuleKey("ddd", "2.0"), + "module(name='ddd',version='2.0')", + "bazel_dep(name='bare_rule',version='1.0')"); + + scratch.overwriteFile( + "BUILD", + "load('@bare_rule//:defs.bzl', 'bare_binary')", + "bare_binary(name='aaa',data=['@bbb'])"); + ImmutableMap buildFiles = + ImmutableMap.of( + "bbb~1.0", "bare_binary(name='bbb',data=['@ddd'])", + "ccc~2.0", "bare_binary(name='ccc',data=['@ddd'])", + "ddd~1.0", "bare_binary(name='ddd')", + "ddd~2.0", "bare_binary(name='ddd')"); + for (Entry entry : buildFiles.entrySet()) { + scratch.overwriteFile( + moduleRoot.getRelative(entry.getKey()).getRelative("WORKSPACE").getPathString()); + scratch.overwriteFile( + moduleRoot.getRelative(entry.getKey()).getRelative("BUILD").getPathString(), + "load('@bare_rule//:defs.bzl', 'bare_binary')", + entry.getValue()); + } + + assertThat(getRepoMappingManifestForTarget("//:aaa")) + .containsExactly( + ",aaa,_main", + ",aaa_ws,_main", + ",bbb,bbb~1.0", + "bbb~1.0,bbb,bbb~1.0", + "bbb~1.0,ddd,ddd~2.0", + "ddd~2.0,ddd,ddd~2.0") + .inOrder(); + assertThat(getRepoMappingManifestForTarget("@@ccc~2.0//:ccc")) + .containsExactly("ccc~2.0,ccc,ccc~2.0", "ccc~2.0,ddd,ddd~2.0", "ddd~2.0,ddd,ddd~2.0") + .inOrder(); + } + + @Test + public void runfilesFromToolchain() throws Exception { + rewriteWorkspace("workspace(name='main')"); + scratch.overwriteFile("MODULE.bazel", "bazel_dep(name='tooled_rule',version='1.0')"); + // tooled_rule offers a tooled_binary rule, which uses a toolchain backed by a binary from + // bare_rule. tooled_binary explicitly requests that runfiles from this binary are included in + // its runfiles tree, which would mean that bare_rule should be included in the repo mapping + // manifest. + registry.addModule( + createModuleKey("tooled_rule", "1.0"), + "module(name='tooled_rule',version='1.0')", + "bazel_dep(name='bare_rule',version='1.0')", + "register_toolchains('//:all')"); + scratch.overwriteFile(moduleRoot.getRelative("tooled_rule~1.0/WORKSPACE").getPathString()); + scratch.overwriteFile( + moduleRoot.getRelative("tooled_rule~1.0/defs.bzl").getPathString(), + "def _tooled_binary_impl(ctx):", + " exe = ctx.actions.declare_file(ctx.label.name)", + " ctx.actions.write(exe, 'i got something', True)", + " runfiles = ctx.runfiles(files=ctx.files.data)", + " for data in ctx.attr.data:", + " runfiles = runfiles.merge(data[DefaultInfo].default_runfiles)", + " runfiles = runfiles.merge(", + " ctx.toolchains['//:toolchain_type'].tooled_info[DefaultInfo].default_runfiles)", + " return DefaultInfo(files=depset(direct=[exe]), executable=exe, runfiles=runfiles)", + "tooled_binary = rule(", + " implementation=_tooled_binary_impl,", + " attrs={'data':attr.label_list(allow_files=True)},", + " executable=True,", + " toolchains=['//:toolchain_type'],", + ")", + "", + "def _tooled_toolchain_rule_impl(ctx):", + " return [platform_common.ToolchainInfo(tooled_info = ctx.attr.backing_binary)]", + "tooled_toolchain_rule=rule(_tooled_toolchain_rule_impl,", + " attrs={'backing_binary':attr.label()})", + "def tooled_toolchain(name, backing_binary):", + " tooled_toolchain_rule(name=name+'_impl',backing_binary=backing_binary)", + " native.toolchain(", + " name=name,", + " toolchain=':'+name+'_impl',", + " toolchain_type=Label('//:toolchain_type'),", + " )"); + scratch.overwriteFile( + moduleRoot.getRelative("tooled_rule~1.0/BUILD").getPathString(), + "load('//:defs.bzl', 'tooled_toolchain')", + "toolchain_type(name='toolchain_type')", + "tooled_toolchain(name='tooled_toolchain', backing_binary='@bare_rule//:bare_binary')"); + + scratch.overwriteFile( + "BUILD", + "load('@tooled_rule//:defs.bzl', 'tooled_binary')", + "tooled_binary(name='tooled')"); + + assertThat(getRepoMappingManifestForTarget("//:tooled")) + .containsExactly( + ",main,_main", + "bare_rule~1.0,bare_rule,bare_rule~1.0", + "tooled_rule~1.0,bare_rule,bare_rule~1.0") + .inOrder(); + } +} 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 e9fd7e03d246f7..81e983a18f01e7 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 @@ -382,7 +382,7 @@ protected boolean usesInliningBzlLoadFunction() { /** * Returns extra precomputed values to inject, both into Skyframe and the testing package loaders. */ - protected ImmutableList extraPrecomputedValues() { + protected ImmutableList extraPrecomputedValues() throws Exception { return ImmutableList.of(); } diff --git a/src/test/py/bazel/bzlmod/bazel_module_test.py b/src/test/py/bazel/bzlmod/bazel_module_test.py index 1fef31a78d1944..4822ff255ac93b 100644 --- a/src/test/py/bazel/bzlmod/bazel_module_test.py +++ b/src/test/py/bazel/bzlmod/bazel_module_test.py @@ -14,7 +14,6 @@ # limitations under the License. # pylint: disable=g-long-ternary -import json import os import pathlib import tempfile @@ -22,6 +21,7 @@ from src.test.py.bazel import test_base from src.test.py.bazel.bzlmod.test_utils import BazelRegistry +from src.test.py.bazel.bzlmod.test_utils import scratchFile class BazelModuleTest(test_base.TestBase): @@ -54,7 +54,7 @@ def writeBazelrcFile(self, allow_yanked_versions=True): [ # In ipv6 only network, this has to be enabled. # 'startup --host_jvm_args=-Djava.net.preferIPv6Addresses=true', - 'common --experimental_enable_bzlmod', + 'common --enable_bzlmod', 'common --registry=' + self.main_registry.getURL(), # We need to have BCR here to make sure built-in modules like # bazel_tools can work. @@ -544,14 +544,7 @@ def testAllowedYankedDepsSuccessMix(self): env_add={'BZLMOD_ALLOW_YANKED_VERSIONS': 'yanked2@1.0'}, allow_failure=False) - def setUpProjectWithLocalRegistryModule(self, dep_name, dep_version, - module_base_path): - bazel_registry = { - 'module_base_path': module_base_path, - } - with self.main_registry.root.joinpath('bazel_registry.json').open('w') as f: - json.dump(bazel_registry, f, indent=4, sort_keys=True) - + def setUpProjectWithLocalRegistryModule(self, dep_name, dep_version): self.main_registry.generateCcSource(dep_name, dep_version) self.main_registry.createLocalPathModule(dep_name, dep_version, dep_name + '/' + dep_version) @@ -575,15 +568,107 @@ def setUpProjectWithLocalRegistryModule(self, dep_name, dep_version, self.ScratchFile('WORKSPACE', []) def testLocalRepoInSourceJsonAbsoluteBasePath(self): - self.setUpProjectWithLocalRegistryModule('sss', '1.3', - str(self.main_registry.projects)) + self.main_registry.setModuleBasePath(str(self.main_registry.projects)) + self.setUpProjectWithLocalRegistryModule('sss', '1.3') _, stdout, _ = self.RunBazel(['run', '//:main'], allow_failure=False) self.assertIn('main function => sss@1.3', stdout) def testLocalRepoInSourceJsonRelativeBasePath(self): - self.setUpProjectWithLocalRegistryModule('sss', '1.3', 'projects') + self.main_registry.setModuleBasePath('projects') + self.setUpProjectWithLocalRegistryModule('sss', '1.3') _, stdout, _ = self.RunBazel(['run', '//:main'], allow_failure=False) self.assertIn('main function => sss@1.3', stdout) + def testRunfilesRepoMappingManifest(self): + self.main_registry.setModuleBasePath('projects') + projects_dir = self.main_registry.projects + + # Set up a "bare_rule" module that contains the "bare_binary" rule which + # passes runfiles along + self.main_registry.createLocalPathModule('bare_rule', '1.0', 'bare_rule') + projects_dir.joinpath('bare_rule').mkdir(exist_ok=True) + scratchFile(projects_dir.joinpath('bare_rule', 'WORKSPACE')) + scratchFile(projects_dir.joinpath('bare_rule', 'BUILD')) + scratchFile( + projects_dir.joinpath('bare_rule', 'defs.bzl'), [ + 'def _bare_binary_impl(ctx):', + ' exe = ctx.actions.declare_file(ctx.label.name)', + ' ctx.actions.write(exe,', + ' "#/bin/bash\\nif [[ ! -f \\"$0.repo_mapping\\" ]]; then\\necho >&2 \\"ERROR: cannot find repo mapping manifest file\\"\\nexit 1\\nfi",', + ' True)', + ' runfiles = ctx.runfiles(files=ctx.files.data)', + ' for data in ctx.attr.data:', + ' runfiles = runfiles.merge(data[DefaultInfo].default_runfiles)', + ' return DefaultInfo(files=depset(direct=[exe]), executable=exe, runfiles=runfiles)', + 'bare_binary=rule(', + ' implementation=_bare_binary_impl,', + ' attrs={"data":attr.label_list(allow_files=True)},', + ' executable=True,', + ')', + ]) + + # Now set up a project tree shaped like a diamond + self.ScratchFile('MODULE.bazel', [ + 'module(name="me",version="1.0")', + 'bazel_dep(name="foo",version="1.0")', + 'bazel_dep(name="bar",version="2.0")', + 'bazel_dep(name="bare_rule",version="1.0")', + ]) + self.ScratchFile('WORKSPACE') + self.ScratchFile('WORKSPACE.bzlmod', ['workspace(name="me_ws")']) + self.ScratchFile('BUILD', [ + 'load("@bare_rule//:defs.bzl", "bare_binary")', + 'bare_binary(name="me",data=["@foo"])', + ]) + self.main_registry.createLocalPathModule('foo', '1.0', 'foo', { + 'quux': '1.0', + 'bare_rule': '1.0' + }) + self.main_registry.createLocalPathModule('bar', '2.0', 'bar', { + 'quux': '2.0', + 'bare_rule': '1.0' + }) + self.main_registry.createLocalPathModule('quux', '1.0', 'quux1', + {'bare_rule': '1.0'}) + self.main_registry.createLocalPathModule('quux', '2.0', 'quux2', + {'bare_rule': '1.0'}) + for dir_name, build_file in [ + ('foo', 'bare_binary(name="foo",data=["@quux"])'), + ('bar', 'bare_binary(name="bar",data=["@quux"])'), + ('quux1', 'bare_binary(name="quux")'), + ('quux2', 'bare_binary(name="quux")'), + ]: + projects_dir.joinpath(dir_name).mkdir(exist_ok=True) + scratchFile(projects_dir.joinpath(dir_name, 'WORKSPACE')) + scratchFile( + projects_dir.joinpath(dir_name, 'BUILD'), [ + 'load("@bare_rule//:defs.bzl", "bare_binary")', + 'package(default_visibility=["//visibility:public"])', + build_file, + ]) + + # We use a shell script to check that the binary itself can see the repo + # mapping manifest. This obviously doesn't work on Windows, so we just build + # the target. TODO(wyv): make this work on Windows by using Batch. + bazel_command = 'build' if self.IsWindows() else 'run' + + # Finally we get to build stuff! + self.RunBazel([bazel_command, '//:me'], allow_failure=False) + with open(self.Path('bazel-bin/me.repo_mapping'), 'r') as f: + self.assertEqual( + f.read().strip(), """,foo,foo~1.0 +,me,_main +,me_ws,_main +foo~1.0,foo,foo~1.0 +foo~1.0,quux,quux~2.0 +quux~2.0,quux,quux~2.0""") + self.RunBazel([bazel_command, '@bar//:bar'], allow_failure=False) + with open(self.Path('bazel-bin/external/bar~2.0/bar.repo_mapping'), + 'r') as f: + self.assertEqual( + f.read().strip(), """bar~2.0,bar,bar~2.0 +bar~2.0,quux,quux~2.0 +quux~2.0,quux,quux~2.0""") + if __name__ == '__main__': unittest.main() diff --git a/src/test/py/bazel/bzlmod/test_utils.py b/src/test/py/bazel/bzlmod/test_utils.py index 786c1a30879fba..dd15b9082e2f72 100644 --- a/src/test/py/bazel/bzlmod/test_utils.py +++ b/src/test/py/bazel/bzlmod/test_utils.py @@ -90,6 +90,13 @@ def __init__(self, root, registry_suffix=''): self.archives.mkdir(parents=True, exist_ok=True) self.registry_suffix = registry_suffix + def setModuleBasePath(self, module_base_path): + bazel_registry = { + 'module_base_path': module_base_path, + } + with self.root.joinpath('bazel_registry.json').open('w') as f: + json.dump(bazel_registry, f, indent=4, sort_keys=True) + def getURL(self): """Return the URL of this registry.""" return self.root.resolve().as_uri() @@ -268,7 +275,7 @@ def addMetadata(self, return self - def createLocalPathModule(self, name, version, path): + def createLocalPathModule(self, name, version, path, deps=None): """Add a local module into the registry.""" module_dir = self.root.joinpath('modules', name, version) module_dir.mkdir(parents=True, exist_ok=True) @@ -279,13 +286,16 @@ def createLocalPathModule(self, name, version, path): 'path': path, } + if deps is None: + deps = {} + scratchFile( module_dir.joinpath('MODULE.bazel'), [ 'module(', ' name = "%s",' % name, ' version = "%s",' % version, ')', - ]) + ] + ['bazel_dep(name="%s",version="%s")' % p for p in deps.items()]) with module_dir.joinpath('source.json').open('w') as f: json.dump(source, f, indent=4, sort_keys=True)