Skip to content

Commit

Permalink
Update lockfile flag from experimental to different modes flag:
Browse files Browse the repository at this point in the history
- Update: run resolution and update the lock file when it mismatches the module
- Error: throw an error if the module doesn't match the lockfile
- Off: don't read/update the lockfile

PiperOrigin-RevId: 524813416
Change-Id: I5cc3577fdbed8339ada50001081b75b4932c017c
  • Loading branch information
SalmaSamy authored and copybara-github committed Apr 17, 2023
1 parent 5b7e3a5 commit 3b11a2f
Show file tree
Hide file tree
Showing 27 changed files with 211 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions;
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.RepositoryOptions.RepositoryOverride;
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
import com.google.devtools.build.lib.bazel.repository.downloader.DelegatingDownloader;
Expand Down Expand Up @@ -141,6 +142,7 @@ public class BazelRepositoryModule extends BlazeModule {
private final AtomicBoolean ignoreDevDeps = new AtomicBoolean(false);
private CheckDirectDepsMode checkDirectDepsMode = CheckDirectDepsMode.WARNING;
private BazelCompatibilityMode bazelCompatibilityMode = BazelCompatibilityMode.ERROR;
private LockfileMode bazelLockfileMode = LockfileMode.OFF;
private List<String> allowedYankedVersions = ImmutableList.of();
private SingleExtensionEvalFunction singleExtensionEvalFunction;

Expand Down Expand Up @@ -429,6 +431,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
ignoreDevDeps.set(repoOptions.ignoreDevDependency);
checkDirectDepsMode = repoOptions.checkDirectDependencies;
bazelCompatibilityMode = repoOptions.bazelCompatibilityMode;
bazelLockfileMode = repoOptions.lockfileMode;
allowedYankedVersions = repoOptions.allowedYankedVersions;

if (repoOptions.registries != null && !repoOptions.registries.isEmpty()) {
Expand Down Expand Up @@ -520,6 +523,7 @@ public ImmutableList<Injected> getPrecomputedValues() {
BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES, checkDirectDepsMode),
PrecomputedValue.injected(
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, bazelCompatibilityMode),
PrecomputedValue.injected(BazelLockFileFunction.LOCKFILE_MODE, bazelLockfileMode),
PrecomputedValue.injected(
BazelModuleResolutionFunction.ALLOWED_YANKED_VERSIONS, allowedYankedVersions));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,24 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableTable;
import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction.BazelModuleResolutionFunctionException;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.LabelConverter;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction;
import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.List;
import javax.annotation.Nullable;
import net.starlark.java.eval.StarlarkSemantics;

/**
* This function runs Bazel module resolution, extracts the dependency graph from it and creates a
Expand All @@ -63,23 +61,19 @@ public BazelDepGraphFunction(Path rootDirectory) {
@Nullable
public SkyValue compute(SkyKey skyKey, Environment env)
throws SkyFunctionException, InterruptedException {

RootModuleFileValue root =
(RootModuleFileValue) env.getValue(ModuleFileValue.KEY_FOR_ROOT_MODULE);
if (root == null) {
return null;
}
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
if (starlarkSemantics == null) {
return null;
}
LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env);

ImmutableMap<ModuleKey, Module> depGraph = null;
BzlmodFlagsAndEnvVars flags = null;

// If the module has not changed (has the same contents and flags as the lockfile),
// read the dependency graph from the lock file, else run resolution and update lockfile
if (starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_LOCKFILE)) {
if (!lockfileMode.equals(LockfileMode.OFF)) {
BazelLockFileValue lockFile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY);
if (lockFile == null) {
return null;
Expand All @@ -91,6 +85,14 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (root.getModuleFileHash().equals(lockFile.getModuleFileHash())
&& flags.equals(lockFile.getFlags())) {
depGraph = lockFile.getModuleDepGraph();
} else if (lockfileMode.equals(LockfileMode.ERROR)) {
List<String> diffLockfile = lockFile.getDiffLockfile(root.getModuleFileHash(), flags);
throw new BazelDepGraphFunctionException(
ExternalDepsException.withMessage(
Code.BAD_MODULE,
"Lock file is no longer up-to-date because: %s",
String.join(", ", diffLockfile)),
Transience.PERSISTENT);
}
}

Expand All @@ -101,7 +103,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
return null;
}
depGraph = selectionResult.getResolvedDepGraph();
if (starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_LOCKFILE)) {
if (lockfileMode.equals(LockfileMode.UPDATE)) {
BazelLockFileFunction.updateLockedModule(
root.getModuleFileHash(), depGraph, rootDirectory, flags);
}
Expand Down Expand Up @@ -167,7 +169,7 @@ public static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env)
* all usages by the label + name (the ModuleExtensionId).
*/
private ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> getExtensionUsagesById(
ImmutableMap<ModuleKey, Module> depGraph) throws BazelModuleResolutionFunctionException {
ImmutableMap<ModuleKey, Module> depGraph) throws BazelDepGraphFunctionException {
ImmutableTable.Builder<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
extensionUsagesTableBuilder = ImmutableTable.builder();
for (Module module : depGraph.values()) {
Expand All @@ -182,7 +184,7 @@ private ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> getEx
ModuleExtensionId.create(
labelConverter.convert(usage.getExtensionBzlFile()), usage.getExtensionName());
} catch (LabelSyntaxException e) {
throw new BazelModuleResolutionFunctionException(
throw new BazelDepGraphFunctionException(
ExternalDepsException.withCauseAndMessage(
Code.BAD_MODULE,
e,
Expand All @@ -191,7 +193,7 @@ private ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> getEx
Transience.PERSISTENT);
}
if (!moduleExtensionId.getBzlFileLabel().getRepository().isVisible()) {
throw new BazelModuleResolutionFunctionException(
throw new BazelDepGraphFunctionException(
ExternalDepsException.withMessage(
Code.BAD_MODULE,
"invalid label for module extension found at %s: no repo visible as '@%s' here",
Expand Down Expand Up @@ -225,4 +227,10 @@ private ImmutableBiMap<String, ModuleExtensionId> calculateUniqueNameForUsedExte
}
return ImmutableBiMap.copyOf(extensionUniqueNames);
}

static class BazelDepGraphFunctionException extends SkyFunctionException {
BazelDepGraphFunctionException(ExternalDepsException e, Transience transience) {
super(e, transience);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction.BazelModuleResolutionFunctionException;
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphFunction.BazelDepGraphFunctionException;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Root;
Expand All @@ -41,6 +43,8 @@
/** Reads the contents of the lock file into its value. */
public class BazelLockFileFunction implements SkyFunction {

public static final Precomputed<LockfileMode> LOCKFILE_MODE = new Precomputed<>("lockfile_mode");

private final Path rootDirectory;

private static final BzlmodFlagsAndEnvVars EMPTY_FLAGS =
Expand Down Expand Up @@ -90,7 +94,7 @@ public static void updateLockedModule(
ImmutableMap<ModuleKey, Module> resolvedDepGraph,
Path rootDirectory,
BzlmodFlagsAndEnvVars flags)
throws BazelModuleResolutionFunctionException {
throws BazelDepGraphFunctionException {
RootedPath lockfilePath =
RootedPath.toRootedPath(Root.fromPath(rootDirectory), LabelConstants.MODULE_LOCKFILE_NAME);

Expand All @@ -100,7 +104,7 @@ public static void updateLockedModule(
try {
FileSystemUtils.writeContent(lockfilePath.asPath(), UTF_8, LOCKFILE_GSON.toJson(value));
} catch (IOException e) {
throw new BazelModuleResolutionFunctionException(
throw new BazelDepGraphFunctionException(
ExternalDepsException.withCauseAndMessage(
Code.BAD_MODULE, e, "Unable to update module-lock file"),
Transience.PERSISTENT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@

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.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
import java.util.ArrayList;

/**
* The result of reading the lockfile. Contains the lockfile version, module hash, definitions of
Expand Down Expand Up @@ -56,4 +58,34 @@ public static BazelLockFileValue create(
/** The post-selection dep graph retrieved from the lock file. */
public abstract ImmutableMap<ModuleKey, Module> getModuleDepGraph();

/** Returns the difference between the lockfile and the current module & flags */
public ArrayList<String> getDiffLockfile(String moduleFileHash, BzlmodFlagsAndEnvVars flags) {
ArrayList<String> diffLockfile = new ArrayList<>();
if (!moduleFileHash.equals(getModuleFileHash())) {
diffLockfile.add("the root MODULE.bazel has been modified");
}
if (!flags.cmdRegistries().equals(getFlags().cmdRegistries())) {
diffLockfile.add("the value of --registry flag has been modified");
}
if (!flags.cmdModuleOverrides().equals(getFlags().cmdModuleOverrides())) {
diffLockfile.add("the value of --override_module flag has been modified");
}
if (!flags.allowedYankedVersions().equals(getFlags().allowedYankedVersions())) {
diffLockfile.add("the value of --allow_yanked_versions flag has been modified");
}
if (!flags.envVarAllowedYankedVersions().equals(getFlags().envVarAllowedYankedVersions())) {
diffLockfile.add(
"the value of BZLMOD_ALLOW_YANKED_VERSIONS environment variable has been modified");
}
if (flags.ignoreDevDependency() != getFlags().ignoreDevDependency()) {
diffLockfile.add("the value of --ignore_dev_dependency flag has been modified");
}
if (!flags.directDependenciesMode().equals(getFlags().directDependenciesMode())) {
diffLockfile.add("the value of --check_direct_dependencies flag has been modified");
}
if (!flags.compatibilityMode().equals(getFlags().compatibilityMode())) {
diffLockfile.add("the value of --check_bazel_compatibility flag has been modified");
}
return diffLockfile;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,19 @@ public class RepositoryOptions extends OptionsBase {
+ " warning when mismatch detected.")
public BazelCompatibilityMode bazelCompatibilityMode;

@Option(
name = "lockfile_mode",
defaultValue = "off", // TODO(salmasamy) later will be changed to 'update'
converter = LockfileMode.Converter.class,
documentationCategory = OptionDocumentationCategory.BZLMOD,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
help =
"Specifies how and whether or not to use the lockfile. Valid values are `update` to"
+ " use the lockfile and update it if there are changes, `error` to use the lockfile"
+ " but throw an error if it's not up-to-date, or `off` to neither read from or write"
+ " to the lockfile.")
public LockfileMode lockfileMode;

/** An enum for specifying different modes for checking direct dependency accuracy. */
public enum CheckDirectDepsMode {
OFF, // Don't check direct dependency accuracy.
Expand All @@ -308,6 +321,7 @@ public Converter() {
}
}
}

/** An enum for specifying different modes for bazel compatibility check. */
public enum BazelCompatibilityMode {
ERROR, // Check and throw an error when mismatched.
Expand All @@ -322,6 +336,20 @@ public Converter() {
}
}

/** An enum for specifying how to use the lockfile. */
public enum LockfileMode {
OFF, // Don't use the lockfile at all.
UPDATE, // Update the lockfile when it mismatches the module.
ERROR; // Throw an error when it mismatches the module.

/** Converts to {@link BazelLockfileMode}. */
public static class Converter extends EnumConverter<LockfileMode> {
public Converter() {
super(LockfileMode.class, "Lockfile mode");
}
}
}

/**
* Converts from an equals-separated pair of strings into RepositoryName->PathFragment mapping.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,6 @@ public final class BuildLanguageOptions extends OptionsBase {
+ " WORKSPACE. See https://bazel.build/docs/bzlmod for more information.")
public boolean enableBzlmod;

@Option(
name = "experimental_enable_bzlmod_lockfile",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = OptionEffectTag.LOADING_AND_ANALYSIS,
help = "If true, enables the Bzlmod lockfile caching the module contents.")
public boolean enableLockfile;

@Option(
name = "experimental_java_proto_library_default_has_services",
defaultValue = "true",
Expand Down Expand Up @@ -686,7 +678,6 @@ public StarlarkSemantics toStarlarkSemantics() {
.setBool(
EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, experimentalEnableAndroidMigrationApis)
.setBool(ENABLE_BZLMOD, enableBzlmod)
.setBool(ENABLE_LOCKFILE, enableLockfile)
.setBool(
EXPERIMENTAL_JAVA_PROTO_LIBRARY_DEFAULT_HAS_SERVICES,
experimentalJavaProtoLibraryDefaultHasServices)
Expand Down Expand Up @@ -780,7 +771,6 @@ public StarlarkSemantics toStarlarkSemantics() {
public static final String EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS =
"-experimental_enable_android_migration_apis";
public static final String ENABLE_BZLMOD = "-enable_bzlmod";
public static final String ENABLE_LOCKFILE = "-experimental_enable_bzlmod_lockfile";
public static final String EXPERIMENTAL_JAVA_PROTO_LIBRARY_DEFAULT_HAS_SERVICES =
"+experimental_java_proto_library_default_has_services";
public static final String INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction;
import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction;
import com.google.devtools.build.lib.bazel.bzlmod.FakeRegistry;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -54,6 +56,7 @@ protected ImmutableList<Injected> extraPrecomputedValues() throws Exception {
BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES, CheckDirectDepsMode.WARNING),
PrecomputedValue.injected(
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, BazelCompatibilityMode.ERROR),
PrecomputedValue.injected(BazelLockFileFunction.LOCKFILE_MODE, LockfileMode.OFF),
PrecomputedValue.injected(
BazelModuleResolutionFunction.ALLOWED_YANKED_VERSIONS, ImmutableList.of()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.analysis.util.DummyTestFragment;
import com.google.devtools.build.lib.analysis.util.DummyTestFragment.DummyTestOptions;
import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction;
import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction;
import com.google.devtools.build.lib.bazel.bzlmod.FakeRegistry;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleTransitionData;
Expand Down Expand Up @@ -74,7 +76,8 @@ protected ImmutableList<Injected> extraPrecomputedValues() {
PrecomputedValue.injected(
BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES, CheckDirectDepsMode.WARNING),
PrecomputedValue.injected(
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, BazelCompatibilityMode.ERROR));
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, BazelCompatibilityMode.ERROR),
PrecomputedValue.injected(BazelLockFileFunction.LOCKFILE_MODE, LockfileMode.OFF));
}

@Override
Expand Down
Loading

0 comments on commit 3b11a2f

Please sign in to comment.