Skip to content

Commit

Permalink
Prevent most side effects of yanked modules
Browse files Browse the repository at this point in the history
Yanked module versions no longer contribute dependency requirements or emit `DEBUG` messages for `print()` statements.

Since the module files of yanked modules are still evaluated to learn their compatibility levels, they can still fail to execute.

Closes bazelbuild#18698.

PiperOrigin-RevId: 544059396
Change-Id: I8a37d5c7975947cd717f6e56d97cce467f22178e
  • Loading branch information
fmeum authored and copybara-github committed Jun 28, 2023
1 parent 0d72103 commit 2a2a474
Show file tree
Hide file tree
Showing 26 changed files with 365 additions and 186 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import com.google.devtools.build.lib.bazel.bzlmod.RepoSpec;
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.commands.FetchCommand;
import com.google.devtools.build.lib.bazel.commands.ModqueryCommand;
import com.google.devtools.build.lib.bazel.commands.SyncCommand;
Expand Down Expand Up @@ -590,7 +591,7 @@ public ImmutableList<Injected> getPrecomputedValues() {
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, bazelCompatibilityMode),
PrecomputedValue.injected(BazelLockFileFunction.LOCKFILE_MODE, bazelLockfileMode),
PrecomputedValue.injected(
BazelModuleResolutionFunction.ALLOWED_YANKED_VERSIONS, allowedYankedVersions));
YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, allowedYankedVersions));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ java_library(
"SingleExtensionUsagesFunction.java",
"StarlarkBazelModule.java",
"TypeCheckedTag.java",
"YankedVersionsUtil.java",
],
deps = [
":common",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
(ClientEnvironmentValue)
env.getValue(
ClientEnvironmentFunction.key(
BazelModuleResolutionFunction.BZLMOD_ALLOWED_YANKED_VERSIONS_ENV));
YankedVersionsUtil.BZLMOD_ALLOWED_YANKED_VERSIONS_ENV));
if (allowedYankedVersionsFromEnv == null) {
return null;
}
Expand All @@ -185,7 +185,7 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
toImmutableMap(e -> e.getKey(), e -> ((LocalPathOverride) e.getValue()).getPath()));

ImmutableList<String> yankedVersions =
ImmutableList.copyOf(BazelModuleResolutionFunction.ALLOWED_YANKED_VERSIONS.get(env));
ImmutableList.copyOf(YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.get(env));
Boolean ignoreDevDeps = ModuleFileFunction.IGNORE_DEV_DEPS.get(env);
String compatabilityMode =
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE.get(env).name();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,38 +15,30 @@

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

import com.google.common.base.Splitter;
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.devtools.build.lib.analysis.BlazeVersionInfo;
import com.google.devtools.build.lib.bazel.BazelVersion;
import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
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.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.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.Precomputed;
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.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import javax.annotation.Nullable;

/**
Expand All @@ -59,22 +51,11 @@ public class BazelModuleResolutionFunction implements SkyFunction {
new Precomputed<>("check_direct_dependency");
public static final Precomputed<BazelCompatibilityMode> BAZEL_COMPATIBILITY_MODE =
new Precomputed<>("bazel_compatibility_mode");
public static final Precomputed<List<String>> ALLOWED_YANKED_VERSIONS =
new Precomputed<>("allowed_yanked_versions");

public static final String BZLMOD_ALLOWED_YANKED_VERSIONS_ENV = "BZLMOD_ALLOW_YANKED_VERSIONS";

@Override
@Nullable
public SkyValue compute(SkyKey skyKey, Environment env)
throws BazelModuleResolutionFunctionException, InterruptedException {

ClientEnvironmentValue allowedYankedVersionsFromEnv =
(ClientEnvironmentValue)
env.getValue(ClientEnvironmentFunction.key(BZLMOD_ALLOWED_YANKED_VERSIONS_ENV));
if (allowedYankedVersionsFromEnv == null) {
return null;
}
RootModuleFileValue root =
(RootModuleFileValue) env.getValue(ModuleFileValue.KEY_FOR_ROOT_MODULE);
if (root == null) {
Expand Down Expand Up @@ -104,12 +85,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
Objects.requireNonNull(BAZEL_COMPATIBILITY_MODE.get(env)),
env.getListener());

verifyYankedVersions(
resolvedDepGraph,
parseYankedVersions(
allowedYankedVersionsFromEnv.getValue(),
Objects.requireNonNull(ALLOWED_YANKED_VERSIONS.get(env))),
env.getListener());
checkNoYankedVersions(resolvedDepGraph);

ImmutableMap<ModuleKey, Module> finalDepGraph =
computeFinalDepGraph(resolvedDepGraph, root.getOverrides(), env.getListener());
Expand Down Expand Up @@ -194,125 +170,10 @@ public static void checkBazelCompatibility(
}
}

/**
* Parse a set of allowed yanked version from command line flag (--allowed_yanked_versions) and
* environment variable (ALLOWED_YANKED_VERSIONS). If `all` is specified, return Optional.empty();
* otherwise returns the set of parsed modulel key.
*/
private Optional<ImmutableSet<ModuleKey>> parseYankedVersions(
String allowedYankedVersionsFromEnv, List<String> allowedYankedVersionsFromFlag)
throws BazelModuleResolutionFunctionException {
ImmutableSet.Builder<ModuleKey> allowedYankedVersionBuilder = new ImmutableSet.Builder<>();
if (allowedYankedVersionsFromEnv != null) {
if (parseModuleKeysFromString(
allowedYankedVersionsFromEnv,
allowedYankedVersionBuilder,
String.format(
"envirnoment variable %s=%s",
BZLMOD_ALLOWED_YANKED_VERSIONS_ENV, allowedYankedVersionsFromEnv))) {
return Optional.empty();
}
}
for (String allowedYankedVersions : allowedYankedVersionsFromFlag) {
if (parseModuleKeysFromString(
allowedYankedVersions,
allowedYankedVersionBuilder,
String.format("command line flag --allow_yanked_versions=%s", allowedYankedVersions))) {
return Optional.empty();
}
}
return Optional.of(allowedYankedVersionBuilder.build());
}

/**
* Parse of a comma-separated list of module version(s) of the form '<module name>@<version>' or
* 'all' from the string. Returns true if 'all' is present, otherwise returns false.
*/
private boolean parseModuleKeysFromString(
String input, ImmutableSet.Builder<ModuleKey> allowedYankedVersionBuilder, String context)
private static void checkNoYankedVersions(ImmutableMap<ModuleKey, InterimModule> depGraph)
throws BazelModuleResolutionFunctionException {
ImmutableList<String> moduleStrs = ImmutableList.copyOf(Splitter.on(',').split(input));

for (String moduleStr : moduleStrs) {
if (moduleStr.equals("all")) {
return true;
}

if (moduleStr.isEmpty()) {
continue;
}

String[] pieces = moduleStr.split("@", 2);

if (pieces.length != 2) {
throw new BazelModuleResolutionFunctionException(
ExternalDepsException.withMessage(
Code.VERSION_RESOLUTION_ERROR,
"Parsing %s failed, module versions must be of the form '<module name>@<version>'",
context),
Transience.PERSISTENT);
}

if (!RepositoryName.VALID_MODULE_NAME.matcher(pieces[0]).matches()) {
throw new BazelModuleResolutionFunctionException(
ExternalDepsException.withMessage(
Code.VERSION_RESOLUTION_ERROR,
"Parsing %s failed, invalid module name '%s': valid names must 1) only contain"
+ " lowercase letters (a-z), digits (0-9), dots (.), hyphens (-), and"
+ " underscores (_); 2) begin with a lowercase letter; 3) end with a lowercase"
+ " letter or digit.",
context,
pieces[0]),
Transience.PERSISTENT);
}

Version version;
try {
version = Version.parse(pieces[1]);
} catch (ParseException e) {
throw new BazelModuleResolutionFunctionException(
ExternalDepsException.withCauseAndMessage(
Code.VERSION_RESOLUTION_ERROR,
e,
"Parsing %s failed, invalid version specified for module: %s",
context,
pieces[1]),
Transience.PERSISTENT);
}

allowedYankedVersionBuilder.add(ModuleKey.create(pieces[0], version));
}
return false;
}

private static void verifyYankedVersions(
ImmutableMap<ModuleKey, InterimModule> depGraph,
Optional<ImmutableSet<ModuleKey>> allowedYankedVersions,
ExtendedEventHandler eventHandler)
throws BazelModuleResolutionFunctionException, InterruptedException {
// Check whether all resolved modules are either not yanked or allowed. Modules with a
// NonRegistryOverride are ignored as their metadata is not available whatsoever.
for (InterimModule m : depGraph.values()) {
if (m.getKey().equals(ModuleKey.ROOT) || m.getRegistry() == null) {
continue;
}
Optional<ImmutableMap<Version, String>> yankedVersions;
try {
yankedVersions = m.getRegistry().getYankedVersions(m.getKey().getName(), eventHandler);
} catch (IOException e) {
eventHandler.handle(
Event.warn(
String.format(
"Could not read metadata file for module %s: %s", m.getKey(), e.getMessage())));
continue;
}
if (yankedVersions.isEmpty()) {
continue;
}
String yankedInfo = yankedVersions.get().get(m.getVersion());
if (yankedInfo != null
&& allowedYankedVersions.isPresent()
&& !allowedYankedVersions.get().contains(m.getKey())) {
if (m.getYankedInfo().isPresent()) {
throw new BazelModuleResolutionFunctionException(
ExternalDepsException.withMessage(
Code.VERSION_RESOLUTION_ERROR,
Expand All @@ -322,7 +183,7 @@ private static void verifyYankedVersions(
+ "continue using this version, allow it using the --allow_yanked_versions "
+ "flag or the BZLMOD_ALLOW_YANKED_VERSIONS env variable.",
m.getKey(),
yankedInfo),
m.getYankedInfo().get()),
Transience.PERSISTENT);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public abstract class InterimModule extends ModuleBase {
/** List of bazel compatible versions that would run/fail this module */
public abstract ImmutableList<String> getBazelCompatibility();

/** The reason why this module was yanked or empty if it hasn't been yanked. */
public abstract Optional<String> getYankedInfo();

/** The specification of a dependency. */
@AutoValue
public abstract static class DepSpec {
Expand Down Expand Up @@ -102,7 +105,8 @@ public static Builder builder() {
.setName("")
.setVersion(Version.EMPTY)
.setKey(ModuleKey.ROOT)
.setCompatibilityLevel(0);
.setCompatibilityLevel(0)
.setYankedInfo(Optional.empty());
}

/**
Expand Down Expand Up @@ -133,6 +137,9 @@ public abstract static class Builder {
/** Optional; defaults to {@link #setName}. */
public abstract Builder setRepoName(String value);

/** Optional; defaults to {@link Optional#empty()}. */
public abstract Builder setYankedInfo(Optional<String> value);

public abstract Builder setBazelCompatibility(ImmutableList<String> value);

abstract ImmutableList.Builder<String> bazelCompatibilityBuilder();
Expand Down
Loading

0 comments on commit 2a2a474

Please sign in to comment.