Skip to content

Commit

Permalink
Turn on Bzlmod for all BuildViewTestCases
Browse files Browse the repository at this point in the history
Notable changes:

- No more injected PrecomputedValue boilerplate to enable Bzlmod; it's all built into BuildViewTestCase
- All "mocked" repos are turned into builtin modules

PiperOrigin-RevId: 573163786
Change-Id: I952bfa8b1cc964c2de4e90e58fb97140cd2475fd
  • Loading branch information
Wyverald authored and copybara-github committed Oct 13, 2023
1 parent cfaefb9 commit d51144c
Show file tree
Hide file tree
Showing 50 changed files with 342 additions and 620 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
Expand All @@ -40,6 +41,11 @@

/** {@link SkyFunction} for {@link RepositoryMappingValue}s. */
public class RepositoryMappingFunction implements SkyFunction {
private final RuleClassProvider ruleClassProvider;

public RepositoryMappingFunction(RuleClassProvider ruleClassProvider) {
this.ruleClassProvider = ruleClassProvider;
}

@Nullable
@Override
Expand All @@ -58,7 +64,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
// such as @platforms.
RepositoryMappingValue bazelToolsMapping =
(RepositoryMappingValue)
env.getValue(RepositoryMappingValue.key(RepositoryName.BAZEL_TOOLS));
env.getValue(
RepositoryMappingValue.Key.create(
ruleClassProvider.getToolsRepository(),
/* rootModuleShouldSeeWorkspaceRepos= */ false));
if (bazelToolsMapping == null) {
return null;
}
Expand All @@ -76,7 +85,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
RepositoryName.MAIN),
StarlarkBuiltinsValue.BUILTINS_REPO)
.withAdditionalMappings(bazelToolsMapping.getRepositoryMapping()),
"bazel_tools",
// The "associated module" doesn't exist here (@_builtins doesn't come from a module),
// so we just supply dummy values.
"",
Version.EMPTY);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions() {
map.put(SkyFunctions.REGISTERED_TOOLCHAINS, new RegisteredToolchainsFunction());
map.put(SkyFunctions.SINGLE_TOOLCHAIN_RESOLUTION, new SingleToolchainResolutionFunction());
map.put(SkyFunctions.TOOLCHAIN_RESOLUTION, new ToolchainResolutionFunction());
map.put(SkyFunctions.REPOSITORY_MAPPING, new RepositoryMappingFunction());
map.put(SkyFunctions.REPOSITORY_MAPPING, new RepositoryMappingFunction(ruleClassProvider));
map.put(SkyFunctions.RESOLVED_FILE, new ResolvedFileFunction());
map.put(
SkyFunctions.PLATFORM_MAPPING,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ public String getBaseNameForLoadedPackage(PackageIdentifier packageName) {
.put(
BzlmodRepoRuleValue.BZLMOD_REPO_RULE,
new BzlmodRepoRuleFunction(ruleClassProvider, directories))
.put(SkyFunctions.REPOSITORY_MAPPING, new RepositoryMappingFunction())
.put(SkyFunctions.REPOSITORY_MAPPING, new RepositoryMappingFunction(ruleClassProvider))
.put(
SkyFunctions.PACKAGE,
new PackageFunction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1830,8 +1830,8 @@ public void ccCommonLink_cppLTOActionExecutesOnFirstPlatform() throws Exception
mockToolsConfig,
CcToolchainConfig.builder()
.withFeatures(CppRuleClasses.THIN_LTO, CppRuleClasses.SUPPORTS_START_END_LIB)
.withToolchainTargetConstraints("@//platforms:constraint_1")
.withToolchainExecConstraints("@//platforms:constraint_1"));
.withToolchainTargetConstraints("@@//platforms:constraint_1")
.withToolchainExecConstraints("@@//platforms:constraint_1"));

ImmutableList<Action> actions = getActions("//test:custom_rule_name", CppLinkAction.class);
ImmutableList<Action> cppLTOActions =
Expand Down Expand Up @@ -2006,8 +2006,8 @@ public void ccCommonCompile_moduleActionsExecuteOnFirstPlatform() throws Excepti
mockToolsConfig,
CcToolchainConfig.builder()
.withFeatures(MockCcSupport.HEADER_MODULES_FEATURES)
.withToolchainTargetConstraints("@//platforms:constraint_1")
.withToolchainExecConstraints("@//platforms:constraint_1"));
.withToolchainTargetConstraints("@@//platforms:constraint_1")
.withToolchainExecConstraints("@@//platforms:constraint_1"));

ImmutableList<Action> cppCompileActions =
getActions("//bazel_internal/test_rules/cc:custom_rule_name", CppCompileAction.class);
Expand Down Expand Up @@ -2077,8 +2077,8 @@ public void ccCommonCompile_codeGenModuleActionExecutesOnFirstPlatform() throws
mockToolsConfig,
CcToolchainConfig.builder()
.withFeatures(MockCcSupport.HEADER_MODULES_FEATURES)
.withToolchainTargetConstraints("@//platforms:constraint_1")
.withToolchainExecConstraints("@//platforms:constraint_1"));
.withToolchainTargetConstraints("@@//platforms:constraint_1")
.withToolchainExecConstraints("@@//platforms:constraint_1"));

ImmutableList<Action> cppCompileActions =
getActions("//bazel_internal/test_rules/cc:custom_rule_name", CppCompileAction.class);
Expand Down Expand Up @@ -2143,8 +2143,8 @@ public void ccCommonCompile_compileHeaderActionExecutesOnFirstPlatform() throws
mockToolsConfig,
CcToolchainConfig.builder()
.withFeatures(CppRuleClasses.PARSE_HEADERS)
.withToolchainTargetConstraints("@//platforms:constraint_1")
.withToolchainExecConstraints("@//platforms:constraint_1"));
.withToolchainTargetConstraints("@@//platforms:constraint_1")
.withToolchainExecConstraints("@@//platforms:constraint_1"));

ImmutableList<Action> cppCompileActions =
getActions("//bazel_internal/test_rules/cc:custom_rule_name", CppCompileAction.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static com.google.common.truth.Truth.assertThat;

import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.Root;
Expand Down Expand Up @@ -138,21 +137,21 @@ public void otherPathExpansion() throws Exception {
assertThat(expander.expand("foo $(rootpaths :foo) bar"))
.matches("foo expansion/foo.txt bar");
assertThat(expander.expand("foo $(rlocationpath :foo) bar"))
.isEqualTo("foo workspace/expansion/foo.txt bar");
.isEqualTo("foo " + ruleClassProvider.getRunfilesPrefix() + "/expansion/foo.txt bar");
assertThat(expander.expand("foo $(rlocationpaths :foo) bar"))
.isEqualTo("foo workspace/expansion/foo.txt bar");
.isEqualTo("foo " + ruleClassProvider.getRunfilesPrefix() + "/expansion/foo.txt bar");
assertThat(expander.expand("foo $(rlocationpath //expansion:foo) bar"))
.isEqualTo("foo workspace/expansion/foo.txt bar");
.isEqualTo("foo " + ruleClassProvider.getRunfilesPrefix() + "/expansion/foo.txt bar");
assertThat(expander.expand("foo $(rlocationpaths //expansion:foo) bar"))
.isEqualTo("foo workspace/expansion/foo.txt bar");
.isEqualTo("foo " + ruleClassProvider.getRunfilesPrefix() + "/expansion/foo.txt bar");
assertThat(expander.expand("foo $(rlocationpath @//expansion:foo) bar"))
.isEqualTo("foo workspace/expansion/foo.txt bar");
.isEqualTo("foo " + ruleClassProvider.getRunfilesPrefix() + "/expansion/foo.txt bar");
assertThat(expander.expand("foo $(rlocationpaths @//expansion:foo) bar"))
.isEqualTo("foo workspace/expansion/foo.txt bar");
.isEqualTo("foo " + ruleClassProvider.getRunfilesPrefix() + "/expansion/foo.txt bar");
assertThat(expander.expand("foo $(rlocationpath @workspace//expansion:foo) bar"))
.isEqualTo("foo workspace/expansion/foo.txt bar");
.isEqualTo("foo " + ruleClassProvider.getRunfilesPrefix() + "/expansion/foo.txt bar");
assertThat(expander.expand("foo $(rlocationpaths @workspace//expansion:foo) bar"))
.isEqualTo("foo workspace/expansion/foo.txt bar");
.isEqualTo("foo " + ruleClassProvider.getRunfilesPrefix() + "/expansion/foo.txt bar");
}

@Test
Expand Down Expand Up @@ -262,6 +261,6 @@ public void otherPathMultiExpansion() throws Exception {
assertThat(expander.expand("foo $(rlocationpaths :foo) bar"))
.isEqualTo(
"foo __main__/expansion/bar.txt __main__/expansion/foo.txt bar"
.replace("__main__", TestConstants.WORKSPACE_NAME));
.replace("__main__", ruleClassProvider.getRunfilesPrefix()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,9 @@
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.bzlmod.YankedVersionsUtil;
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.rules.repository.RepositoryDirectoryDirtinessChecker;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected;
import com.google.devtools.build.lib.skyframe.SkyframeExecutorRepositoryHelpersHolder;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Map.Entry;
import net.starlark.java.eval.EvalException;
Expand All @@ -48,25 +37,6 @@
/** 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<Injected> 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(BazelLockFileFunction.LOCKFILE_MODE, LockfileMode.UPDATE),
PrecomputedValue.injected(YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, ImmutableList.of()));
}

@Override
protected SkyframeExecutorRepositoryHelpersHolder getRepositoryHelpersHolder() {
Expand All @@ -76,11 +46,6 @@ protected SkyframeExecutorRepositoryHelpersHolder getRepositoryHelpersHolder() {
new RepositoryDirectoryDirtinessChecker());
}

@Before
public void enableBzlmod() throws Exception {
setBuildLanguageOptions("--enable_bzlmod");
}

/**
* Sets up a Bazel module [email protected], which provides a bare_binary rule that passes along
* runfiles in the data attribute, and does nothing else.
Expand Down Expand Up @@ -269,6 +234,7 @@ public void actionRerunsOnRepoMappingChange_workspaceName() throws Exception {
"bazel_dep(name='bare_rule',version='1.0')");
scratch.overwriteFile(
"BUILD", "load('@bare_rule//:defs.bzl', 'bare_binary')", "bare_binary(name='aaa')");
invalidatePackages();

RepoMappingManifestAction actionBeforeChange = getRepoMappingManifestActionForTarget("//:aaa");

Expand All @@ -288,6 +254,7 @@ public void actionRerunsOnRepoMappingChange_repoName() throws Exception {
"bazel_dep(name='bare_rule',version='1.0')");
scratch.overwriteFile(
"BUILD", "load('@bare_rule//:defs.bzl', 'bare_binary')", "bare_binary(name='aaa')");
invalidatePackages();

RepoMappingManifestAction actionBeforeChange = getRepoMappingManifestActionForTarget("//:aaa");

Expand Down Expand Up @@ -320,6 +287,7 @@ public void actionRerunsOnRepoMappingChange_newEntry() throws Exception {
scratch.overwriteFile(moduleRoot.getRelative("bbb~1.0").getRelative("BUILD").getPathString());
scratch.overwriteFile(
moduleRoot.getRelative("bbb~1.0").getRelative("def.bzl").getPathString(), "BBB = '1'");
invalidatePackages();

RepoMappingManifestAction actionBeforeChange = getRepoMappingManifestActionForTarget("//:aaa");

Expand Down Expand Up @@ -391,6 +359,7 @@ public void hasMappingForSymlinks() throws Exception {
moduleRoot.getRelative("ddd~1.0/BUILD").getPathString(),
"load('@bare_rule//:defs.bzl', 'bare_binary')",
"bare_binary(name='ddd')");
invalidatePackages();

RunfilesSupport runfilesSupport = getRunfilesSupport("@aaa~1.0//:aaa");
ImmutableList<String> runfilesPaths =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ private void writeReadAndPassthroughOptionsTestFiles() throws Exception {
"settings_under_test = {",
" '//command_line_option:cpu': 'armeabi-v7a',",
" '//command_line_option:compilation_mode': 'dbg',",
" '//command_line_option:crosstool_top': '@//android/crosstool:everything',",
" '//command_line_option:crosstool_top': '@@//android/crosstool:everything',",
" '//command_line_option:platform_suffix': 'my-platform-suffix',",
"}",
"def set_options_transition_func(settings, attr):",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,18 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.analysis.util.DummyTestFragment;
import com.google.devtools.build.lib.analysis.util.DummyTestFragment.DummyTestOptions;
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.bzlmod.YankedVersionsUtil;
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.cmdline.Label;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleTransitionData;
import com.google.devtools.build.lib.rules.cpp.CppOptions;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import com.google.testing.junit.testparameterinjector.TestParameters;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand All @@ -60,28 +49,6 @@
/** Tests for StarlarkRuleTransitionProvider. */
@RunWith(TestParameterInjector.class)
public final class StarlarkRuleTransitionProviderTest extends BuildViewTestCase {
private FakeRegistry registry;

@Override
protected ImmutableList<Injected> extraPrecomputedValues() {
try {
registry =
FakeRegistry.DEFAULT_FACTORY.newFakeRegistry(scratch.dir("modules").getPathString());
} catch (IOException e) {
throw new IllegalStateException(e);
}
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(YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, ImmutableList.of()),
PrecomputedValue.injected(
BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES, CheckDirectDepsMode.WARNING),
PrecomputedValue.injected(
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, BazelCompatibilityMode.ERROR),
PrecomputedValue.injected(BazelLockFileFunction.LOCKFILE_MODE, LockfileMode.UPDATE));
}

@Override
protected ConfiguredRuleClassProvider createRuleClassProvider() {
Expand Down Expand Up @@ -1320,7 +1287,7 @@ public void successfulTypeConversionOfNativeListOption() throws Exception {

@Test
public void successfulTypeConversionOfNativeListOption_unambiguousLabels() throws Exception {
setBuildLanguageOptions("--enable_bzlmod", "--incompatible_unambiguous_label_stringification");
setBuildLanguageOptions("--incompatible_unambiguous_label_stringification");

scratch.overwriteFile("MODULE.bazel", "bazel_dep(name='rules_x',version='1.0')");
registry.addModule(createModuleKey("rules_x", "1.0"), "module(name='rules_x', version='1.0')");
Expand Down Expand Up @@ -1358,6 +1325,8 @@ public void successfulTypeConversionOfNativeListOption_unambiguousLabels() throw
"platform(name = 'my_platform')",
"my_rule(name = 'test')");

invalidatePackages();

getConfiguredTarget("//test");
assertNoEvents();
}
Expand Down
Loading

0 comments on commit d51144c

Please sign in to comment.