Skip to content

Commit

Permalink
Flip --enable_bzlmod to true
Browse files Browse the repository at this point in the history
Bazel tests status:

- Bzlmod disabled:

  - AnalysisTestCase: to be migrated
  - ConfigurationTestCase: to be migrated
  - ConfigCommandTest: to be migrated, probably blocked by a bug

- Bzlmod enabled:

  - BuildViewTestCase: migrated at bazelbuild@d51144c
  - Java integration tests migrated at bazelbuild@8d04711
  - Shell integration tests migrated at bazelbuild@175a18d (Bzlmod disabled in some tests)
  - Python integration tests migrated at bazelbuild@50c8375 (Bzlmod disabled in some tests)
  - BuildIntegrationTestCase: migrated in this change
  - Other Java unit tests migrated in this change

Issues identified:

- cc_shared_library doesn't work well with Bzlmod: bazelbuild#19822
- `bazel config` doesn't work well with Bzlmod: bazelbuild#19823

Fixes bazelbuild#18958

Tracking migration of remaining test cases in bazelbuild#19824

RELNOTES[INC]: Bzlmod is enabled by default, please consider migrating your external dependencies from WORKSPACE to MODULE.bazel. Find more details at bazelbuild#18958

PiperOrigin-RevId: 573827480
Change-Id: I097b4bd7caafc996b034284ee688b8f3d2bca1f7
  • Loading branch information
meteorcloudy authored and copybara-github committed Oct 16, 2023
1 parent 659d7b2 commit 30d033c
Show file tree
Hide file tree
Showing 33 changed files with 323 additions and 136 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package com.google.devtools.build.lib.bazel;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.base.Supplier;
Expand Down Expand Up @@ -162,11 +163,19 @@ public class BazelRepositoryModule extends BlazeModule {

@Nullable private CredentialModule credentialModule;

private ImmutableMap<String, NonRegistryOverride> builtinModules = null;

public BazelRepositoryModule() {
this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager);
this.repositoryHandlers = repositoryRules();
}

@VisibleForTesting
public BazelRepositoryModule(ImmutableMap<String, NonRegistryOverride> builtinModules) {
this();
this.builtinModules = builtinModules;
}

private static DetailedExitCode detailedExitCode(String message, ExternalRepository.Code code) {
return DetailedExitCode.of(
FailureDetail.newBuilder()
Expand Down Expand Up @@ -234,37 +243,39 @@ public void workspaceInit(
singleExtensionEvalFunction =
new SingleExtensionEvalFunction(directories, clientEnvironmentSupplier, downloadManager);

ImmutableMap<String, NonRegistryOverride> builtinModules =
ImmutableMap.of(
// @bazel_tools is a special repo that we pull from the extracted install dir.
"bazel_tools",
LocalPathOverride.create(
directories.getEmbeddedBinariesRoot().getChild("embedded_tools").getPathString()),
// @local_config_platform is currently generated by the native repo rule
// local_config_platform
// It has to be a special repo for now because:
// - It's embedded in local_config_platform.WORKSPACE and depended on by many
// toolchains.
// - The canonical name "local_config_platform" is hardcoded in Bazel code.
// See {@link PlatformOptions}
"local_config_platform",
new NonRegistryOverride() {
@Override
public RepoSpec getRepoSpec(RepositoryName repoName) {
return RepoSpec.builder()
.setRuleClassName("local_config_platform")
.setAttributes(
AttributeValues.create(ImmutableMap.of("name", repoName.getName())))
.build();
}

@Override
public ResolutionReason getResolutionReason() {
// NOTE: It is not exactly a LOCAL_PATH_OVERRIDE, but there is no inspection
// ResolutionReason for builtin modules
return ResolutionReason.LOCAL_PATH_OVERRIDE;
}
});
if (builtinModules == null) {
builtinModules =
ImmutableMap.of(
// @bazel_tools is a special repo that we pull from the extracted install dir.
"bazel_tools",
LocalPathOverride.create(
directories.getEmbeddedBinariesRoot().getChild("embedded_tools").getPathString()),
// @local_config_platform is currently generated by the native repo rule
// local_config_platform
// It has to be a special repo for now because:
// - It's embedded in local_config_platform.WORKSPACE and depended on by many
// toolchains.
// - The canonical name "local_config_platform" is hardcoded in Bazel code.
// See {@link PlatformOptions}
"local_config_platform",
new NonRegistryOverride() {
@Override
public RepoSpec getRepoSpec(RepositoryName repoName) {
return RepoSpec.builder()
.setRuleClassName("local_config_platform")
.setAttributes(
AttributeValues.create(ImmutableMap.of("name", repoName.getName())))
.build();
}

@Override
public ResolutionReason getResolutionReason() {
// NOTE: It is not exactly a LOCAL_PATH_OVERRIDE, but there is no inspection
// ResolutionReason for builtin modules
return ResolutionReason.LOCAL_PATH_OVERRIDE;
}
});
}

builder
.addSkyFunction(SkyFunctions.REPOSITORY_DIRECTORY, repositoryDelegatorFunction)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

package com.google.devtools.build.lib.bazel.bzlmod;

import static com.google.common.collect.ImmutableMap.toImmutableMap;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -49,6 +51,32 @@ public static BazelDepGraphValue create(
extensionUniqueNames);
}

public static BazelDepGraphValue createEmptyDepGraph() {
Module root =
Module.builder()
.setName("")
.setVersion(Version.EMPTY)
.setRepoName("")
.setKey(ModuleKey.ROOT)
.setExtensionUsages(ImmutableList.of())
.setExecutionPlatformsToRegister(ImmutableList.of())
.setToolchainsToRegister(ImmutableList.of())
.build();

ImmutableMap<ModuleKey, Module> emptyDepGraph = ImmutableMap.of(ModuleKey.ROOT, root);

ImmutableMap<RepositoryName, ModuleKey> canonicalRepoNameLookup =
emptyDepGraph.keySet().stream()
.collect(toImmutableMap(ModuleKey::getCanonicalRepoName, key -> key));

return BazelDepGraphValue.create(
emptyDepGraph,
canonicalRepoNameLookup,
ImmutableList.of(),
ImmutableTable.of(),
ImmutableMap.of());
}

/**
* The post-selection dep graph. Must have BFS iteration order, starting from the root module. For
* any KEY in the returned map, it's guaranteed that {@code depGraph[KEY].getKey() == KEY}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public final class BuildLanguageOptions extends OptionsBase {
@Option(
name = "enable_bzlmod",
oldName = "experimental_enable_bzlmod",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = OptionEffectTag.LOADING_AND_ANALYSIS,
help =
Expand Down Expand Up @@ -821,7 +821,7 @@ public StarlarkSemantics toStarlarkSemantics() {
public static final String EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS =
"-experimental_enable_android_migration_apis";
public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "-experimental_enable_scl_dialect";
public static final String ENABLE_BZLMOD = "-enable_bzlmod";
public static final String ENABLE_BZLMOD = "+enable_bzlmod";
public static final String EXPERIMENTAL_ISOLATED_EXTENSION_USAGES =
"-experimental_isolated_extension_usages";
public static final String INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)

if (enableBzlmod) {
if (StarlarkBuiltinsValue.isBuiltinsRepo(repositoryName)) {
// If tools repo is not set, repo mapping for @_builtins should be always fallback.
if (ruleClassProvider.getToolsRepository() == null) {
return RepositoryMappingValue.createForWorkspaceRepo(RepositoryMapping.ALWAYS_FALLBACK);
}
// Builtins .bzl files should use the repo mapping of @bazel_tools, to get access to repos
// such as @platforms.
RepositoryMappingValue bazelToolsMapping =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics;
import com.google.devtools.build.lib.analysis.producers.ConfiguredTargetAndDataProducer;
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions;
import com.google.devtools.build.lib.bugreport.BugReporter;
Expand Down Expand Up @@ -672,6 +673,16 @@ private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions() {
throw new IllegalStateException("supposed to be unused");
});
map.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction(externalPackageHelper));
// Inject an empty default BAZEL_DEP_GRAPH SkyFunction for Blaze, it'll be overridden by
// BazelRepositoryModule in Bazel
map.put(
SkyFunctions.BAZEL_DEP_GRAPH,
new SkyFunction() {
@Override
public SkyValue compute(SkyKey skyKey, Environment env) {
return BazelDepGraphValue.createEmptyDepGraph();
}
});
map.put(
BzlmodRepoRuleValue.BZLMOD_REPO_RULE,
new BzlmodRepoRuleFunction(ruleClassProvider, directories));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ java_library(
":AbstractPackageLoader",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/bazel:repository_module",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:registry",
"//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/bazel/repository/cache",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.bazel.BazelRepositoryModule;
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphFunction;
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.ModuleFileFunction;
import com.google.devtools.build.lib.bazel.bzlmod.RegistryFactory;
import com.google.devtools.build.lib.bazel.bzlmod.RegistryFactoryImpl;
import com.google.devtools.build.lib.bazel.bzlmod.YankedVersionsUtil;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions;
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
Expand Down Expand Up @@ -98,7 +106,23 @@ private Builder(Root workspaceDir, Path installBase, Path outputBase, AtomicBool
RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE, Optional.empty()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY));
RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY),
PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, ImmutableMap.of()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_CONFIGURING,
RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY),
PrecomputedValue.injected(ModuleFileFunction.REGISTRIES, ImmutableList.of()),
PrecomputedValue.injected(ModuleFileFunction.IGNORE_DEV_DEPS, false),
PrecomputedValue.injected(
BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES,
RepositoryOptions.CheckDirectDepsMode.OFF),
PrecomputedValue.injected(
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE,
RepositoryOptions.BazelCompatibilityMode.OFF),
PrecomputedValue.injected(
BazelLockFileFunction.LOCKFILE_MODE, RepositoryOptions.LockfileMode.OFF),
PrecomputedValue.injected(
YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, ImmutableList.of()));
}

@Override
Expand All @@ -107,6 +131,19 @@ public BazelPackageLoader buildImpl() {
RepositoryCache repositoryCache = new RepositoryCache();
HttpDownloader httpDownloader = new HttpDownloader();
DownloadManager downloadManager = new DownloadManager(repositoryCache, httpDownloader);
RegistryFactory registryFactory =
new RegistryFactoryImpl(
directories.getWorkspace(), downloadManager, Suppliers.ofInstance(ImmutableMap.of()));

// Allow tests to override SkyFunctions.MODULE_FILE to use fake registry
if (!this.extraSkyFunctions.containsKey(SkyFunctions.MODULE_FILE)) {
addExtraSkyFunctions(
ImmutableMap.of(
SkyFunctions.MODULE_FILE,
new ModuleFileFunction(
registryFactory, directories.getWorkspace(), ImmutableMap.of())));
}

addExtraSkyFunctions(
ImmutableMap.<SkyFunctionName, SkyFunction>builder()
.put(
Expand All @@ -129,7 +166,9 @@ public BazelPackageLoader buildImpl() {
ImmutableMap::of,
directories,
EXTERNAL_PACKAGE_HELPER))
.build());
.put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction())
.put(SkyFunctions.BAZEL_MODULE_RESOLUTION, new BazelModuleResolutionFunction())
.buildOrThrow());

return new BazelPackageLoader(this);
}
Expand Down
1 change: 1 addition & 0 deletions src/main/starlark/tests/builtins_bzl/cc_builtin_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ msys*)
esac

# TODO: cc_shared_library doesn't work well with Bzlmod.
# https://github.com/bazelbuild/bazel/issues/19822
disable_bzlmod

function test_starlark_cc() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
config.create("protobuf_workspace/WORKSPACE");
config.create("protobuf_workspace/MODULE.bazel", "module(name='com_google_protobuf')");
config.overwrite("WORKSPACE", workspaceContents.toArray(new String[0]));
config.overwrite("MODULE.bazel");
/* The rest of platforms is initialized in {@link MockPlatformSupport}. */
config.create("platforms_workspace/WORKSPACE", "workspace(name = 'platforms')");
config.create("platforms_workspace/MODULE.bazel", "module(name = 'platforms')");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
import com.google.devtools.build.lib.bazel.bzlmod.RepoSpecFunction;
import com.google.devtools.build.lib.bazel.bzlmod.SingleExtensionEvalFunction;
import com.google.devtools.build.lib.bazel.bzlmod.SingleExtensionUsagesFunction;
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.bazel.repository.downloader.DownloadManager;
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction;
import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryFunction;
Expand All @@ -43,6 +47,7 @@
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.packages.PackageFactoryBuilderWithSkyframeForTesting;
import com.google.devtools.build.lib.testutil.TestConstants;
Expand All @@ -52,6 +57,7 @@
import java.io.IOException;
import java.lang.reflect.Field;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import org.mockito.Mockito;
Expand All @@ -69,6 +75,16 @@ public static AnalysisMock get() {
}
}

public static AnalysisMock getAnalysisMockWithoutBuiltinModules() {
return new AnalysisMock.Delegate(AnalysisMock.get()) {
@Override
public ImmutableMap<String, NonRegistryOverride> getBuiltinModules(
BlazeDirectories directories) {
return ImmutableMap.of();
}
};
}

@Override
public String getProductName() {
return TestConstants.PRODUCT_NAME;
Expand All @@ -82,15 +98,16 @@ public ImmutableList<String> getEmbeddedTools() {
public PackageFactoryBuilderWithSkyframeForTesting getPackageFactoryBuilderForTesting(
BlazeDirectories directories) {
return super.getPackageFactoryBuilderForTesting(directories)
.setExtraSkyFunctions(getSkyFunctions(directories));
.setExtraSkyFunctions(getSkyFunctions(directories))
.setExtraPrecomputeValues(getPrecomputedValues());
}

/**
* This is called from test setup to create the mock directory layout needed to create the
* configuration.
*/
public void setupMockClient(MockToolsConfig mockToolsConfig) throws IOException {
List<String> workspaceContents = getWorkspaceContents(mockToolsConfig);
ImmutableList<String> workspaceContents = getWorkspaceContents(mockToolsConfig);
setupMockClient(mockToolsConfig, workspaceContents);
}

Expand Down Expand Up @@ -165,6 +182,29 @@ public ImmutableMap<SkyFunctionName, SkyFunction> getSkyFunctions(BlazeDirectori
.buildOrThrow();
}

public ImmutableList<PrecomputedValue.Injected> getPrecomputedValues() {
// PrecomputedValues required by SkyFunctions in getSkyFunctions()
return ImmutableList.of(
PrecomputedValue.injected(PrecomputedValue.REPO_ENV, ImmutableMap.of()),
PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, ImmutableMap.of()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, ImmutableMap.of()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE, Optional.empty()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY),
PrecomputedValue.injected(ModuleFileFunction.REGISTRIES, ImmutableList.of()),
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));
}

// Allow subclasses to add extra repository functions.
public abstract void addExtraRepositoryFunctions(
ImmutableMap.Builder<String, RepositoryFunction> repositoryHandlers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,8 @@ public void useConfiguration(String... args) throws Exception {
}
if (defaultFlags().contains(Flag.ENABLE_BZLMOD)) {
optionsParser.parse("--enable_bzlmod");
} else {
optionsParser.parse("--noenable_bzlmod");
}
optionsParser.parse(args);

Expand Down
Loading

0 comments on commit 30d033c

Please sign in to comment.