Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.1.0] Remove user specific path from the lockfile (Fixes #19621) #21009

Merged
merged 2 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading