Skip to content

Commit

Permalink
[7.1.0] Remove user specific path from the lockfile (Fixes #19621) (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
SalmaSamy authored Jan 24, 2024
1 parent e67f968 commit 9d12d79
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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
Expand All @@ -348,18 +350,21 @@ public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> 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()
Expand All @@ -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)
Expand Down
31 changes: 27 additions & 4 deletions src/test/py/bazel/bzlmod/bazel_lockfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
[
Expand All @@ -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',
Expand All @@ -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):
Expand Down

0 comments on commit 9d12d79

Please sign in to comment.