Skip to content

Commit eee3ec9

Browse files
fmeumcopybara-github
fmeum
authored and
copybara-github
committed
Add support for isolated extension usages to the lockfile
Previously, Bazel would crash if a module declares an isolated extension usage and the lockfile is used. Closes #18991. PiperOrigin-RevId: 549631385 Change-Id: Id8e706991dc5053b2873847a62f5e0b777347c69
1 parent 81cf998 commit eee3ec9

4 files changed

Lines changed: 127 additions & 18 deletions

File tree

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

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import java.lang.reflect.ParameterizedType;
4242
import java.lang.reflect.Type;
4343
import java.util.Base64;
44-
import java.util.List;
4544
import java.util.Optional;
4645
import javax.annotation.Nullable;
4746
import net.starlark.java.syntax.Location;
@@ -82,52 +81,81 @@ public void write(JsonWriter jsonWriter, ModuleKey moduleKey) throws IOException
8281
@Override
8382
public ModuleKey read(JsonReader jsonReader) throws IOException {
8483
String jsonString = jsonReader.nextString();
85-
if (jsonString.equals("<root>")) {
86-
return ModuleKey.ROOT;
87-
}
88-
List<String> parts = Splitter.on('@').splitToList(jsonString);
89-
if (parts.get(1).equals("_")) {
90-
return ModuleKey.create(parts.get(0), Version.EMPTY);
91-
}
92-
93-
Version version;
9484
try {
95-
version = Version.parse(parts.get(1));
85+
return ModuleKey.fromString(jsonString);
9686
} catch (ParseException e) {
9787
throw new JsonParseException(
9888
String.format("Unable to parse ModuleKey %s version from the lockfile", jsonString),
9989
e);
10090
}
101-
return ModuleKey.create(parts.get(0), version);
10291
}
10392
};
10493

105-
// TODO(salmasamy) need to handle "isolated" in module extensions when it is stable
10694
public static final TypeAdapter<ModuleExtensionId> MODULE_EXTENSION_ID_TYPE_ADAPTER =
10795
new TypeAdapter<>() {
10896
@Override
10997
public void write(JsonWriter jsonWriter, ModuleExtensionId moduleExtId) throws IOException {
110-
jsonWriter.value(moduleExtId.getBzlFileLabel() + "%" + moduleExtId.getExtensionName());
98+
String isolationKeyPart = moduleExtId.getIsolationKey().map(key -> "%" + key).orElse("");
99+
jsonWriter.value(
100+
moduleExtId.getBzlFileLabel()
101+
+ "%"
102+
+ moduleExtId.getExtensionName()
103+
+ isolationKeyPart);
111104
}
112105

113106
@Override
114107
public ModuleExtensionId read(JsonReader jsonReader) throws IOException {
115108
String jsonString = jsonReader.nextString();
116-
// [0] is labelString, [1] is extensionName
117-
List<String> extIdParts = Splitter.on("%").splitToList(jsonString);
109+
var extIdParts = Splitter.on('%').splitToList(jsonString);
110+
Optional<ModuleExtensionId.IsolationKey> isolationKey;
111+
if (extIdParts.size() > 2) {
112+
try {
113+
isolationKey =
114+
Optional.of(ModuleExtensionId.IsolationKey.fromString(extIdParts.get(2)));
115+
} catch (ParseException e) {
116+
throw new JsonParseException(
117+
String.format(
118+
"Unable to parse ModuleExtensionID isolation key: '%s' from the lockfile",
119+
extIdParts.get(2)),
120+
e);
121+
}
122+
} else {
123+
isolationKey = Optional.empty();
124+
}
118125
try {
119126
return ModuleExtensionId.create(
120-
Label.parseCanonical(extIdParts.get(0)), extIdParts.get(1), Optional.empty());
127+
Label.parseCanonical(extIdParts.get(0)), extIdParts.get(1), isolationKey);
121128
} catch (LabelSyntaxException e) {
122129
throw new JsonParseException(
123130
String.format(
124-
"Unable to parse ModuleExtensionID bzl file label: '%s' from the lockfile",
131+
"Unable to parse ModuleExtensionID bzl file label: '%s' from the lockfile",
125132
extIdParts.get(0)),
126133
e);
127134
}
128135
}
129136
};
130137

138+
public static final TypeAdapter<ModuleExtensionId.IsolationKey> ISOLATION_KEY_TYPE_ADAPTER =
139+
new TypeAdapter<>() {
140+
@Override
141+
public void write(JsonWriter jsonWriter, ModuleExtensionId.IsolationKey isolationKey)
142+
throws IOException {
143+
jsonWriter.value(isolationKey.toString());
144+
}
145+
146+
@Override
147+
public ModuleExtensionId.IsolationKey read(JsonReader jsonReader) throws IOException {
148+
String jsonString = jsonReader.nextString();
149+
try {
150+
return ModuleExtensionId.IsolationKey.fromString(jsonString);
151+
} catch (ParseException e) {
152+
throw new JsonParseException(
153+
String.format("Unable to parse isolation key: '%s' from the lockfile", jsonString),
154+
e);
155+
}
156+
}
157+
};
158+
131159
public static final TypeAdapter<byte[]> BYTE_ARRAY_TYPE_ADAPTER =
132160
new TypeAdapter<>() {
133161
@Override
@@ -283,6 +311,7 @@ public static Gson createLockFileGson(Path moduleFilePath) {
283311
.registerTypeAdapter(Version.class, VERSION_TYPE_ADAPTER)
284312
.registerTypeAdapter(ModuleKey.class, MODULE_KEY_TYPE_ADAPTER)
285313
.registerTypeAdapter(ModuleExtensionId.class, MODULE_EXTENSION_ID_TYPE_ADAPTER)
314+
.registerTypeAdapter(ModuleExtensionId.IsolationKey.class, ISOLATION_KEY_TYPE_ADAPTER)
286315
.registerTypeAdapter(AttributeValues.class, new AttributeValuesAdapter())
287316
.registerTypeAdapter(byte[].class, BYTE_ARRAY_TYPE_ADAPTER)
288317
.create();

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
import static java.util.Comparator.comparing;
2020

2121
import com.google.auto.value.AutoValue;
22+
import com.google.common.base.Splitter;
2223
import com.google.devtools.build.lib.cmdline.Label;
2324
import java.util.Comparator;
25+
import java.util.List;
2426
import java.util.Optional;
2527

2628
/** A unique identifier for a {@link ModuleExtension}. */
@@ -49,6 +51,17 @@ abstract static class IsolationKey {
4951
public static IsolationKey create(ModuleKey module, String usageExportedName) {
5052
return new AutoValue_ModuleExtensionId_IsolationKey(module, usageExportedName);
5153
}
54+
55+
@Override
56+
public final String toString() {
57+
return getModule() + "~" + getUsageExportedName();
58+
}
59+
60+
public static IsolationKey fromString(String s) throws Version.ParseException {
61+
List<String> isolationKeyParts = Splitter.on("~").splitToList(s);
62+
return ModuleExtensionId.IsolationKey.create(
63+
ModuleKey.fromString(isolationKeyParts.get(0)), isolationKeyParts.get(1));
64+
}
5265
}
5366

5467
public abstract Label getBzlFileLabel();

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@
1616
package com.google.devtools.build.lib.bazel.bzlmod;
1717

1818
import com.google.auto.value.AutoValue;
19+
import com.google.common.base.Splitter;
1920
import com.google.common.collect.ImmutableMap;
2021
import com.google.devtools.build.lib.cmdline.RepositoryName;
2122
import java.util.Comparator;
23+
import java.util.List;
2224

2325
/** A module name, version pair that identifies a module in the external dependency graph. */
2426
@AutoValue
@@ -79,4 +81,17 @@ public RepositoryName getCanonicalRepoName() {
7981
return RepositoryName.createUnvalidated(
8082
String.format("%s~%s", getName(), getVersion().isEmpty() ? "override" : getVersion()));
8183
}
84+
85+
public static ModuleKey fromString(String s) throws Version.ParseException {
86+
if (s.equals("<root>")) {
87+
return ModuleKey.ROOT;
88+
}
89+
List<String> parts = Splitter.on('@').splitToList(s);
90+
if (parts.get(1).equals("_")) {
91+
return ModuleKey.create(parts.get(0), Version.EMPTY);
92+
}
93+
94+
Version version = Version.parse(parts.get(1));
95+
return ModuleKey.create(parts.get(0), version);
96+
}
8297
}

‎src/test/py/bazel/bzlmod/bazel_lockfile_test.py‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,58 @@ def testModuleExtension(self):
299299
)
300300
self.assertNotIn('Hello from the other side!', ''.join(stderr))
301301

302+
def testIsolatedModuleExtension(self):
303+
self.ScratchFile(
304+
'MODULE.bazel',
305+
[
306+
(
307+
'lockfile_ext = use_extension("extension.bzl", "lockfile_ext",'
308+
' isolate = True)'
309+
),
310+
'lockfile_ext.dep(name = "bmbm", versions = ["v1", "v2"])',
311+
'use_repo(lockfile_ext, "hello")',
312+
],
313+
)
314+
self.ScratchFile('BUILD.bazel')
315+
self.ScratchFile(
316+
'extension.bzl',
317+
[
318+
'def _repo_rule_impl(ctx):',
319+
' ctx.file("WORKSPACE")',
320+
' ctx.file("BUILD", "filegroup(name=\'lala\')")',
321+
'',
322+
'repo_rule = repository_rule(implementation=_repo_rule_impl)',
323+
'',
324+
'def _module_ext_impl(ctx):',
325+
' print("Hello from the other side!")',
326+
' repo_rule(name="hello")',
327+
' for mod in ctx.modules:',
328+
' for dep in mod.tags.dep:',
329+
' print("Name:", dep.name, ", Versions:", dep.versions)',
330+
'',
331+
'_dep = tag_class(attrs={"name": attr.string(), "versions":',
332+
' attr.string_list()})',
333+
'lockfile_ext = module_extension(',
334+
' implementation=_module_ext_impl,',
335+
' tag_classes={"dep": _dep},',
336+
' environ=["GREEN_TREES", "NOT_SET"],',
337+
')',
338+
],
339+
)
340+
341+
# Only set one env var, to make sure null variables don't crash
342+
_, _, stderr = self.RunBazel(
343+
['build', '@hello//:all'], env_add={'GREEN_TREES': 'In the city'}
344+
)
345+
self.assertIn('Hello from the other side!', ''.join(stderr))
346+
self.assertIn('Name: bmbm , Versions: ["v1", "v2"]', ''.join(stderr))
347+
348+
self.RunBazel(['shutdown'])
349+
_, _, stderr = self.RunBazel(
350+
['build', '@hello//:all'], env_add={'GREEN_TREES': 'In the city'}
351+
)
352+
self.assertNotIn('Hello from the other side!', ''.join(stderr))
353+
302354
def testModuleExtensionsInDifferentBuilds(self):
303355
# Test that the module extension stays in the lockfile (as long as it's
304356
# used in the module) even if it is not in the current build

0 commit comments

Comments
 (0)