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 d51144c
  - Java integration tests migrated at 8d04711
  - Shell integration tests migrated at 175a18d (Bzlmod disabled in some tests)
  - Python integration tests migrated at 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: #19822
- `bazel config` doesn't work well with Bzlmod: #19823

Fixes #18958

Tracking migration of remaining test cases in #19824

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

PiperOrigin-RevId: 573827480
Change-Id: I097b4bd7caafc996b034284ee688b8f3d2bca1f7
  • Loading branch information
meteorcloudy authored and Copybara-Service 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.
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
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
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
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
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
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
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
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
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
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
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

0 comments on commit 30d033c

Please sign in to comment.