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 546afe042eb40c..c8b032e3eb9730 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -995,9 +995,9 @@ java_library( "//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/packages", "//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", ], 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 index 59534d2c72f1be..11f5fe82022ebf 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RepoMappingManifestAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RepoMappingManifestAction.java @@ -13,69 +13,62 @@ // limitations under the License. package com.google.devtools.build.lib.analysis; -import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static com.google.common.collect.ImmutableSortedMap.toImmutableSortedMap; import static java.nio.charset.StandardCharsets.ISO_8859_1; import static java.util.Comparator.comparing; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedMap; +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.CommandLineItem.MapFn; 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.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; import com.google.devtools.build.lib.collect.nestedset.Order; -import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.util.Fingerprint; import java.io.PrintWriter; -import java.util.Map.Entry; +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 final class RepoMappingManifestAction extends AbstractFileWriteAction { - +public class RepoMappingManifestAction extends AbstractFileWriteAction { private static final UUID MY_UUID = UUID.fromString("458e351c-4d30-433d-b927-da6cddd4737f"); - // Uses MapFn's args parameter just like Fingerprint#addString to compute a cacheable fingerprint - // of just the repo name and mapping of a given Package. - private static final MapFn REPO_AND_MAPPING_DIGEST_FN = - (pkg, args) -> { - args.accept(pkg.getPackageIdentifier().getRepository().getName()); + private final ImmutableList entries; + private final String workspaceName; - var mapping = pkg.getRepositoryMapping().entries(); - args.accept(String.valueOf(mapping.size())); - mapping.forEach( - (apparentName, canonicalName) -> { - args.accept(apparentName); - args.accept(canonicalName.getName()); - }); - }; + /** 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); + } - private final NestedSet transitivePackages; - private final NestedSet runfilesArtifacts; - private final String workspaceName; + public abstract RepositoryName sourceRepo(); + + public abstract String targetRepoApparentName(); + + public abstract RepositoryName targetRepo(); + } public RepoMappingManifestAction( - ActionOwner owner, - Artifact output, - NestedSet transitivePackages, - NestedSet runfilesArtifacts, - String workspaceName) { + ActionOwner owner, Artifact output, List entries, String workspaceName) { super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, /*makeExecutable=*/ false); - this.transitivePackages = transitivePackages; - this.runfilesArtifacts = runfilesArtifacts; + this.entries = + ImmutableList.sortedCopyOf( + comparing((Entry e) -> e.sourceRepo().getName()) + .thenComparing(Entry::targetRepoApparentName), + entries); this.workspaceName = workspaceName; } @@ -86,7 +79,7 @@ public String getMnemonic() { @Override protected String getRawProgressMessage() { - return "Writing repo mapping manifest for " + getOwner().getLabel(); + return "writing repo mapping manifest for " + getOwner().getLabel(); } @Override @@ -96,61 +89,38 @@ protected void computeKey( Fingerprint fp) throws CommandLineExpansionException, EvalException, InterruptedException { fp.addUUID(MY_UUID); - actionKeyContext.addNestedSetToFingerprint(REPO_AND_MAPPING_DIGEST_FN, fp, transitivePackages); - actionKeyContext.addNestedSetToFingerprint(fp, runfilesArtifacts); 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); - - ImmutableSet reposContributingRunfiles = - runfilesArtifacts.toList().stream() - .filter(a -> a.getOwner() != null) - .map(a -> a.getOwner().getRepository()) - .collect(toImmutableSet()); - transitivePackages.toList().stream() - .collect( - toImmutableSortedMap( - comparing(RepositoryName::getName), - pkg -> pkg.getPackageIdentifier().getRepository(), - Package::getRepositoryMapping, - // All packages in a given repository have the same repository mapping, so the - // particular way of resolving duplicates does not matter. - (first, second) -> first)) - .forEach( - (repoName, mapping) -> - writeRepoMapping(writer, reposContributingRunfiles, repoName, mapping)); + 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(); }; } - - private void writeRepoMapping( - PrintWriter writer, - ImmutableSet reposContributingRunfiles, - RepositoryName repoName, - RepositoryMapping repoMapping) { - for (Entry mappingEntry : - ImmutableSortedMap.copyOf(repoMapping.entries()).entrySet()) { - if (mappingEntry.getKey().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. - continue; - } - if (!reposContributingRunfiles.contains(mappingEntry.getValue())) { - // We only write entries for repos that actually contribute runfiles. - 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 = - mappingEntry.getValue().isMain() ? workspaceName : mappingEntry.getValue().getName(); - writer.format( - "%s,%s,%s\n", repoName.getName(), mappingEntry.getKey(), targetRepoDirectoryName); - } - } } 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 b6154a2ed4d4a5..b9df741c78e422 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,20 +14,26 @@ 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; @@ -35,6 +41,7 @@ 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; @@ -139,6 +146,8 @@ private static RunfilesSupport create( runfilesInputManifest = null; runfilesManifest = null; } + Artifact repoMappingManifest = + createRepoMappingManifestAction(ruleContext, runfiles, owningExecutable); Artifact runfilesMiddleman = createRunfilesMiddleman( ruleContext, owningExecutable, runfiles, runfilesManifest, repoMappingManifest); @@ -551,9 +560,45 @@ private static Artifact createRepoMappingManifestAction( new RepoMappingManifestAction( ruleContext.getActionOwner(), repoMappingManifest, - ruleContext.getTransitivePackagesForRunfileRepoMappingManifest(), - runfiles.getAllArtifacts(), + 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 0246c930171776..bda9291a8a63df 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 @@ -70,7 +70,7 @@ private static RepositoryMapping createInternal( public RepositoryMapping withAdditionalMappings(Map additionalMappings) { HashMap allMappings = new HashMap<>(additionalMappings); allMappings.putAll(entries()); - return createInternal(allMappings, ownerRepo()); + return new AutoValue_RepositoryMapping(ImmutableMap.copyOf(allMappings), ownerRepo()); } /** 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 a7d493fe1de797..d89744dfd73bd8 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD @@ -361,14 +361,16 @@ java_test( srcs = ["RunfilesRepoMappingManifestTest.java"], deps = [ "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", + "//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/util", + "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/vfs", - "//src/main/java/net/starlark/java/eval", + "//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", 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 index 377164876ce25a..4c27d2bd2a4b44 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesRepoMappingManifestTest.java @@ -20,21 +20,16 @@ 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.actions.CommandLineExpansionException; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; -import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction; 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.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected; -import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.Path; import java.util.Map.Entry; -import net.starlark.java.eval.EvalException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -59,7 +54,6 @@ protected ImmutableList extraPrecomputedValues() throws Exception { BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES, CheckDirectDepsMode.WARNING), PrecomputedValue.injected( BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, BazelCompatibilityMode.ERROR), - PrecomputedValue.injected(BazelLockFileFunction.LOCKFILE_MODE, LockfileMode.OFF), PrecomputedValue.injected( BazelModuleResolutionFunction.ALLOWED_YANKED_VERSIONS, ImmutableList.of())); } @@ -98,22 +92,10 @@ public void setupBareBinaryRule() throws Exception { "bare_binary(name='bare_binary')"); } - private RepoMappingManifestAction getRepoMappingManifestActionForTarget(String label) - throws Exception { + private ImmutableList getRepoMappingManifestForTarget(String label) throws Exception { Action action = getGeneratingAction(getRunfilesSupport(label).getRepoMappingManifest()); assertThat(action).isInstanceOf(RepoMappingManifestAction.class); - return (RepoMappingManifestAction) action; - } - - private String computeKey(RepoMappingManifestAction action) - throws CommandLineExpansionException, EvalException, InterruptedException { - Fingerprint fp = new Fingerprint(); - action.computeKey(actionKeyContext, /* artifactExpander= */ null, fp); - return fp.hexDigestAndReset(); - } - - private ImmutableList getRepoMappingManifestForTarget(String label) throws Exception { - return getRepoMappingManifestActionForTarget(label) + return ((RepoMappingManifestAction) action) .newDeterministicWriter(null) .getBytes() .toStringUtf8() @@ -243,82 +225,4 @@ public void runfilesFromToolchain() throws Exception { "tooled_rule~1.0,bare_rule,bare_rule~1.0") .inOrder(); } - - @Test - public void actionRerunsOnRepoMappingChange_workspaceName() throws Exception { - rewriteWorkspace("workspace(name='aaa_ws')"); - scratch.overwriteFile( - "MODULE.bazel", - "module(name='aaa',version='1.0')", - "bazel_dep(name='bare_rule',version='1.0')"); - scratch.overwriteFile( - "BUILD", "load('@bare_rule//:defs.bzl', 'bare_binary')", "bare_binary(name='aaa')"); - - RepoMappingManifestAction actionBeforeChange = getRepoMappingManifestActionForTarget("//:aaa"); - - rewriteWorkspace("workspace(name='not_aaa_ws')"); - - RepoMappingManifestAction actionAfterChange = getRepoMappingManifestActionForTarget("//:aaa"); - assertThat(computeKey(actionBeforeChange)).isNotEqualTo(computeKey(actionAfterChange)); - } - - @Test - public void actionRerunsOnRepoMappingChange_repoName() throws Exception { - rewriteWorkspace("workspace(name='aaa_ws')"); - scratch.overwriteFile( - "MODULE.bazel", - "module(name='aaa',version='1.0')", - "bazel_dep(name='bare_rule',version='1.0')"); - scratch.overwriteFile( - "BUILD", "load('@bare_rule//:defs.bzl', 'bare_binary')", "bare_binary(name='aaa')"); - - RepoMappingManifestAction actionBeforeChange = getRepoMappingManifestActionForTarget("//:aaa"); - - scratch.overwriteFile( - "MODULE.bazel", - "module(name='aaa',version='1.0',repo_name='not_aaa')", - "bazel_dep(name='bare_rule',version='1.0')"); - invalidatePackages(); - - RepoMappingManifestAction actionAfterChange = getRepoMappingManifestActionForTarget("//:aaa"); - assertThat(computeKey(actionBeforeChange)).isNotEqualTo(computeKey(actionAfterChange)); - } - - @Test - public void actionRerunsOnRepoMappingChange_newEntry() throws Exception { - rewriteWorkspace("workspace(name='aaa_ws')"); - scratch.overwriteFile( - "MODULE.bazel", - "module(name='aaa',version='1.0')", - "bazel_dep(name='bare_rule',version='1.0')"); - scratch.overwriteFile( - "BUILD", "load('@bare_rule//:defs.bzl', 'bare_binary')", "bare_binary(name='aaa')"); - - registry.addModule( - createModuleKey("bbb", "1.0"), - "module(name='bbb',version='1.0')", - "bazel_dep(name='bare_rule',version='1.0')"); - scratch.overwriteFile( - moduleRoot.getRelative("bbb~1.0").getRelative("WORKSPACE").getPathString()); - scratch.overwriteFile(moduleRoot.getRelative("bbb~1.0").getRelative("BUILD").getPathString()); - scratch.overwriteFile( - moduleRoot.getRelative("bbb~1.0").getRelative("def.bzl").getPathString(), "BBB = '1'"); - - RepoMappingManifestAction actionBeforeChange = getRepoMappingManifestActionForTarget("//:aaa"); - - scratch.overwriteFile( - "MODULE.bazel", - "module(name='aaa',version='1.0')", - "bazel_dep(name='bbb',version='1.0')", - "bazel_dep(name='bare_rule',version='1.0')"); - scratch.overwriteFile( - "BUILD", - "load('@bare_rule//:defs.bzl', 'bare_binary')", - "load('@bbb//:def.bzl', 'BBB')", - "bare_binary(name='aaa')"); - invalidatePackages(); - - RepoMappingManifestAction actionAfterChange = getRepoMappingManifestActionForTarget("//:aaa"); - assertThat(computeKey(actionBeforeChange)).isNotEqualTo(computeKey(actionAfterChange)); - } } diff --git a/src/test/py/bazel/bzlmod/bazel_module_test.py b/src/test/py/bazel/bzlmod/bazel_module_test.py index 941823f11fb680..9c35677dd71fc5 100644 --- a/src/test/py/bazel/bzlmod/bazel_module_test.py +++ b/src/test/py/bazel/bzlmod/bazel_module_test.py @@ -292,6 +292,110 @@ def testDownload(self): ]) self.RunBazel(['build', '@no_op//:no_op'], allow_failure=False) + def testNonRegistryOverriddenModulesIgnoreYanked(self): + self.writeBazelrcFile(allow_yanked_versions=False) + src_yanked1 = self.main_registry.projects.joinpath('yanked1', '1.0') + self.ScratchFile('MODULE.bazel', [ + 'bazel_dep(name = "yanked1", version = "1.0")', 'local_path_override(', + ' module_name = "yanked1",', + ' path = "%s",' % str(src_yanked1.resolve()).replace('\\', '/'), ')' + ]) + self.ScratchFile('WORKSPACE') + self.ScratchFile('BUILD', [ + 'cc_binary(', + ' name = "main",', + ' srcs = ["main.cc"],', + ' deps = ["@yanked1//:lib_yanked1"],', + ')', + ]) + self.RunBazel(['build', '--nobuild', '//:main'], allow_failure=False) + + def testContainingYankedDepFails(self): + self.writeBazelrcFile(allow_yanked_versions=False) + self.ScratchFile('MODULE.bazel', [ + 'bazel_dep(name = "yanked1", version = "1.0")', + ]) + self.ScratchFile('WORKSPACE') + self.ScratchFile('BUILD', [ + 'cc_binary(', + ' name = "main",', + ' srcs = ["main.cc"],', + ' deps = ["@ddd//:lib_ddd"],', + ')', + ]) + exit_code, _, stderr = self.RunBazel(['build', '--nobuild', '//:main'], + allow_failure=True) + self.AssertExitCode(exit_code, 48, stderr) + self.assertIn( + 'Yanked version detected in your resolved dependency graph: ' + + 'yanked1@1.0, for the reason: dodgy.', ''.join(stderr)) + + def testAllowedYankedDepsSuccessByFlag(self): + self.writeBazelrcFile(allow_yanked_versions=False) + self.ScratchFile('MODULE.bazel', [ + 'bazel_dep(name = "ddd", version = "1.0")', + ]) + self.ScratchFile('WORKSPACE') + self.ScratchFile('BUILD', [ + 'cc_binary(', + ' name = "main",', + ' srcs = ["main.cc"],', + ' deps = ["@ddd//:lib_ddd"],', + ')', + ]) + self.RunBazel([ + 'build', '--nobuild', '--allow_yanked_versions=yanked1@1.0,yanked2@1.0', + '//:main' + ], + allow_failure=False) + + def testAllowedYankedDepsByEnvVar(self): + self.writeBazelrcFile(allow_yanked_versions=False) + self.ScratchFile('MODULE.bazel', [ + 'bazel_dep(name = "ddd", version = "1.0")', + ]) + self.ScratchFile('WORKSPACE') + self.ScratchFile('BUILD', [ + 'cc_binary(', + ' name = "main",', + ' srcs = ["main.cc"],', + ' deps = ["@ddd//:lib_ddd"],', + ')', + ]) + self.RunBazel( + ['build', '--nobuild', '//:main'], + env_add={'BZLMOD_ALLOW_YANKED_VERSIONS': 'yanked1@1.0,yanked2@1.0'}, + allow_failure=False) + + # Test changing the env var, the build should fail again. + exit_code, _, stderr = self.RunBazel( + ['build', '--nobuild', '//:main'], + env_add={'BZLMOD_ALLOW_YANKED_VERSIONS': 'yanked2@1.0'}, + allow_failure=True) + self.AssertExitCode(exit_code, 48, stderr) + self.assertIn( + 'Yanked version detected in your resolved dependency graph: ' + + 'yanked1@1.0, for the reason: dodgy.', ''.join(stderr)) + + def testAllowedYankedDepsSuccessMix(self): + self.writeBazelrcFile(allow_yanked_versions=False) + self.ScratchFile('MODULE.bazel', [ + 'bazel_dep(name = "ddd", version = "1.0")', + ]) + self.ScratchFile('WORKSPACE') + self.ScratchFile('BUILD', [ + 'cc_binary(', + ' name = "main",', + ' srcs = ["main.cc"],', + ' deps = ["@ddd//:lib_ddd"],', + ')', + ]) + self.RunBazel([ + 'build', '--nobuild', '--allow_yanked_versions=yanked1@1.0', '//:main' + ], + env_add={'BZLMOD_ALLOW_YANKED_VERSIONS': 'yanked2@1.0'}, + allow_failure=False) + def setUpProjectWithLocalRegistryModule(self, dep_name, dep_version): self.main_registry.generateCcSource(dep_name, dep_version) self.main_registry.createLocalPathModule(dep_name, dep_version, @@ -327,236 +431,96 @@ def testLocalRepoInSourceJsonRelativeBasePath(self): _, stdout, _ = self.RunBazel(['run', '//:main'], allow_failure=False) self.assertIn('main function => sss@1.3', stdout) - def testNativePackageRelativeLabel(self): - self.ScratchFile( - 'MODULE.bazel', - [ - 'module(name="foo")', - 'bazel_dep(name="bar")', - 'local_path_override(module_name="bar",path="bar")', - ], - ) - self.ScratchFile('WORKSPACE') - self.ScratchFile('BUILD') - self.ScratchFile( - 'defs.bzl', - [ - 'def mac(name):', - ' native.filegroup(name=name)', - ' print("1st: " + str(native.package_relative_label(":bleb")))', - ' print("2nd: " + str(native.package_relative_label(' - + '"//bleb:bleb")))', - ' print("3rd: " + str(native.package_relative_label(' - + '"@bleb//bleb:bleb")))', - ' print("4th: " + str(native.package_relative_label("//bleb")))', - ' print("5th: " + str(native.package_relative_label(' - + '"@@bleb//bleb:bleb")))', - ' print("6th: " + str(native.package_relative_label(Label(' - + '"//bleb"))))', - ], - ) - - self.ScratchFile( - 'bar/MODULE.bazel', - [ - 'module(name="bar")', - 'bazel_dep(name="foo", repo_name="bleb")', - ], - ) - self.ScratchFile('bar/WORKSPACE') - self.ScratchFile( - 'bar/quux/BUILD', - [ - 'load("@bleb//:defs.bzl", "mac")', - 'mac(name="book")', - ], - ) - - _, _, stderr = self.RunBazel( - ['build', '@bar//quux:book'], allow_failure=False - ) - stderr = '\n'.join(stderr) - self.assertIn('1st: @@bar~override//quux:bleb', stderr) - self.assertIn('2nd: @@bar~override//bleb:bleb', stderr) - self.assertIn('3rd: @@//bleb:bleb', stderr) - self.assertIn('4th: @@bar~override//bleb:bleb', stderr) - self.assertIn('5th: @@bleb//bleb:bleb', stderr) - self.assertIn('6th: @@//bleb:bleb', stderr) - - def testWorkspaceEvaluatedBzlCanSeeRootModuleMappings(self): - self.ScratchFile( - 'MODULE.bazel', - [ - 'bazel_dep(name="aaa",version="1.0")', - 'bazel_dep(name="bbb",version="1.0")', - ], - ) - self.ScratchFile( - 'WORKSPACE.bzlmod', - [ - 'local_repository(name="foo", path="foo", repo_mapping={', - ' "@bar":"@baz",', - ' "@my_aaa":"@aaa",', - '})', - 'load("@foo//:test.bzl", "test")', - 'test()', - ], - ) - self.ScratchFile('foo/WORKSPACE') - self.ScratchFile('foo/BUILD', ['filegroup(name="test")']) - self.ScratchFile( - 'foo/test.bzl', - [ - 'def test():', - ' print("1st: " + str(Label("@bar//:z")))', - ' print("2nd: " + str(Label("@my_aaa//:z")))', - ' print("3rd: " + str(Label("@bbb//:z")))', - ' print("4th: " + str(Label("@blarg//:z")))', - ], - ) - - _, _, stderr = self.RunBazel(['build', '@foo//:test'], allow_failure=False) - stderr = '\n'.join(stderr) - # @bar is mapped to @@baz, which Bzlmod doesn't recognize, so we leave it be - self.assertIn('1st: @@baz//:z', stderr) - # @my_aaa is mapped to @@aaa, which Bzlmod remaps to @@aaa~1.0 - self.assertIn('2nd: @@aaa~1.0//:z', stderr) - # @bbb isn't mapped in WORKSPACE, but Bzlmod maps it to @@bbb~1.0 - self.assertIn('3rd: @@bbb~1.0//:z', stderr) - # @blarg isn't mapped by WORKSPACE or Bzlmod - self.assertIn('4th: @@blarg//:z', stderr) - - def testWorkspaceItselfCanSeeRootModuleMappings(self): - self.ScratchFile( - 'MODULE.bazel', - [ - 'bazel_dep(name="hello")', - 'local_path_override(module_name="hello",path="hello")', - ], - ) - self.ScratchFile( - 'WORKSPACE.bzlmod', - [ - 'load("@hello//:world.bzl", "message")', - 'print(message)', - ], - ) - self.ScratchFile('BUILD', ['filegroup(name="a")']) - self.ScratchFile('hello/WORKSPACE') - self.ScratchFile('hello/BUILD') - self.ScratchFile('hello/MODULE.bazel', ['module(name="hello")']) - self.ScratchFile('hello/world.bzl', ['message="I LUV U!"']) - - _, _, stderr = self.RunBazel(['build', ':a'], allow_failure=False) - self.assertIn('I LUV U!', '\n'.join(stderr)) - - def testNativeModuleNameAndVersion(self): + def testRunfilesRepoMappingManifest(self): self.main_registry.setModuleBasePath('projects') projects_dir = self.main_registry.projects - self.ScratchFile( - 'MODULE.bazel', - [ - 'module(name="root",version="0.1")', - 'bazel_dep(name="foo",version="1.0")', - 'report_ext = use_extension("@foo//:ext.bzl", "report_ext")', - 'use_repo(report_ext, "report_repo")', - 'bazel_dep(name="bar")', - 'local_path_override(module_name="bar",path="bar")', - ], - ) - self.ScratchFile('WORKSPACE') - self.ScratchFile( - 'WORKSPACE.bzlmod', ['local_repository(name="quux",path="quux")'] - ) - self.ScratchFile( - 'BUILD', - [ - 'load("@foo//:report.bzl", "report")', - 'report()', - ], - ) - # foo: a repo defined by a normal Bazel module. Also hosts the extension - # `report_ext` which generates a repo `report_repo`. - self.main_registry.createLocalPathModule('foo', '1.0', 'foo') - projects_dir.joinpath('foo').mkdir(exist_ok=True) - scratchFile(projects_dir.joinpath('foo', 'WORKSPACE')) - scratchFile( - projects_dir.joinpath('foo', 'BUILD'), - [ - 'load(":report.bzl", "report")', - 'report()', - ], - ) + # 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('foo', 'report.bzl'), - [ - 'def report():', - ' repo = native.repository_name()', - ' name = str(native.module_name())', - ' version = str(native.module_version())', - ' print("@" + repo + " reporting in: " + name + "@" + version)', - ' native.filegroup(name="a")', - ], - ) - scratchFile( - projects_dir.joinpath('foo', 'ext.bzl'), - [ - 'def _report_repo(rctx):', - ' rctx.file("BUILD",', - ' "load(\\"@foo//:report.bzl\\", \\"report\\")\\n" +', - ' "report()")', - 'report_repo = repository_rule(_report_repo)', - 'report_ext = module_extension(', - ' lambda mctx: report_repo(name="report_repo"))', - ], - ) - # bar: a repo defined by a Bazel module with a non-registry override - self.ScratchFile('bar/WORKSPACE') - self.ScratchFile( - 'bar/MODULE.bazel', - [ - 'module(name="bar", version="2.0")', - 'bazel_dep(name="foo",version="1.0")', - ], - ) - self.ScratchFile( - 'bar/BUILD', - [ - 'load("@foo//:report.bzl", "report")', - 'report()', - ], - ) - # quux: a repo defined by WORKSPACE - self.ScratchFile('quux/WORKSPACE') - self.ScratchFile( - 'quux/BUILD', - [ - 'load("@foo//:report.bzl", "report")', - 'report()', - ], - ) + 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,', + ')', + ]) - _, _, stderr = self.RunBazel( - [ - 'build', - ':a', - '@foo//:a', - '@report_repo//:a', - '@bar//:a', - '@quux//:a', - ], - allow_failure=False, - ) - stderr = '\n'.join(stderr) - self.assertIn('@@ reporting in: root@0.1', stderr) - self.assertIn('@@foo~1.0 reporting in: foo@1.0', stderr) - self.assertIn( - '@@foo~1.0~report_ext~report_repo reporting in: foo@1.0', stderr - ) - self.assertIn('@@bar~override reporting in: bar@2.0', stderr) - self.assertIn('@@quux reporting in: None@None', stderr) + # 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()