From 9d12d79ecb407ff6f2f83e90f9cd1d30b07b5986 Mon Sep 17 00:00:00 2001 From: Salma Samy Date: Wed, 24 Jan 2024 18:08:47 +0200 Subject: [PATCH] [7.1.0] Remove user specific path from the lockfile (Fixes #19621) (#21009) If a local registry is specified using a workspace placeholder, the lockfile stores location details of the files with resolved workspace information, including user-specific paths. To fix that: Updating the logic to use the workspace placeholder %workspace% during lockfile writing and reverting to the resolved paths when reading. PiperOrigin-RevId: 600876401 Change-Id: I6162d8e8f1fe5e29b7899fc5bb218d1b6be926d0 --- .../bazel/bzlmod/BazelLockFileFunction.java | 8 +++-- .../lib/bazel/bzlmod/BazelLockFileModule.java | 3 +- .../lib/bazel/bzlmod/GsonTypeAdapterUtil.java | 23 ++++++++------ .../py/bazel/bzlmod/bazel_lockfile_test.py | 31 ++++++++++++++++--- 4 files changed, 48 insertions(+), 17 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 763b3828492301..500a5775bd8051 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 @@ -84,7 +84,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) } try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.BZLMOD, "parse lockfile")) { - return getLockfileValue(lockfilePath); + return getLockfileValue(lockfilePath, rootDirectory); } catch (IOException | JsonSyntaxException | NullPointerException e) { throw new BazelLockfileFunctionException( ExternalDepsException.withMessage( @@ -96,7 +96,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) } } - public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath) throws IOException { + public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath, Path rootDirectory) + throws IOException { BazelLockFileValue bazelLockFileValue; try { String json = FileSystemUtils.readContent(lockfilePath.asPath(), UTF_8); @@ -108,7 +109,8 @@ public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath) throw lockfilePath .asPath() .getParentDirectory() - .getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME)) + .getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME), + rootDirectory) .fromJson(json, BazelLockFileValue.class); } else { // This is an old version, needs to be updated 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 96fe6ac21d5f2f..6c3a69e5e92b6b 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 @@ -201,7 +201,8 @@ public static void updateLockfile(Path workspaceRoot, BazelLockFileValue updated lockfilePath .asPath() .getParentDirectory() - .getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME)) + .getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME), + workspaceRoot) .toJson(updatedLockfile) + "\n"); } catch (IOException e) { 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 05a54e2e053cc7..ce91c25a8b0d6b 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 @@ -301,23 +301,23 @@ protected abstract static class RootModuleFileEscapingLocation { public abstract int column(); - public Location toLocation(String moduleFilePath) { + public Location toLocation(String moduleFilePath, String workspaceRoot) { String file; if (file().equals(ROOT_MODULE_FILE_LABEL)) { file = moduleFilePath; } else { - file = file(); + file = file().replace("%workspace%", workspaceRoot); } return Location.fromFileLineColumn(file, line(), column()); } public static RootModuleFileEscapingLocation fromLocation( - Location location, String moduleFilePath) { + Location location, String moduleFilePath, String workspaceRoot) { String file; if (location.file().equals(moduleFilePath)) { file = ROOT_MODULE_FILE_LABEL; } else { - file = location.file(); + file = location.file().replace(workspaceRoot, "%workspace%"); } return new AutoValue_GsonTypeAdapterUtil_RootModuleFileEscapingLocation( file, location.line(), location.column()); @@ -327,9 +327,11 @@ public static RootModuleFileEscapingLocation fromLocation( private static final class LocationTypeAdapterFactory implements TypeAdapterFactory { private final String moduleFilePath; + private final String workspaceRoot; - public LocationTypeAdapterFactory(Path moduleFilePath) { + public LocationTypeAdapterFactory(Path moduleFilePath, Path workspaceRoot) { this.moduleFilePath = moduleFilePath.getPathString(); + this.workspaceRoot = workspaceRoot.getPathString(); } @Nullable @@ -348,18 +350,21 @@ public TypeAdapter create(Gson gson, TypeToken typeToken) { public void write(JsonWriter jsonWriter, Location location) throws IOException { relativizedLocationTypeAdapter.write( jsonWriter, - RootModuleFileEscapingLocation.fromLocation(location, moduleFilePath)); + RootModuleFileEscapingLocation.fromLocation( + location, moduleFilePath, workspaceRoot)); } @Override public Location read(JsonReader jsonReader) throws IOException { - return relativizedLocationTypeAdapter.read(jsonReader).toLocation(moduleFilePath); + return relativizedLocationTypeAdapter + .read(jsonReader) + .toLocation(moduleFilePath, workspaceRoot); } }; } } - public static Gson createLockFileGson(Path moduleFilePath) { + public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) { return new GsonBuilder() .setPrettyPrinting() .disableHtmlEscaping() @@ -371,7 +376,7 @@ public static Gson createLockFileGson(Path moduleFilePath) { .registerTypeAdapterFactory(IMMUTABLE_BIMAP) .registerTypeAdapterFactory(IMMUTABLE_SET) .registerTypeAdapterFactory(OPTIONAL) - .registerTypeAdapterFactory(new LocationTypeAdapterFactory(moduleFilePath)) + .registerTypeAdapterFactory(new LocationTypeAdapterFactory(moduleFilePath, workspaceRoot)) .registerTypeAdapter(Label.class, LABEL_TYPE_ADAPTER) .registerTypeAdapter(Version.class, VERSION_TYPE_ADAPTER) .registerTypeAdapter(ModuleKey.class, MODULE_KEY_TYPE_ADAPTER) diff --git a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py index 65dd87d667e367..9d5b6c15d89c0a 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -1286,6 +1286,7 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self): def testLockfileWithNoUserSpecificPath(self): self.my_registry = BazelRegistry(os.path.join(self._test_cwd, 'registry')) + self.my_registry.setModuleBasePath('projects') patch_file = self.ScratchFile( 'ss.patch', [ @@ -1301,9 +1302,27 @@ def testLockfileWithNoUserSpecificPath(self): ' }', ], ) + # Module with a local patch & extension self.my_registry.createCcModule( - 'ss', '1.3-1', patches=[patch_file], patch_strip=1 + 'ss', + '1.3-1', + {'ext': '1.0'}, + patches=[patch_file], + patch_strip=1, + extra_module_file_contents=[ + 'my_ext = use_extension("@ext//:ext.bzl", "ext")', + 'use_repo(my_ext, "justRepo")', + ], ) + ext_src = [ + 'def _repo_impl(ctx): ctx.file("BUILD")', + 'repo = repository_rule(_repo_impl)', + 'def _ext_impl(ctx): repo(name=justRepo)', + 'ext=module_extension(_ext_impl)', + ] + self.my_registry.createLocalPathModule('ext', '1.0', 'ext') + scratchFile(self.my_registry.projects.joinpath('ext', 'BUILD')) + scratchFile(self.my_registry.projects.joinpath('ext', 'ext.bzl'), ext_src) self.ScratchFile( 'MODULE.bazel', @@ -1318,10 +1337,14 @@ def testLockfileWithNoUserSpecificPath(self): with open('MODULE.bazel.lock', 'r') as json_file: lockfile = json.load(json_file) - remote_patches = lockfile['moduleDepGraph']['ss@1.3-1']['repoSpec'][ - 'attributes' - ]['remote_patches'] + ss_dep = lockfile['moduleDepGraph']['ss@1.3-1'] + remote_patches = ss_dep['repoSpec']['attributes']['remote_patches'] + ext_usage_location = ss_dep['extensionUsages'][0]['location']['file'] + + self.assertNotIn(self.my_registry.getURL(), ext_usage_location) + self.assertIn('%workspace%', ext_usage_location) for key in remote_patches.keys(): + self.assertNotIn(self.my_registry.getURL(), key) self.assertIn('%workspace%', key) def testExtensionEvaluationRerunsIfDepGraphOrderChanges(self):