Skip to content

Commit da329e3

Browse files
Wyveraldfmeum
andauthored
Prevent most side effects of yanked modules (#18908)
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 #18698. PiperOrigin-RevId: 544059396 Change-Id: I8a37d5c7975947cd717f6e56d97cce467f22178e Co-authored-by: Fabian Meumertzheim <[email protected]>
1 parent ab35c6f commit da329e3

25 files changed

+363
-183
lines changed

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.google.devtools.build.lib.bazel.bzlmod.RepoSpec;
4949
import com.google.devtools.build.lib.bazel.bzlmod.SingleExtensionEvalFunction;
5050
import com.google.devtools.build.lib.bazel.bzlmod.SingleExtensionUsagesFunction;
51+
import com.google.devtools.build.lib.bazel.bzlmod.YankedVersionsUtil;
5152
import com.google.devtools.build.lib.bazel.commands.FetchCommand;
5253
import com.google.devtools.build.lib.bazel.commands.ModqueryCommand;
5354
import com.google.devtools.build.lib.bazel.commands.SyncCommand;
@@ -570,7 +571,7 @@ public ImmutableList<Injected> getPrecomputedValues() {
570571
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, bazelCompatibilityMode),
571572
PrecomputedValue.injected(BazelLockFileFunction.LOCKFILE_MODE, bazelLockfileMode),
572573
PrecomputedValue.injected(
573-
BazelModuleResolutionFunction.ALLOWED_YANKED_VERSIONS, allowedYankedVersions));
574+
YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, allowedYankedVersions));
574575
}
575576

576577
@Override

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ java_library(
175175
"SingleExtensionUsagesFunction.java",
176176
"StarlarkBazelModule.java",
177177
"TypeCheckedTag.java",
178+
"YankedVersionsUtil.java",
178179
],
179180
deps = [
180181
":common",

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
173173
(ClientEnvironmentValue)
174174
env.getValue(
175175
ClientEnvironmentFunction.key(
176-
BazelModuleResolutionFunction.BZLMOD_ALLOWED_YANKED_VERSIONS_ENV));
176+
YankedVersionsUtil.BZLMOD_ALLOWED_YANKED_VERSIONS_ENV));
177177
if (allowedYankedVersionsFromEnv == null) {
178178
return null;
179179
}
@@ -185,7 +185,7 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
185185
toImmutableMap(e -> e.getKey(), e -> ((LocalPathOverride) e.getValue()).getPath()));
186186

187187
ImmutableList<String> yankedVersions =
188-
ImmutableList.copyOf(BazelModuleResolutionFunction.ALLOWED_YANKED_VERSIONS.get(env));
188+
ImmutableList.copyOf(YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.get(env));
189189
Boolean ignoreDevDeps = ModuleFileFunction.IGNORE_DEV_DEPS.get(env);
190190
String compatabilityMode =
191191
BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE.get(env).name();

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java

Lines changed: 4 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -15,38 +15,30 @@
1515

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

18-
import com.google.common.base.Splitter;
1918
import com.google.common.base.Strings;
2019
import com.google.common.collect.ImmutableCollection;
2120
import com.google.common.collect.ImmutableList;
2221
import com.google.common.collect.ImmutableMap;
23-
import com.google.common.collect.ImmutableSet;
2422
import com.google.common.collect.Maps;
2523
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
2624
import com.google.devtools.build.lib.bazel.BazelVersion;
2725
import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec;
2826
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
29-
import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
3027
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode;
3128
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode;
32-
import com.google.devtools.build.lib.cmdline.RepositoryName;
3329
import com.google.devtools.build.lib.events.Event;
3430
import com.google.devtools.build.lib.events.EventHandler;
3531
import com.google.devtools.build.lib.events.ExtendedEventHandler;
3632
import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
37-
import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction;
38-
import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue;
3933
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
4034
import com.google.devtools.build.skyframe.SkyFunction;
4135
import com.google.devtools.build.skyframe.SkyFunctionException;
4236
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
4337
import com.google.devtools.build.skyframe.SkyKey;
4438
import com.google.devtools.build.skyframe.SkyValue;
4539
import java.io.IOException;
46-
import java.util.List;
4740
import java.util.Map;
4841
import java.util.Objects;
49-
import java.util.Optional;
5042
import javax.annotation.Nullable;
5143

5244
/**
@@ -59,22 +51,11 @@ public class BazelModuleResolutionFunction implements SkyFunction {
5951
new Precomputed<>("check_direct_dependency");
6052
public static final Precomputed<BazelCompatibilityMode> BAZEL_COMPATIBILITY_MODE =
6153
new Precomputed<>("bazel_compatibility_mode");
62-
public static final Precomputed<List<String>> ALLOWED_YANKED_VERSIONS =
63-
new Precomputed<>("allowed_yanked_versions");
64-
65-
public static final String BZLMOD_ALLOWED_YANKED_VERSIONS_ENV = "BZLMOD_ALLOW_YANKED_VERSIONS";
6654

6755
@Override
6856
@Nullable
6957
public SkyValue compute(SkyKey skyKey, Environment env)
7058
throws SkyFunctionException, InterruptedException {
71-
72-
ClientEnvironmentValue allowedYankedVersionsFromEnv =
73-
(ClientEnvironmentValue)
74-
env.getValue(ClientEnvironmentFunction.key(BZLMOD_ALLOWED_YANKED_VERSIONS_ENV));
75-
if (allowedYankedVersionsFromEnv == null) {
76-
return null;
77-
}
7859
RootModuleFileValue root =
7960
(RootModuleFileValue) env.getValue(ModuleFileValue.KEY_FOR_ROOT_MODULE);
8061
if (root == null) {
@@ -104,12 +85,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
10485
Objects.requireNonNull(BAZEL_COMPATIBILITY_MODE.get(env)),
10586
env.getListener());
10687

107-
verifyYankedVersions(
108-
resolvedDepGraph,
109-
parseYankedVersions(
110-
allowedYankedVersionsFromEnv.getValue(),
111-
Objects.requireNonNull(ALLOWED_YANKED_VERSIONS.get(env))),
112-
env.getListener());
88+
checkNoYankedVersions(resolvedDepGraph);
11389

11490
ImmutableMap<ModuleKey, Module> finalDepGraph =
11591
computeFinalDepGraph(resolvedDepGraph, root.getOverrides(), env.getListener());
@@ -194,125 +170,10 @@ public static void checkBazelCompatibility(
194170
}
195171
}
196172

197-
/**
198-
* Parse a set of allowed yanked version from command line flag (--allowed_yanked_versions) and
199-
* environment variable (ALLOWED_YANKED_VERSIONS). If `all` is specified, return Optional.empty();
200-
* otherwise returns the set of parsed modulel key.
201-
*/
202-
private Optional<ImmutableSet<ModuleKey>> parseYankedVersions(
203-
String allowedYankedVersionsFromEnv, List<String> allowedYankedVersionsFromFlag)
204-
throws BazelModuleResolutionFunctionException {
205-
ImmutableSet.Builder<ModuleKey> allowedYankedVersionBuilder = new ImmutableSet.Builder<>();
206-
if (allowedYankedVersionsFromEnv != null) {
207-
if (parseModuleKeysFromString(
208-
allowedYankedVersionsFromEnv,
209-
allowedYankedVersionBuilder,
210-
String.format(
211-
"envirnoment variable %s=%s",
212-
BZLMOD_ALLOWED_YANKED_VERSIONS_ENV, allowedYankedVersionsFromEnv))) {
213-
return Optional.empty();
214-
}
215-
}
216-
for (String allowedYankedVersions : allowedYankedVersionsFromFlag) {
217-
if (parseModuleKeysFromString(
218-
allowedYankedVersions,
219-
allowedYankedVersionBuilder,
220-
String.format("command line flag --allow_yanked_versions=%s", allowedYankedVersions))) {
221-
return Optional.empty();
222-
}
223-
}
224-
return Optional.of(allowedYankedVersionBuilder.build());
225-
}
226-
227-
/**
228-
* Parse of a comma-separated list of module version(s) of the form '<module name>@<version>' or
229-
* 'all' from the string. Returns true if 'all' is present, otherwise returns false.
230-
*/
231-
private boolean parseModuleKeysFromString(
232-
String input, ImmutableSet.Builder<ModuleKey> allowedYankedVersionBuilder, String context)
173+
private static void checkNoYankedVersions(ImmutableMap<ModuleKey, InterimModule> depGraph)
233174
throws BazelModuleResolutionFunctionException {
234-
ImmutableList<String> moduleStrs = ImmutableList.copyOf(Splitter.on(',').split(input));
235-
236-
for (String moduleStr : moduleStrs) {
237-
if (moduleStr.equals("all")) {
238-
return true;
239-
}
240-
241-
if (moduleStr.isEmpty()) {
242-
continue;
243-
}
244-
245-
String[] pieces = moduleStr.split("@", 2);
246-
247-
if (pieces.length != 2) {
248-
throw new BazelModuleResolutionFunctionException(
249-
ExternalDepsException.withMessage(
250-
Code.VERSION_RESOLUTION_ERROR,
251-
"Parsing %s failed, module versions must be of the form '<module name>@<version>'",
252-
context),
253-
Transience.PERSISTENT);
254-
}
255-
256-
if (!RepositoryName.VALID_MODULE_NAME.matcher(pieces[0]).matches()) {
257-
throw new BazelModuleResolutionFunctionException(
258-
ExternalDepsException.withMessage(
259-
Code.VERSION_RESOLUTION_ERROR,
260-
"Parsing %s failed, invalid module name '%s': valid names must 1) only contain"
261-
+ " lowercase letters (a-z), digits (0-9), dots (.), hyphens (-), and"
262-
+ " underscores (_); 2) begin with a lowercase letter; 3) end with a lowercase"
263-
+ " letter or digit.",
264-
context,
265-
pieces[0]),
266-
Transience.PERSISTENT);
267-
}
268-
269-
Version version;
270-
try {
271-
version = Version.parse(pieces[1]);
272-
} catch (ParseException e) {
273-
throw new BazelModuleResolutionFunctionException(
274-
ExternalDepsException.withCauseAndMessage(
275-
Code.VERSION_RESOLUTION_ERROR,
276-
e,
277-
"Parsing %s failed, invalid version specified for module: %s",
278-
context,
279-
pieces[1]),
280-
Transience.PERSISTENT);
281-
}
282-
283-
allowedYankedVersionBuilder.add(ModuleKey.create(pieces[0], version));
284-
}
285-
return false;
286-
}
287-
288-
private static void verifyYankedVersions(
289-
ImmutableMap<ModuleKey, InterimModule> depGraph,
290-
Optional<ImmutableSet<ModuleKey>> allowedYankedVersions,
291-
ExtendedEventHandler eventHandler)
292-
throws BazelModuleResolutionFunctionException, InterruptedException {
293-
// Check whether all resolved modules are either not yanked or allowed. Modules with a
294-
// NonRegistryOverride are ignored as their metadata is not available whatsoever.
295175
for (InterimModule m : depGraph.values()) {
296-
if (m.getKey().equals(ModuleKey.ROOT) || m.getRegistry() == null) {
297-
continue;
298-
}
299-
Optional<ImmutableMap<Version, String>> yankedVersions;
300-
try {
301-
yankedVersions = m.getRegistry().getYankedVersions(m.getKey().getName(), eventHandler);
302-
} catch (IOException e) {
303-
eventHandler.handle(
304-
Event.warn(
305-
String.format(
306-
"Could not read metadata file for module %s: %s", m.getKey(), e.getMessage())));
307-
continue;
308-
}
309-
if (yankedVersions.isEmpty()) {
310-
continue;
311-
}
312-
String yankedInfo = yankedVersions.get().get(m.getVersion());
313-
if (yankedInfo != null
314-
&& allowedYankedVersions.isPresent()
315-
&& !allowedYankedVersions.get().contains(m.getKey())) {
176+
if (m.getYankedInfo().isPresent()) {
316177
throw new BazelModuleResolutionFunctionException(
317178
ExternalDepsException.withMessage(
318179
Code.VERSION_RESOLUTION_ERROR,
@@ -322,7 +183,7 @@ private static void verifyYankedVersions(
322183
+ "continue using this version, allow it using the --allow_yanked_versions "
323184
+ "flag or the BZLMOD_ALLOW_YANKED_VERSIONS env variable.",
324185
m.getKey(),
325-
yankedInfo),
186+
m.getYankedInfo().get()),
326187
Transience.PERSISTENT);
327188
}
328189
}

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ public abstract class InterimModule extends ModuleBase {
5050
/** List of bazel compatible versions that would run/fail this module */
5151
public abstract ImmutableList<String> getBazelCompatibility();
5252

53+
/** The reason why this module was yanked or empty if it hasn't been yanked. */
54+
public abstract Optional<String> getYankedInfo();
55+
5356
/** The specification of a dependency. */
5457
@AutoValue
5558
public abstract static class DepSpec {
@@ -102,7 +105,8 @@ public static Builder builder() {
102105
.setName("")
103106
.setVersion(Version.EMPTY)
104107
.setKey(ModuleKey.ROOT)
105-
.setCompatibilityLevel(0);
108+
.setCompatibilityLevel(0)
109+
.setYankedInfo(Optional.empty());
106110
}
107111

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

140+
/** Optional; defaults to {@link Optional#empty()}. */
141+
public abstract Builder setYankedInfo(Optional<String> value);
142+
136143
public abstract Builder setBazelCompatibility(ImmutableList<String> value);
137144

138145
abstract ImmutableList.Builder<String> bazelCompatibilityBuilder();

0 commit comments

Comments
 (0)