Skip to content

Commit

Permalink
Relativize absolute root module file path in MODULE.bazel.lock
Browse files Browse the repository at this point in the history
The absolute paths of the root module file path in Starlark `Location`s
is replaced with a constant when written to the lock file and replaced
back when reading from the file.

Work towards #18936
  • Loading branch information
fmeum committed Jul 17, 2023
1 parent 9932e4a commit 1929128
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

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

import static com.google.devtools.build.lib.bazel.bzlmod.GsonTypeAdapterUtil.LOCKFILE_GSON;
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -93,7 +92,13 @@ public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath) throw
BazelLockFileValue bazelLockFileValue;
try {
String json = FileSystemUtils.readContent(lockfilePath.asPath(), UTF_8);
bazelLockFileValue = LOCKFILE_GSON.fromJson(json, BazelLockFileValue.class);
bazelLockFileValue =
GsonTypeAdapterUtil.createLockFileGson(
lockfilePath
.asPath()
.getParentDirectory()
.getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME))
.fromJson(json, BazelLockFileValue.class);
} catch (FileNotFoundException e) {
bazelLockFileValue = EMPTY_LOCKFILE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

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

import static com.google.devtools.build.lib.bazel.bzlmod.GsonTypeAdapterUtil.LOCKFILE_GSON;
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -136,7 +135,14 @@ private ImmutableMap<ModuleExtensionId, LockFileModuleExtension> combineModuleEx
public static void updateLockfile(RootedPath lockfilePath, BazelLockFileValue updatedLockfile) {
try {
FileSystemUtils.writeContent(
lockfilePath.asPath(), UTF_8, LOCKFILE_GSON.toJson(updatedLockfile));
lockfilePath.asPath(),
UTF_8,
GsonTypeAdapterUtil.createLockFileGson(
lockfilePath
.asPath()
.getParentDirectory()
.getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME))
.toJson(updatedLockfile));
} catch (IOException e) {
logger.atSevere().withCause(e).log(
"Error while updating MODULE.bazel.lock file: %s", e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@
import static com.google.devtools.build.lib.bazel.bzlmod.DelegateTypeAdapterFactory.IMMUTABLE_MAP;
import static com.google.devtools.build.lib.bazel.bzlmod.DelegateTypeAdapterFactory.IMMUTABLE_SET;

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.vfs.Path;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonParseException;
Expand All @@ -42,6 +44,7 @@
import java.util.List;
import java.util.Optional;
import javax.annotation.Nullable;
import net.starlark.java.syntax.Location;

/**
* Utility class to hold type adapters and helper methods to get gson registered with type adapters
Expand Down Expand Up @@ -188,24 +191,100 @@ public Optional<T> read(JsonReader jsonReader) throws IOException {
}
}

public static final Gson LOCKFILE_GSON =
new GsonBuilder()
.setPrettyPrinting()
.disableHtmlEscaping()
.enableComplexMapKeySerialization()
.registerTypeAdapterFactory(GenerateTypeAdapter.FACTORY)
.registerTypeAdapterFactory(DICT)
.registerTypeAdapterFactory(IMMUTABLE_MAP)
.registerTypeAdapterFactory(IMMUTABLE_LIST)
.registerTypeAdapterFactory(IMMUTABLE_BIMAP)
.registerTypeAdapterFactory(IMMUTABLE_SET)
.registerTypeAdapterFactory(OPTIONAL)
.registerTypeAdapter(Version.class, VERSION_TYPE_ADAPTER)
.registerTypeAdapter(ModuleKey.class, MODULE_KEY_TYPE_ADAPTER)
.registerTypeAdapter(ModuleExtensionId.class, MODULE_EXTENSION_ID_TYPE_ADAPTER)
.registerTypeAdapter(AttributeValues.class, new AttributeValuesAdapter())
.registerTypeAdapter(byte[].class, BYTE_ARRAY_TYPE_ADAPTER)
.create();
/**
* A variant of {@link Location} that converts the absolute path to the root module file to a
* constant and back.
*/
// protected only for @AutoValue
@GenerateTypeAdapter
@AutoValue
protected abstract static class RootModuleFileEscapingLocation {
// This marker string is neither a valid absolute path nor a valid URL and thus cannot conflict
// with any real module file location.
private static final String ROOT_MODULE_FILE_LABEL = "@@//:MODULE.bazel";

public abstract String file();

public abstract int line();

public abstract int column();

public Location toLocation(String moduleFilePath) {
String file;
if (file().equals(ROOT_MODULE_FILE_LABEL)) {
file = moduleFilePath;
} else {
file = file();
}
return Location.fromFileLineColumn(file, line(), column());
}

public static RootModuleFileEscapingLocation fromLocation(
Location location, String moduleFilePath) {
String file;
if (location.file().equals(moduleFilePath)) {
file = ROOT_MODULE_FILE_LABEL;
} else {
file = location.file();
}
return new AutoValue_GsonTypeAdapterUtil_RootModuleFileEscapingLocation(
file, location.line(), location.column());
}
}

private static final class LocationTypeAdapterFactory implements TypeAdapterFactory {

private final String moduleFilePath;

public LocationTypeAdapterFactory(Path moduleFilePath) {
this.moduleFilePath = moduleFilePath.getPathString();
}

@Override
public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
if (typeToken.getRawType() != Location.class) {
return null;
}
TypeAdapter<RootModuleFileEscapingLocation> relativizedLocationTypeAdapter =
gson.getAdapter(RootModuleFileEscapingLocation.class);
return (TypeAdapter<T>)
new TypeAdapter<Location>() {

@Override
public void write(JsonWriter jsonWriter, Location location) throws IOException {
relativizedLocationTypeAdapter.write(
jsonWriter, RootModuleFileEscapingLocation.fromLocation(location,
moduleFilePath));
}

@Override
public Location read(JsonReader jsonReader) throws IOException {
return relativizedLocationTypeAdapter.read(jsonReader).toLocation(moduleFilePath);
}
};
}
}

public static Gson createLockFileGson(Path moduleFilePath) {
return new GsonBuilder()
.setPrettyPrinting()
.disableHtmlEscaping()
.enableComplexMapKeySerialization()
.registerTypeAdapterFactory(GenerateTypeAdapter.FACTORY)
.registerTypeAdapterFactory(DICT)
.registerTypeAdapterFactory(IMMUTABLE_MAP)
.registerTypeAdapterFactory(IMMUTABLE_LIST)
.registerTypeAdapterFactory(IMMUTABLE_BIMAP)
.registerTypeAdapterFactory(IMMUTABLE_SET)
.registerTypeAdapterFactory(OPTIONAL)
.registerTypeAdapterFactory(new LocationTypeAdapterFactory(moduleFilePath))
.registerTypeAdapter(Version.class, VERSION_TYPE_ADAPTER)
.registerTypeAdapter(ModuleKey.class, MODULE_KEY_TYPE_ADAPTER)
.registerTypeAdapter(ModuleExtensionId.class, MODULE_EXTENSION_ID_TYPE_ADAPTER)
.registerTypeAdapter(AttributeValues.class, new AttributeValuesAdapter())
.registerTypeAdapter(byte[].class, BYTE_ARRAY_TYPE_ADAPTER)
.create();
}

private GsonTypeAdapterUtil() {}
}
57 changes: 57 additions & 0 deletions src/test/py/bazel/bzlmod/bazel_lockfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,63 @@ def testRemoveModuleExtensionsNotUsed(self):
lockfile = json.loads(f.read().strip())
self.assertEqual(len(lockfile['moduleExtensions']), 0)

def testNoAbsoluteRootModuleFilePath(self):
self.ScratchFile(
'MODULE.bazel',
[
'ext = use_extension("extension.bzl", "ext")',
'ext.dep(generate = True)',
'use_repo(ext, ext_hello = "hello")',
'other_ext = use_extension("extension.bzl", "other_ext")',
'other_ext.dep(generate = False)',
'use_repo(other_ext, other_ext_hello = "hello")',
],
)
self.ScratchFile('BUILD.bazel')
self.ScratchFile(
'extension.bzl',
[
'def _repo_rule_impl(ctx):',
' ctx.file("WORKSPACE")',
' ctx.file("BUILD", "filegroup(name=\'lala\')")',
'',
'repo_rule = repository_rule(implementation=_repo_rule_impl)',
'',
'def _module_ext_impl(ctx):',
' for mod in ctx.modules:',
' for dep in mod.tags.dep:',
' if dep.generate:',
' repo_rule(name="hello")',
'',
'_dep = tag_class(attrs={"generate": attr.bool()})',
'ext = module_extension(',
' implementation=_module_ext_impl,',
' tag_classes={"dep": _dep},',
')',
'other_ext = module_extension(',
' implementation=_module_ext_impl,',
' tag_classes={"dep": _dep},',
')',
],
)

module_file_path = self.Path('MODULE.bazel')

self.RunBazel(['build', '--nobuild', '@ext_hello//:all'])
with open(self.Path('MODULE.bazel.lock'), 'r') as f:
self.assertNotIn(module_file_path, f.read())

self.RunBazel(['shutdown'])
exit_code, _, stderr = self.RunBazel(['build', '--nobuild', '@other_ext_hello//:all'],
allow_failure=True)
self.AssertNotExitCode(exit_code, 0, stderr)
self.assertIn(
(
'ERROR: module extension "other_ext" from "//:extension.bzl" does '
'not generate repository "hello", yet it is imported as '
'"other_ext_hello" in the usage at ' + module_file_path + ":4:26"),
stderr)


if __name__ == '__main__':
unittest.main()

0 comments on commit 1929128

Please sign in to comment.