From 674ba8ae8665ed80389a330e5ca1ab54a337602a Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 12 Aug 2023 21:33:42 +0200 Subject: [PATCH] Add `bazel mod tidy` --- src/MODULE.tools | 5 + .../com/google/devtools/build/lib/bazel/BUILD | 1 + .../lib/bazel/BazelRepositoryModule.java | 34 +-- .../devtools/build/lib/bazel/bzlmod/BUILD | 62 +++++ .../lib/bazel/bzlmod/BazelLockFileModule.java | 2 + .../lib/bazel/bzlmod/BazelLockFileValue.java | 21 ++ .../bazel/bzlmod/BazelModTidyFunction.java | 119 ++++++++ .../lib/bazel/bzlmod/BazelModTidyValue.java | 51 ++++ .../bzlmod/BazelModuleResolutionFunction.java | 47 +--- .../build/lib/bazel/bzlmod/InterimModule.java | 43 +++ .../bazel/bzlmod/ModuleExtensionContext.java | 12 +- .../bazel/bzlmod/ModuleExtensionMetadata.java | 46 ++-- .../lib/bazel/bzlmod/ModuleFileFunction.java | 91 ++++-- .../bzlmod/RootModuleFileFixupEvent.java | 43 +++ .../bzlmod/SingleExtensionEvalFunction.java | 2 +- .../bazel/bzlmod/modcommand/ModOptions.java | 3 +- .../devtools/build/lib/bazel/commands/BUILD | 5 + .../build/lib/bazel/commands/ModCommand.java | 132 ++++++++- .../devtools/build/lib/bazel/commands/mod.txt | 4 +- .../build/lib/skyframe/SkyFunctions.java | 3 + src/main/protobuf/failure_details.proto | 2 + .../devtools/build/lib/bazel/bzlmod/BUILD | 1 + .../bzlmod/BazelLockFileFunctionTest.java | 14 +- .../bzlmod/ModuleExtensionResolutionTest.java | 115 +++++--- src/test/py/bazel/bzlmod/mod_command_test.py | 259 ++++++++++++++++++ src/test/tools/bzlmod/MODULE.bazel.lock | 124 ++++++++- 26 files changed, 1058 insertions(+), 183 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java create mode 100644 src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java create mode 100644 src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixupEvent.java diff --git a/src/MODULE.tools b/src/MODULE.tools index 3b3a4d70b0004c..f7c1fbf597cc2d 100644 --- a/src/MODULE.tools +++ b/src/MODULE.tools @@ -10,6 +10,7 @@ bazel_dep(name = "rules_license", version = "0.0.3") bazel_dep(name = "rules_proto", version = "4.0.0") bazel_dep(name = "rules_python", version = "0.22.1") +bazel_dep(name = "buildozer", version = "6.4.0.2") bazel_dep(name = "platforms", version = "0.0.7") bazel_dep(name = "protobuf", version = "3.19.6", repo_name = "com_google_protobuf") bazel_dep(name = "zlib", version = "1.3") @@ -42,5 +43,9 @@ use_repo(remote_coverage_tools_extension, "remote_coverage_tools") remote_android_extensions = use_extension("//tools/android:android_extensions.bzl", "remote_android_tools_extensions") use_repo(remote_android_extensions, "android_gmaven_r8", "android_tools") +# Used by bazel mod tidy (see BazelModTidyFunction). +buildozer_binary = use_extension("@buildozer//:buildozer_binary.bzl", "buildozer_binary") +use_repo(buildozer_binary, "buildozer_binary") + # Platforms used by transitions in builtins bazel_dep(name = "apple_support", version = "1.5.0", repo_name = "build_bazel_apple_support") diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/BUILD index 709c010c9ab11c..88d160d201d6ff 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD @@ -35,6 +35,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:registry", "//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/bzlmod:tidy_impl", "//src/main/java/com/google/devtools/build/lib/bazel/commands", "//src/main/java/com/google/devtools/build/lib/bazel/repository", "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 401e1759da7886..575e04ba9ca6d8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphFunction; import com.google.devtools.build.lib.bazel.bzlmod.BazelFetchAllFunction; import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction; +import com.google.devtools.build.lib.bazel.bzlmod.BazelModTidyFunction; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorFunction; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule.ResolutionReason; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction; @@ -249,37 +250,7 @@ public void workspaceInit( new SingleExtensionEvalFunction(directories, clientEnvironmentSupplier, downloadManager); 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; - } - }); + builtinModules = ModuleFileFunction.getBuiltinModules(directories.getEmbeddedBinariesRoot()); } builder @@ -291,6 +262,7 @@ public ResolutionReason getResolutionReason() { .addSkyFunction( SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace())) .addSkyFunction(SkyFunctions.BAZEL_FETCH_ALL, new BazelFetchAllFunction()) + .addSkyFunction(SkyFunctions.BAZEL_MOD_TIDY, new BazelModTidyFunction()) .addSkyFunction(SkyFunctions.BAZEL_MODULE_INSPECTION, new BazelModuleInspectorFunction()) .addSkyFunction(SkyFunctions.BAZEL_MODULE_RESOLUTION, new BazelModuleResolutionFunction()) .addSkyFunction(SkyFunctions.SINGLE_EXTENSION_EVAL, singleExtensionEvalFunction) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 3212cdc7545382..cc69b3740ff191 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -197,8 +197,10 @@ java_library( deps = [ ":common", ":exception", + ":inspection", ":module_extension", ":module_extension_metadata", + ":module_file_fixup_event", ":registry", ":repo_rule_creator", ":resolution", @@ -273,6 +275,51 @@ java_library( ], ) +java_library( + name = "tidy", + srcs = [ + "BazelModTidyValue.java", + ], + deps = [ + ":common", + ":resolution", + "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", + "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/main/java/net/starlark/java/eval", + "//third_party:auto_value", + "//third_party:guava", + ], +) + +java_library( + name = "tidy_impl", + srcs = [ + "BazelModTidyFunction.java", + ], + deps = [ + ":common", + ":exception", + ":module_extension", + ":resolution", + ":resolution_impl", + ":tidy", + "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", + "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/skyframe", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/main/java/net/starlark/java/eval", + "//src/main/protobuf:failure_details_java_proto", + "//third_party:guava", + "//third_party:jsr305", + ], +) + java_library( name = "inspection", srcs = [ @@ -314,6 +361,7 @@ java_library( deps = [ ":common", ":module_extension", + ":module_file_fixup_event", "//src/main/java/com/google/devtools/build/docgen/annot", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", @@ -326,3 +374,17 @@ java_library( "//third_party:jsr305", ], ) + +java_library( + name = "module_file_fixup_event", + srcs = [ + "RootModuleFileFixupEvent.java", + ], + deps = [ + ":module_extension", + "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/java/net/starlark/java/syntax", + "//third_party:auto_value", + "//third_party:guava", + ], +) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java index 6c3a69e5e92b6b..50fbd82feb5a07 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java @@ -213,6 +213,8 @@ public static void updateLockfile(Path workspaceRoot, BazelLockFileValue updated @Subscribe public void bazelModuleResolved(BazelModuleResolutionEvent moduleResolutionEvent) { + // Latest event wins, which is relevant in the case of `bazel mod tidy`, where a new event is + // sent after the command has modified the module file. this.moduleResolutionEvent = moduleResolutionEvent; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java index 8ac7fb38e77d64..e8563e8f601900 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.bazel.bzlmod; +import static com.google.devtools.build.lib.bazel.bzlmod.InterimModule.toModule; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; @@ -117,4 +118,24 @@ public ImmutableList getModuleAndFlagsDiff( } return moduleDiff.build(); } + + /** + * Returns a new BazelLockFileValue in which all information about the root module has been + * replaced by the given value. + * + *

This operation is shallow: If the new root module has different dependencies, the dep graph + * will not be updated. + */ + public BazelLockFileValue withShallowlyReplacedRootModule( + ModuleFileValue.RootModuleFileValue value) { + ImmutableMap.Builder newDepGraph = ImmutableMap.builder(); + newDepGraph.putAll(getModuleDepGraph()); + newDepGraph.put( + ModuleKey.ROOT, + toModule(value.getModule(), /* override= */ null, /* remoteRepoSpec= */ null)); + return toBuilder() + .setModuleFileHash(value.getModuleFileHash()) + .setModuleDepGraph(newDepGraph.buildKeepingLast()) + .build(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java new file mode 100644 index 00000000000000..c6b9d85c2e400a --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java @@ -0,0 +1,119 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package com.google.devtools.build.lib.bazel.bzlmod; + +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction.LOCKFILE_MODE; +import static com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction.IGNORE_DEV_DEPS; +import static com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction.MODULE_OVERRIDES; +import static com.google.devtools.build.lib.skyframe.PrecomputedValue.STARLARK_SEMANTICS; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.rules.repository.NeedsSkyframeRestartException; +import com.google.devtools.build.lib.rules.repository.RepositoryFunction; +import com.google.devtools.build.lib.server.FailureDetails; +import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; +import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.SkyframeLookupResult; +import javax.annotation.Nullable; +import net.starlark.java.eval.EvalException; + +/** + * Computes all information required for the {@code bazel mod tidy} command. The evaluation of all + * module extensions used by the root module is triggered to, as a side effect, emit any {@link + * RootModuleFileFixupEvent}s. + */ +public class BazelModTidyFunction implements SkyFunction { + + @Override + @Nullable + public SkyValue compute(SkyKey skyKey, Environment env) + throws InterruptedException, SkyFunctionException { + BazelDepGraphValue depGraphValue = (BazelDepGraphValue) env.getValue(BazelDepGraphValue.KEY); + if (depGraphValue == null) { + return null; + } + RepositoryMappingValue bazelToolsRepoMapping = + (RepositoryMappingValue) + env.getValue(RepositoryMappingValue.key(RepositoryName.BAZEL_TOOLS)); + if (bazelToolsRepoMapping == null) { + return null; + } + Label buildozerLabel; + try { + buildozerLabel = + Label.parseWithRepoContext( + "@buildozer_binary//:buildozer.exe", + Label.RepoContext.of( + RepositoryName.BAZEL_TOOLS, bazelToolsRepoMapping.getRepositoryMapping())); + } catch (LabelSyntaxException e) { + throw new IllegalStateException(e); + } + RootedPath buildozer; + try { + buildozer = RepositoryFunction.getRootedPathFromLabel(buildozerLabel, env); + } catch (NeedsSkyframeRestartException e) { + return null; + } catch (EvalException e) { + throw new IllegalStateException(e); + } + + ImmutableSet extensionsUsedByRootModule = + depGraphValue.getExtensionUsagesTable().columnMap().get(ModuleKey.ROOT).keySet().stream() + .map(SingleExtensionEvalValue::key) + .collect(toImmutableSet()); + SkyframeLookupResult result = env.getValuesAndExceptions(extensionsUsedByRootModule); + if (env.valuesMissing()) { + return null; + } + for (SkyKey extension : extensionsUsedByRootModule) { + try { + result.getOrThrow(extension, ExternalDepsException.class); + } catch (ExternalDepsException e) { + if (e.getDetailedExitCode().getFailureDetail() == null + || !e.getDetailedExitCode() + .getFailureDetail() + .getExternalDeps() + .getCode() + .equals(FailureDetails.ExternalDeps.Code.INVALID_EXTENSION_IMPORT)) { + throw new BazelModTidyFunctionException(e, SkyFunctionException.Transience.PERSISTENT); + } + // This is an error bazel mod tidy can fix, so don't fail. + } + } + + return BazelModTidyValue.create( + buildozer.asPath(), + MODULE_OVERRIDES.get(env), + IGNORE_DEV_DEPS.get(env), + LOCKFILE_MODE.get(env), + STARLARK_SEMANTICS.get(env)); + } + + static final class BazelModTidyFunctionException extends SkyFunctionException { + + BazelModTidyFunctionException(ExternalDepsException cause, Transience transience) { + super(cause, transience); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java new file mode 100644 index 00000000000000..f5ff1b1836c90c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java @@ -0,0 +1,51 @@ +package com.google.devtools.build.lib.bazel.bzlmod; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; +import com.google.devtools.build.lib.skyframe.SkyFunctions; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import java.util.Map; +import net.starlark.java.eval.StarlarkSemantics; + +/** All Skyframe information required for the {@code bazel mod tidy} command. */ +@AutoValue +public abstract class BazelModTidyValue implements SkyValue { + + @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_MOD_TIDY; + + /** The path of the buildozer binary provided by the "buildozer" module. */ + public abstract Path buildozer(); + + /** The value of {@link ModuleFileFunction#MODULE_OVERRIDES}. */ + public abstract ImmutableMap moduleOverrides(); + + /** The value of {@link ModuleFileFunction#IGNORE_DEV_DEPS}. */ + public abstract boolean ignoreDevDeps(); + + /** The value of {@link BazelLockFileFunction#LOCKFILE_MODE}. */ + public abstract LockfileMode lockfileMode(); + + /** + * The value of {@link + * com.google.devtools.build.lib.skyframe.PrecomputedValue#STARLARK_SEMANTICS}. + */ + public abstract StarlarkSemantics starlarkSemantics(); + + static BazelModTidyValue create( + Path buildozer, + Map moduleOverrides, + boolean ignoreDevDeps, + LockfileMode lockfileMode, + StarlarkSemantics starlarkSemantics) { + return new AutoValue_BazelModTidyValue( + buildozer, + ImmutableMap.copyOf(moduleOverrides), + ignoreDevDeps, + lockfileMode, + starlarkSemantics); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java index cf93fb2933e8de..0a7d086b5f522e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java @@ -20,10 +20,8 @@ import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.common.collect.ImmutableCollection; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; import com.google.devtools.build.lib.bazel.BazelVersion; @@ -266,49 +264,6 @@ private static void checkNoYankedVersions(ImmutableMap } } - private static RepoSpec maybeAppendAdditionalPatches( - @Nullable RepoSpec repoSpec, @Nullable ModuleOverride override) { - if (!(override instanceof SingleVersionOverride)) { - return repoSpec; - } - SingleVersionOverride singleVersion = (SingleVersionOverride) override; - if (singleVersion.getPatches().isEmpty()) { - return repoSpec; - } - ImmutableMap.Builder attrBuilder = ImmutableMap.builder(); - attrBuilder.putAll(repoSpec.attributes().attributes()); - attrBuilder.put("patches", singleVersion.getPatches()); - attrBuilder.put("patch_cmds", singleVersion.getPatchCmds()); - attrBuilder.put("patch_args", ImmutableList.of("-p" + singleVersion.getPatchStrip())); - return RepoSpec.builder() - .setBzlFile(repoSpec.bzlFile()) - .setRuleClassName(repoSpec.ruleClassName()) - .setAttributes(AttributeValues.create(attrBuilder.buildOrThrow())) - .build(); - } - - /** - * Builds a {@link Module} from an {@link InterimModule}, discarding unnecessary fields and adding - * extra necessary ones (such as the repo spec). - * - * @param remoteRepoSpec the {@link RepoSpec} for the module obtained from a registry or null if - * the module has a non-registry override - */ - static Module moduleFromInterimModule( - InterimModule interim, @Nullable ModuleOverride override, @Nullable RepoSpec remoteRepoSpec) { - return Module.builder() - .setName(interim.getName()) - .setVersion(interim.getVersion()) - .setKey(interim.getKey()) - .setRepoName(interim.getRepoName()) - .setExecutionPlatformsToRegister(interim.getExecutionPlatformsToRegister()) - .setToolchainsToRegister(interim.getToolchainsToRegister()) - .setDeps(ImmutableMap.copyOf(Maps.transformValues(interim.getDeps(), DepSpec::toModuleKey))) - .setRepoSpec(maybeAppendAdditionalPatches(remoteRepoSpec, override)) - .setExtensionUsages(interim.getExtensionUsages()) - .build(); - } - private static ImmutableMap computeFinalDepGraph( ImmutableMap resolvedDepGraph, ImmutableMap overrides, @@ -317,7 +272,7 @@ private static ImmutableMap computeFinalDepGraph( for (Map.Entry entry : resolvedDepGraph.entrySet()) { finalDepGraph.put( entry.getKey(), - moduleFromInterimModule( + InterimModule.toModule( entry.getValue(), overrides.get(entry.getKey().getName()), remoteRepoSpecs.get(entry.getKey()))); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java index b4a6a3d8fe50c7..cc70b04467e155 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java @@ -217,4 +217,47 @@ final InterimModule build() { return autoBuild(); } } + + /** + * Builds a {@link Module} from an {@link InterimModule}, discarding unnecessary fields and adding + * extra necessary ones (such as the repo spec). + * + * @param remoteRepoSpec the {@link RepoSpec} for the module obtained from a registry or null if + * the module has a non-registry override + */ + static Module toModule( + InterimModule interim, @Nullable ModuleOverride override, @Nullable RepoSpec remoteRepoSpec) { + return Module.builder() + .setName(interim.getName()) + .setVersion(interim.getVersion()) + .setKey(interim.getKey()) + .setRepoName(interim.getRepoName()) + .setExecutionPlatformsToRegister(interim.getExecutionPlatformsToRegister()) + .setToolchainsToRegister(interim.getToolchainsToRegister()) + .setDeps(ImmutableMap.copyOf(Maps.transformValues(interim.getDeps(), DepSpec::toModuleKey))) + .setRepoSpec(maybeAppendAdditionalPatches(remoteRepoSpec, override)) + .setExtensionUsages(interim.getExtensionUsages()) + .build(); + } + + private static RepoSpec maybeAppendAdditionalPatches( + @Nullable RepoSpec repoSpec, @Nullable ModuleOverride override) { + if (!(override instanceof SingleVersionOverride)) { + return repoSpec; + } + SingleVersionOverride singleVersion = (SingleVersionOverride) override; + if (singleVersion.getPatches().isEmpty()) { + return repoSpec; + } + ImmutableMap.Builder attrBuilder = ImmutableMap.builder(); + attrBuilder.putAll(repoSpec.attributes().attributes()); + attrBuilder.put("patches", singleVersion.getPatches()); + attrBuilder.put("patch_cmds", singleVersion.getPatchCmds()); + attrBuilder.put("patch_args", ImmutableList.of("-p" + singleVersion.getPatchStrip())); + return RepoSpec.builder() + .setBzlFile(repoSpec.bzlFile()) + .setRuleClassName(repoSpec.ruleClassName()) + .setAttributes(AttributeValues.create(attrBuilder.buildOrThrow())) + .build(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java index 087b91e7784a4e..0318ab1a55ca87 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java @@ -160,6 +160,9 @@ public boolean rootModuleHasNonDevDependency() { + " dependencies of the root module. If the root module imports additional" + " repositories or does not import all of these repositories via use_repo, Bazel" + + " will print a warning when the extension is evaluated, instructing the user" + + " to run bazel mod tidy to fix the use_repo calls" + + " automatically.

If one of root_module_direct_deps and" + " will print a warning and a fixup command when the extension is" + " evaluated.

If one of root_module_direct_deps and" + " root_module_direct_dev_deps is specified, the other has to be" @@ -185,9 +188,10 @@ public boolean rootModuleHasNonDevDependency() { + " href=\"../globals/module.html#use_repo\">use_repo on an" + " extension proxy created with use_extension(...," - + " dev_dependency = True), Bazel will print a warning and a fixup" - + " command when the extension is evaluated.

If one of" - + " root_module_direct_deps and" + + " dev_dependency = True), Bazel will print a warning when the " + + " extension is evaluated, instructing the user to run" + + " bazel mod tidy to fix the use_repo calls" + + " automatically.

If one of root_module_direct_deps and" + " root_module_direct_dev_deps is specified, the other has to be" + " as well. The lists specified by these two parameters must be" + " disjoint.

Exactly one of root_module_direct_deps and" @@ -207,6 +211,6 @@ public ModuleExtensionMetadata extensionMetadata( Object rootModuleDirectDepsUnchecked, Object rootModuleDirectDevDepsUnchecked) throws EvalException { return ModuleExtensionMetadata.create( - rootModuleDirectDepsUnchecked, rootModuleDirectDevDepsUnchecked, extensionId); + rootModuleDirectDepsUnchecked, rootModuleDirectDevDepsUnchecked); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java index 443a71c86b69db..f268df5f2b978a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java @@ -16,10 +16,10 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static java.util.stream.Collectors.joining; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; @@ -27,11 +27,13 @@ import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.events.Reportable; import com.ryanharter.auto.value.gson.GenerateTypeAdapter; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashSet; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -77,9 +79,7 @@ private static ModuleExtensionMetadata create( } static ModuleExtensionMetadata create( - Object rootModuleDirectDepsUnchecked, - Object rootModuleDirectDevDepsUnchecked, - ModuleExtensionId extensionId) + Object rootModuleDirectDepsUnchecked, Object rootModuleDirectDevDepsUnchecked) throws EvalException { if (rootModuleDirectDepsUnchecked == Starlark.NONE && rootModuleDirectDevDepsUnchecked == Starlark.NONE) { @@ -157,12 +157,12 @@ static ModuleExtensionMetadata create( } public void evaluate( - Collection usages, Set allRepos, EventHandler handler) + Collection usages, Set allRepos, ExtendedEventHandler handler) throws EvalException { - generateFixupMessage(usages, allRepos).ifPresent(handler::handle); + generateFixupMessage(usages, allRepos).forEach(reportable -> reportable.reportTo(handler)); } - Optional generateFixupMessage( + private List generateFixupMessage( Collection usages, Set allRepos) throws EvalException { var rootUsages = usages.stream() @@ -171,7 +171,7 @@ Optional generateFixupMessage( if (rootUsages.isEmpty()) { // The root module doesn't use the current extension. Do not suggest fixes as the user isn't // expected to modify any other module's MODULE.bazel file. - return Optional.empty(); + return ImmutableList.of(); } // Every module only has at most a single usage of a given extension. ModuleExtensionUsage rootUsage = Iterables.getOnlyElement(rootUsages); @@ -179,7 +179,7 @@ Optional generateFixupMessage( var rootModuleDirectDevDeps = getRootModuleDirectDevDeps(allRepos); var rootModuleDirectDeps = getRootModuleDirectDeps(allRepos); if (rootModuleDirectDevDeps.isEmpty() && rootModuleDirectDeps.isEmpty()) { - return Optional.empty(); + return ImmutableList.of(); } Preconditions.checkState( rootModuleDirectDevDeps.isPresent() && rootModuleDirectDeps.isPresent()); @@ -199,7 +199,7 @@ Optional generateFixupMessage( rootUsage, allRepos, rootModuleDirectDeps.get(), rootModuleDirectDevDeps.get()); } - private static Optional generateFixupMessage( + private static List generateFixupMessage( ModuleExtensionUsage rootUsage, Set allRepos, Set expectedImports, @@ -226,7 +226,7 @@ private static Optional generateFixupMessage( && importsToRemove.isEmpty() && devImportsToAdd.isEmpty() && devImportsToRemove.isEmpty()) { - return Optional.empty(); + return ImmutableList.of(); } var message = @@ -291,7 +291,9 @@ private static Optional generateFixupMessage( String.join(", ", indirectDepImports)); } - var fixupCommand = + message += "Fix the use_repo calls by running 'bazel mod tidy'."; + + var buildozerCommands = Stream.of( makeUseRepoCommand( "use_repo_add", @@ -322,17 +324,10 @@ private static Optional generateFixupMessage( extensionName, rootUsage.getIsolationKey())) .flatMap(Optional::stream) - .collect(joining(" ", "buildozer ", " //MODULE.bazel:all")); - - return Optional.of( - Event.warn( - location, - message - + String.format( - "%s ** You can use the following buildozer command to fix these" - + " issues:%s\n\n" - + "%s", - "\033[35m\033[1m", "\033[0m", fixupCommand))); + .collect(toImmutableList()); + + return ImmutableList.of( + Event.warn(location, message), new RootModuleFileFixupEvent(buildozerCommands, rootUsage)); } private static Optional makeUseRepoCommand( @@ -342,7 +337,6 @@ private static Optional makeUseRepoCommand( String extensionBzlFile, String extensionName, Optional isolationKey) { - if (repos.isEmpty()) { return Optional.empty(); } @@ -359,7 +353,7 @@ private static Optional makeUseRepoCommand( commandParts.add(extensionName); } commandParts.addAll(repos); - return Optional.of(commandParts.stream().collect(joining(" ", "'", "'"))); + return Optional.of(String.join(" ", commandParts)); } private Optional> getRootModuleDirectDeps(Set allRepos) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index f241e483389238..fbb5a382bb0caa 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.DotBazelFileSyntaxChecker; import com.google.devtools.build.lib.packages.StarlarkExportable; import com.google.devtools.build.lib.profiler.Profiler; @@ -160,10 +161,11 @@ public SkyValue compute(SkyKey skyKey, Environment env) moduleKey, // Dev dependencies should always be ignored if the current module isn't the root module /* ignoreDevDeps= */ true, + builtinModules, // We try to prevent most side effects of yanked modules, in particular print(). /* printIsNoop= */ getModuleFileResult.yankedInfo != null, starlarkSemantics, - env); + env.getListener()); // Perform some sanity checks. InterimModule module; @@ -211,9 +213,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) @Nullable private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Environment env) throws ModuleFileFunctionException, InterruptedException { - RootedPath moduleFilePath = - RootedPath.toRootedPath( - Root.fromPath(workspaceRoot), LabelConstants.MODULE_DOT_BAZEL_FILE_NAME); + RootedPath moduleFilePath = getModuleFilePath(workspaceRoot); if (env.getValue(FileValue.key(moduleFilePath)) == null) { return null; } @@ -232,21 +232,46 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir + " more details, please refer to" + " https://github.com/bazelbuild/bazel/issues/18958.")); } + return evaluateRootModuleFile( + moduleFileContents, + moduleFilePath, + builtinModules, + MODULE_OVERRIDES.get(env), + IGNORE_DEV_DEPS.get(env), + starlarkSemantics, + env.getListener()); + } + + public static RootedPath getModuleFilePath(Path workspaceRoot) { + return RootedPath.toRootedPath( + Root.fromPath(workspaceRoot), LabelConstants.MODULE_DOT_BAZEL_FILE_NAME); + } + + public static RootModuleFileValue evaluateRootModuleFile( + byte[] moduleFileContents, + RootedPath moduleFilePath, + ImmutableMap builtinModules, + Map commandOverrides, + boolean ignoreDevDeps, + StarlarkSemantics starlarkSemantics, + ExtendedEventHandler eventHandler) + throws ModuleFileFunctionException, InterruptedException { String moduleFileHash = new Fingerprint().addBytes(moduleFileContents).hexDigestAndReset(); ModuleFileGlobals moduleFileGlobals = execModuleFile( ModuleFile.create(moduleFileContents, moduleFilePath.asPath().toString()), /* registry= */ null, ModuleKey.ROOT, - /* ignoreDevDeps= */ Objects.requireNonNull(IGNORE_DEV_DEPS.get(env)), - /* printIsNoop= */ false, + ignoreDevDeps, + /* printIsNoop= */ builtinModules, + false, starlarkSemantics, - env); + eventHandler); InterimModule module; try { module = moduleFileGlobals.buildModule(); } catch (EvalException e) { - env.getListener().handle(Event.error(e.getMessageWithStack())); + eventHandler.handle(Event.error(e.getMessageWithStack())); throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for the root module"); } for (ModuleExtensionUsage usage : module.getExtensionUsages()) { @@ -263,7 +288,6 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir } ImmutableMap moduleOverrides = moduleFileGlobals.buildOverrides(); - Map commandOverrides = MODULE_OVERRIDES.get(env); ImmutableMap overrides = ImmutableMap.builder() .putAll(moduleOverrides) @@ -287,19 +311,20 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir module, moduleFileHash, overrides, nonRegistryOverrideCanonicalRepoNameLookup); } - private ModuleFileGlobals execModuleFile( + private static ModuleFileGlobals execModuleFile( ModuleFile moduleFile, @Nullable Registry registry, ModuleKey moduleKey, boolean ignoreDevDeps, + ImmutableMap builtinModules, boolean printIsNoop, StarlarkSemantics starlarkSemantics, - Environment env) + ExtendedEventHandler eventHandler) throws ModuleFileFunctionException, InterruptedException { StarlarkFile starlarkFile = StarlarkFile.parse(ParserInput.fromUTF8(moduleFile.getContent(), moduleFile.getLocation())); if (!starlarkFile.ok()) { - Event.replayEventsOn(env.getListener(), starlarkFile.errors()); + Event.replayEventsOn(eventHandler, starlarkFile.errors()); throw errorf(Code.BAD_MODULE, "error parsing MODULE.bazel file for %s", moduleKey); } @@ -318,23 +343,23 @@ private ModuleFileGlobals execModuleFile( if (printIsNoop) { thread.setPrintHandler((t, msg) -> {}); } else { - thread.setPrintHandler(Event.makeDebugPrintHandler(env.getListener())); + thread.setPrintHandler(Event.makeDebugPrintHandler(eventHandler)); } thread.setPostAssignHook( (name, value) -> { if (value instanceof StarlarkExportable) { StarlarkExportable exportable = (StarlarkExportable) value; if (!exportable.isExported()) { - exportable.export(env.getListener(), null, name); + exportable.export(eventHandler, null, name); } } }); Starlark.execFileProgram(program, predeclaredEnv, thread); } catch (SyntaxError.Exception e) { - Event.replayEventsOn(env.getListener(), e.errors()); + Event.replayEventsOn(eventHandler, e.errors()); throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for %s", moduleKey); } catch (EvalException e) { - env.getListener().handle(Event.error(e.getMessageWithStack())); + eventHandler.handle(Event.error(e.getMessageWithStack())); throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for %s", moduleKey); } return moduleFileGlobals; @@ -463,7 +488,7 @@ private static void createModuleFile(Path path, byte[] bytes) throws ModuleFileF } } - private net.starlark.java.eval.Module getPredeclaredEnv( + private static net.starlark.java.eval.Module getPredeclaredEnv( ModuleFileGlobals moduleFileGlobals, StarlarkSemantics starlarkSemantics) { ImmutableMap.Builder env = ImmutableMap.builder(); Starlark.addMethods(env, moduleFileGlobals, starlarkSemantics); @@ -492,4 +517,36 @@ static final class ModuleFileFunctionException extends SkyFunctionException { super(cause, transience); } } + + public static ImmutableMap getBuiltinModules( + Path embeddedBinariesRoot) { + return ImmutableMap.of( + // @bazel_tools is a special repo that we pull from the extracted install dir. + "bazel_tools", + LocalPathOverride.create(embeddedBinariesRoot.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 BazelModuleInspectorValue.AugmentedModule.ResolutionReason getResolutionReason() { + // NOTE: It is not exactly a LOCAL_PATH_OVERRIDE, but there is no inspection + // ResolutionReason for builtin modules + return BazelModuleInspectorValue.AugmentedModule.ResolutionReason.LOCAL_PATH_OVERRIDE; + } + }); + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixupEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixupEvent.java new file mode 100644 index 00000000000000..52635ae09dc436 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixupEvent.java @@ -0,0 +1,43 @@ +package com.google.devtools.build.lib.bazel.bzlmod; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.events.ExtendedEventHandler; +import java.util.List; + +/** + * Generated when incorrect use_repo calls are detected in the root module file according to {@link + * ModuleExtensionMetadata}, this event contains the buildozer commands required to bring the root + * module file into the expected state. + */ +public final class RootModuleFileFixupEvent implements ExtendedEventHandler.Postable { + private final ImmutableList buildozerCommands; + private final ModuleExtensionUsage usage; + + public RootModuleFileFixupEvent(List buildozerCommands, ModuleExtensionUsage usage) { + this.buildozerCommands = ImmutableList.copyOf(buildozerCommands); + this.usage = usage; + } + + /** The buildozer commands required to bring the root module file into the expected state. */ + public ImmutableList getBuildozerCommands() { + return buildozerCommands; + } + + /** A human-readable message describing the fixup after it has been applied. */ + public String getSuccessMessage() { + String extensionId = usage.getExtensionBzlFile() + "%" + usage.getExtensionName(); + return usage + .getIsolationKey() + .map( + key -> + String.format( + "Updated use_repo calls for isolated usage '%s' of %s", + key.getUsageExportedName(), extensionId)) + .orElseGet(() -> String.format("Updated use_repo calls for %s", extensionId)); + } + + @Override + public boolean storeForReplay() { + return true; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index 291a37ab6747a2..e4b2db56d4d9b6 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -466,7 +466,7 @@ private SingleExtensionEvalValue validateAndCreateSingleExtensionEvalValue( if (!generatedRepoSpecs.containsKey(repoImport.getValue())) { throw new SingleExtensionEvalFunctionException( ExternalDepsException.withMessage( - Code.BAD_MODULE, + Code.INVALID_EXTENSION_IMPORT, "module extension \"%s\" from \"%s\" does not generate repository \"%s\", yet it" + " is imported as \"%s\" in the usage at %s%s", extensionId.getExtensionName(), diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModOptions.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModOptions.java index bac6b42d6b9c48..3a4dd279765be3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModOptions.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModOptions.java @@ -173,7 +173,8 @@ public enum ModSubcommand { EXPLAIN(true), SHOW_REPO(false), SHOW_EXTENSION(false), - DUMP_REPO_MAPPING(false); + DUMP_REPO_MAPPING(false), + TIDY(false); /** Whether this subcommand produces a graph output. */ private final boolean isGraph; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD index 4a11300519be3b..1987fcaea7cf5c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD @@ -33,8 +33,11 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_file_fixup_event", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value", "//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/bzlmod:tidy", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand", "//src/main/java/com/google/devtools/build/lib/bazel/repository", "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", @@ -52,11 +55,13 @@ java_library( "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", "//src/main/java/com/google/devtools/build/lib/rules:repository/resolved_file_value", "//src/main/java/com/google/devtools/build/lib/runtime/commands", + "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", + "//src/main/java/com/google/devtools/build/lib/util:command", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/util:interrupted_failure_details", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java index ec6d9d3caa8158..7a683bdb23b6a2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java @@ -28,13 +28,19 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; +import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.analysis.NoBuildEvent; import com.google.devtools.build.lib.analysis.NoBuildRequestFinishedEvent; import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue; +import com.google.devtools.build.lib.bazel.bzlmod.BazelModTidyValue; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule; +import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionEvent; import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue; import com.google.devtools.build.lib.bazel.bzlmod.ModuleExtensionId; +import com.google.devtools.build.lib.bazel.bzlmod.RootModuleFileFixupEvent; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue; import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ExtensionArg; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ExtensionArg.ExtensionArgConverter; @@ -46,6 +52,7 @@ import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ModuleArg; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ModuleArg.ModuleArgConverter; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; @@ -60,13 +67,19 @@ import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.ModCommand.Code; import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; +import com.google.devtools.build.lib.shell.AbnormalTerminationException; +import com.google.devtools.build.lib.shell.CommandException; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.util.AbruptExitException; +import com.google.devtools.build.lib.util.CommandBuilder; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.InterruptedFailureDetails; import com.google.devtools.build.lib.util.MaybeCompleteSet; +import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.EvaluationContext; import com.google.devtools.build.skyframe.EvaluationResult; +import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.common.options.OptionPriority.PriorityCategory; @@ -79,11 +92,15 @@ import java.io.IOException; import java.io.OutputStreamWriter; import java.io.Writer; +import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.stream.IntStream; +import java.util.stream.Stream; +import javax.annotation.Nullable; /** Queries the Bzlmod external dependency graph. */ @Command( @@ -102,6 +119,9 @@ public final class ModCommand implements BlazeCommand { public static final String NAME = "mod"; + private final ArrayList fixupEvents = new ArrayList<>(); + @Nullable private BazelModuleResolutionEvent bazelModuleResolutionEvent = null; + @Override public void editOptions(OptionsParser optionsParser) { try { @@ -117,6 +137,7 @@ public void editOptions(OptionsParser optionsParser) { @Override public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult options) { + env.getEventBus().register(this); env.getEventBus() .post( new NoBuildEvent( @@ -125,7 +146,13 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti /* separateFinishedEvent= */ true, /* showProgress= */ true, /* id= */ null)); - BlazeCommandResult result = execInternal(env, options); + BlazeCommandResult result; + try { + result = execInternal(env, options); + } finally { + fixupEvents.clear(); + bazelModuleResolutionEvent = null; + } env.getEventBus() .post( new NoBuildRequestFinishedEvent( @@ -177,7 +204,8 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe repositoryMappingKeysBuilder.build(); BazelDepGraphValue depGraphValue; - BazelModuleInspectorValue moduleInspector; + @Nullable BazelModuleInspectorValue moduleInspector; + @Nullable BazelModTidyValue modTidyValue; ImmutableList repoMappingValues; SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor(); @@ -195,6 +223,8 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe ImmutableSet.Builder keys = ImmutableSet.builder(); if (subcommand.equals(ModSubcommand.DUMP_REPO_MAPPING)) { keys.addAll(repoMappingKeys); + } else if (subcommand.equals(ModSubcommand.TIDY)) { + keys.add(BazelModTidyValue.KEY); } else { keys.add(BazelDepGraphValue.KEY, BazelModuleInspectorValue.KEY); } @@ -215,6 +245,8 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe moduleInspector = (BazelModuleInspectorValue) evaluationResult.get(BazelModuleInspectorValue.KEY); + modTidyValue = (BazelModTidyValue) evaluationResult.get(BazelModTidyValue.KEY); + repoMappingValues = repoMappingKeys.stream() .map(evaluationResult::get) @@ -230,6 +262,7 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe return BlazeCommandResult.detailedExitCode(e.getDetailedExitCode()); } + // Handle commands that do not require BazelModuleInspectorValue. if (subcommand.equals(ModSubcommand.DUMP_REPO_MAPPING)) { String missingRepos = IntStream.range(0, repoMappingKeys.size()) @@ -252,6 +285,13 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe throw new IllegalStateException(e); } return BlazeCommandResult.success(); + } else if (subcommand == ModSubcommand.TIDY) { + // tidy doesn't take extra arguments. + if (!args.isEmpty()) { + return reportAndCreateFailureResult( + env, "the 'fix' command doesn't take extra arguments", Code.TOO_MANY_ARGUMENTS); + } + return runTidy(env, modTidyValue); } // Extract and check the --base_module argument first to use it when parsing the other args. @@ -523,6 +563,84 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe return BlazeCommandResult.success(); } + private BlazeCommandResult runTidy(CommandEnvironment env, BazelModTidyValue modTidyValue) { + CommandBuilder buildozerCommand = + new CommandBuilder() + .setWorkingDir(env.getWorkspace()) + .addArg(modTidyValue.buildozer().getPathString()) + .addArgs( + Stream.concat( + fixupEvents.stream() + .map(RootModuleFileFixupEvent::getBuildozerCommands) + .flatMap(Collection::stream), + Stream.of("format")) + .collect(toImmutableList())) + .addArg("MODULE.bazel:all"); + try { + buildozerCommand.build().execute(); + } catch (InterruptedException | CommandException e) { + String suffix = ""; + if (e instanceof AbnormalTerminationException) { + suffix = + ":\n" + new String(((AbnormalTerminationException) e).getResult().getStderr(), UTF_8); + } + return reportAndCreateFailureResult( + env, + "Unexpected error while running buildozer: " + e.getMessage() + suffix, + Code.BUILDOZER_FAILED); + } + + for (RootModuleFileFixupEvent fixupEvent : fixupEvents) { + env.getReporter().handle(Event.info(fixupEvent.getSuccessMessage())); + } + + if (modTidyValue.lockfileMode().equals(LockfileMode.UPDATE)) { + // We cannot safely rerun Skyframe evaluation here to pick up the updated module file. + // Instead, we construct a new BazelModuleResolutionEvent with the updated module file + // contents to be picked up by BazelLockFileModule. Since changing use_repos doesn't affect + // module resolution or module extension evaluation, we can reuse the existing lockfile + // information except for the root module file value. + RootedPath moduleFilePath = ModuleFileFunction.getModuleFilePath(env.getWorkspace()); + byte[] moduleFileContents; + try { + moduleFileContents = FileSystemUtils.readContent(moduleFilePath.asPath()); + } catch (IOException e) { + return reportAndCreateFailureResult( + env, + "Unexpected error while reading module file after running buildozer: " + e.getMessage(), + Code.BUILDOZER_FAILED); + } + ModuleFileValue.RootModuleFileValue newRootModuleFileValue; + try { + newRootModuleFileValue = + ModuleFileFunction.evaluateRootModuleFile( + moduleFileContents, + moduleFilePath, + ModuleFileFunction.getBuiltinModules( + env.getDirectories().getEmbeddedBinariesRoot()), + modTidyValue.moduleOverrides(), + modTidyValue.ignoreDevDeps(), + modTidyValue.starlarkSemantics(), + env.getReporter()); + } catch (SkyFunctionException | InterruptedException e) { + return reportAndCreateFailureResult( + env, + "Unexpected error parsing module file after running buildozer: " + e.getMessage(), + Code.BUILDOZER_FAILED); + } + BazelModuleResolutionEvent updatedModuleResolutionEvent = + BazelModuleResolutionEvent.create( + bazelModuleResolutionEvent.getOnDiskLockfileValue(), + bazelModuleResolutionEvent + .getResolutionOnlyLockfileValue() + .withShallowlyReplacedRootModule(newRootModuleFileValue), + bazelModuleResolutionEvent.getExtensionUsagesById()); + env.getReporter().post(updatedModuleResolutionEvent); + } + + return BlazeCommandResult.success(); + } + /** Collects a list of {@link ModuleArg} into a set of {@link ModuleKey}s. */ private static ImmutableSet moduleArgListToKeys( ImmutableList argList, @@ -592,4 +710,14 @@ public static void dumpRepoMappings(List repoMappings, W } writer.flush(); } + + @Subscribe + public void fixupGenerated(RootModuleFileFixupEvent event) { + fixupEvents.add(event); + } + + @Subscribe + public void bazelModuleResolved(BazelModuleResolutionEvent event) { + bazelModuleResolutionEvent = event; + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/mod.txt b/src/main/java/com/google/devtools/build/lib/bazel/commands/mod.txt index 8cdc0fdc1aa1ab..efd63df44df19b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/mod.txt +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/mod.txt @@ -1,5 +1,7 @@ -Usage: %{product} %{command} [