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

[6.3.0] Fix absolute file paths showing up in lockfiles #18993

Merged
merged 2 commits into from
Jul 19, 2023
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 @@ -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,102 @@ 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();
}

@Nullable
@Override
@SuppressWarnings("unchecked")
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() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.NonRootModuleFileValue;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.StarlarkExportable;
Expand All @@ -37,6 +39,7 @@
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
Expand Down Expand Up @@ -199,11 +202,11 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir
if (env.getValue(FileValue.key(moduleFilePath)) == null) {
return null;
}
ModuleFile moduleFile = readModuleFile(moduleFilePath.asPath());
String moduleFileHash = new Fingerprint().addBytes(moduleFile.getContent()).hexDigestAndReset();
byte[] moduleFileContents = readModuleFile(moduleFilePath.asPath());
String moduleFileHash = new Fingerprint().addBytes(moduleFileContents).hexDigestAndReset();
ModuleFileGlobals moduleFileGlobals =
execModuleFile(
moduleFile,
ModuleFile.create(moduleFileContents, moduleFilePath.asPath().toString()),
/* registry= */ null,
ModuleKey.ROOT,
/* ignoreDevDeps= */ Objects.requireNonNull(IGNORE_DEV_DEPS.get(env)),
Expand Down Expand Up @@ -335,8 +338,15 @@ private GetModuleFileResult getModuleFile(
if (env.getValue(FileValue.key(moduleFilePath)) == null) {
return null;
}
Label moduleFileLabel =
Label.createUnvalidated(
PackageIdentifier.create(key.getCanonicalRepoName(), PathFragment.EMPTY_FRAGMENT),
LabelConstants.MODULE_DOT_BAZEL_FILE_NAME.getBaseName());
GetModuleFileResult result = new GetModuleFileResult();
result.moduleFile = readModuleFile(moduleFilePath.asPath());
result.moduleFile =
ModuleFile.create(
readModuleFile(moduleFilePath.asPath()),
moduleFileLabel.getUnambiguousCanonicalForm());
return result;
}

Expand Down Expand Up @@ -392,10 +402,9 @@ private GetModuleFileResult getModuleFile(
throw errorf(Code.MODULE_NOT_FOUND, "module not found in registries: %s", key);
}

private static ModuleFile readModuleFile(Path path) throws ModuleFileFunctionException {
private static byte[] readModuleFile(Path path) throws ModuleFileFunctionException {
try {
return ModuleFile.create(
FileSystemUtils.readWithKnownFileSize(path, path.getFileSize()), path.getPathString());
return FileSystemUtils.readWithKnownFileSize(path, path.getFileSize());
} catch (IOException e) {
throw errorf(Code.MODULE_NOT_FOUND, "MODULE.bazel expected but not found at %s", path);
}
Expand Down
64 changes: 64 additions & 0 deletions src/test/py/bazel/bzlmod/bazel_lockfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,70 @@ 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},',
')',
],
)

# Paths to module files in error message always use forward slashes as
# separators, even on Windows.
module_file_path = self.Path('MODULE.bazel').replace('\\', '/')

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()
Loading