From 1929128525c86355b3492313116c1f1663caf705 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 14 Jul 2023 12:31:35 +0200 Subject: [PATCH] Relativize absolute root module file path in `MODULE.bazel.lock` 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 --- .../bazel/bzlmod/BazelLockFileFunction.java | 9 +- .../lib/bazel/bzlmod/BazelLockFileModule.java | 10 +- .../lib/bazel/bzlmod/GsonTypeAdapterUtil.java | 115 +++++++++++++++--- .../py/bazel/bzlmod/bazel_lockfile_test.py | 57 +++++++++ 4 files changed, 169 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java index 41cff393000ac1..1451f431cf916f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java @@ -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; @@ -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; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java index 5bd3b40a06bd16..fd2bfddd42120a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java @@ -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; @@ -136,7 +135,14 @@ private ImmutableMap 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()); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java index c558e806e9d6d3..edeb6890827764 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java @@ -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; @@ -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 @@ -188,24 +191,100 @@ public Optional 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 TypeAdapter create(Gson gson, TypeToken typeToken) { + if (typeToken.getRawType() != Location.class) { + return null; + } + TypeAdapter relativizedLocationTypeAdapter = + gson.getAdapter(RootModuleFileEscapingLocation.class); + return (TypeAdapter) + new TypeAdapter() { + + @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() {} } diff --git a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py index 4f99b84bb1df0d..56b3e1c628704b 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -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()