Skip to content

Commit

Permalink
Detect extension staleness due root module name/version change
Browse files Browse the repository at this point in the history
  • Loading branch information
@fmeum
fmeum committed Apr 25, 2024
1 parent c5cb07a commit 4326d90
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 15 deletions.
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ public Builder addExtensionUsage(ModuleExtensionUsage value) {

abstract String getName();

abstract Version getVersion();

abstract Optional<String> getRepoName();

abstract InterimModule autoBuild();
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ public void evaluate(
private ImmutableList<Reportable> generateFixupMessage(
Collection<ModuleExtensionUsage> usages, Set<String> allRepos) throws EvalException {
var rootUsages =
usages.stream()
.filter(usage -> usage.getUsingModule().equals(ModuleKey.ROOT))
.collect(toImmutableList());
usages.stream().filter(ModuleExtensionUsage::getIsRootUsage).collect(toImmutableList());
if (rootUsages.isEmpty()) {
// The root module doesn't use the current extension. Do not suggest fixes as the user isn't
// expected to modify any other module's MODULE.bazel file.
Expand Down
26 changes: 23 additions & 3 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,24 @@ public abstract class ModuleExtensionUsage {
*/
public abstract Optional<ModuleExtensionId.IsolationKey> getIsolationKey();

/** The module that contains this particular extension usage. */
public abstract ModuleKey getUsingModule();
/**
* The name of the module that contains this particular extension usage.
*
* <p>Not called, but included in the lockfile hash since it can be read by the extension.
*/
@SuppressWarnings("unused")
public abstract String getUsingModuleName();

/**
* The version of the module that contains this particular extension usage.
*
* <p>Not called, but included in the lockfile hash since it can be read by the extension.
*/
@SuppressWarnings("unused")
public abstract Version getUsingModuleVersion();

/** Whether the module containing the extension usage is the root module. */
public abstract boolean getIsRootUsage();

/**
* The location where this proxy object was created (by the {@code use_extension} call). Note that
Expand Down Expand Up @@ -135,7 +151,11 @@ public abstract static class Builder {

public abstract Builder setIsolationKey(Optional<ModuleExtensionId.IsolationKey> value);

public abstract Builder setUsingModule(ModuleKey value);
public abstract Builder setUsingModuleName(String value);

public abstract Builder setUsingModuleVersion(Version value);

public abstract Builder setIsRootUsage(boolean value);

public abstract Builder setLocation(Location value);

Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ ModuleExtensionUsage buildUsage() throws EvalException {
ModuleExtensionUsage.builder()
.setExtensionBzlFile(extensionBzlFile)
.setExtensionName(extensionName)
.setUsingModule(context.getModuleBuilder().getKey())
.setUsingModuleName(context.getModuleBuilder().getName())
.setUsingModuleVersion(context.getModuleBuilder().getVersion())
.setIsRootUsage(context.getModuleBuilder().getKey().equals(ModuleKey.ROOT))
.setLocation(location)
.setImports(ImmutableBiMap.copyOf(imports))
.setDevImports(devImports.build())
Expand Down
4 changes: 3 additions & 1 deletion src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ private static ModuleExtensionUsage createModuleExtensionUsage(
.setIsolationKey(Optional.empty())
.setImports(importsBuilder.buildOrThrow())
.setDevImports(ImmutableSet.of())
.setUsingModule(ModuleKey.ROOT)
.setUsingModuleName("foo")
.setUsingModuleVersion(Version.EMPTY)
.setIsRootUsage(true)
.setLocation(Location.BUILTIN)
.setHasDevUseExtension(false)
.setHasNonDevUseExtension(true)
Expand Down
24 changes: 18 additions & 6 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,9 @@ public void testModuleExtensions_good() throws Exception {
.setExtensionBzlFile("@mymod//:defs.bzl")
.setExtensionName("myext1")
.setIsolationKey(Optional.empty())
.setUsingModule(myMod)
.setUsingModuleName(myMod.getName())
.setUsingModuleVersion(myMod.getVersion())
.setIsRootUsage(false)
.setLocation(
Location.fromFileLineColumn(
"fake:0/modules/mymod/1.0/MODULE.bazel", 2, 23))
Expand All @@ -778,7 +780,9 @@ public void testModuleExtensions_good() throws Exception {
.setExtensionBzlFile("@mymod//:defs.bzl")
.setExtensionName("myext2")
.setIsolationKey(Optional.empty())
.setUsingModule(myMod)
.setUsingModuleName(myMod.getName())
.setUsingModuleVersion(myMod.getVersion())
.setIsRootUsage(false)
.setLocation(
Location.fromFileLineColumn(
"fake:0/modules/mymod/1.0/MODULE.bazel", 5, 23))
Expand Down Expand Up @@ -818,7 +822,9 @@ public void testModuleExtensions_good() throws Exception {
.setExtensionBzlFile("@rules_jvm_external//:defs.bzl")
.setExtensionName("maven")
.setIsolationKey(Optional.empty())
.setUsingModule(myMod)
.setUsingModuleName(myMod.getName())
.setUsingModuleVersion(myMod.getVersion())
.setIsRootUsage(false)
.setLocation(
Location.fromFileLineColumn(
"fake:0/modules/mymod/1.0/MODULE.bazel", 10, 22))
Expand Down Expand Up @@ -891,7 +897,9 @@ public void testModuleExtensions_duplicateProxy_asRoot() throws Exception {
.setExtensionBzlFile("@//:defs.bzl")
.setExtensionName("myext")
.setIsolationKey(Optional.empty())
.setUsingModule(ModuleKey.ROOT)
.setUsingModuleName("")
.setUsingModuleVersion(Version.EMPTY)
.setIsRootUsage(true)
.setLocation(Location.fromFileLineColumn("/workspace/MODULE.bazel", 1, 23))
.setImports(
ImmutableBiMap.of(
Expand Down Expand Up @@ -991,7 +999,9 @@ public void testModuleExtensions_duplicateProxy_asDep() throws Exception {
.setExtensionBzlFile("@mymod//:defs.bzl")
.setExtensionName("myext")
.setIsolationKey(Optional.empty())
.setUsingModule(myMod)
.setUsingModuleName(myMod.getName())
.setUsingModuleVersion(myMod.getVersion())
.setIsRootUsage(false)
.setLocation(
Location.fromFileLineColumn(
"fake:0/modules/mymod/1.0/MODULE.bazel", 5, 23))
Expand Down Expand Up @@ -1096,7 +1106,9 @@ public void testModuleExtensions_innate() throws Exception {
.setExtensionBzlFile("//:MODULE.bazel")
.setExtensionName("_repo_rules")
.setIsolationKey(Optional.empty())
.setUsingModule(ModuleKey.ROOT)
.setUsingModuleName("")
.setUsingModuleVersion(Version.EMPTY)
.setIsRootUsage(true)
.setLocation(Location.fromFile("/workspace/MODULE.bazel"))
.setImports(
ImmutableBiMap.of(
Expand Down
4 changes: 3 additions & 1 deletion src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ private static ModuleExtensionUsage.Builder getBaseUsageBuilder() {
.setExtensionBzlFile("//:rje.bzl")
.setExtensionName("maven")
.setIsolationKey(Optional.empty())
.setUsingModule(ModuleKey.ROOT)
.setUsingModuleName("")
.setUsingModuleVersion(Version.EMPTY)
.setIsRootUsage(true)
.setLocation(Location.BUILTIN)
.setImports(ImmutableBiMap.of())
.setDevImports(ImmutableSet.of())
Expand Down
103 changes: 103 additions & 0 deletions src/test/py/bazel/bzlmod/bazel_lockfile_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2114,6 +2114,109 @@ def testForceWatchingDirentsOutsideWorkspaceFails(self):
'attempted to watch path outside workspace', '\n'.join(stderr)
)

def testModuleExtensionRerunsOnNameChange(self):
self.ScratchFile(
'MODULE.bazel',
[
'module(name = "old_name", version = "1.2.3", repo_name = "fixed_name")',
'lockfile_ext = use_extension("//:extension.bzl", "lockfile_ext")',
'use_repo(lockfile_ext, "hello")',
],
)
self.ScratchFile('BUILD.bazel')
self.ScratchFile(
'extension.bzl',
[
'def _repo_rule_impl(ctx):',
' ctx.file("BUILD", "filegroup(name=\'lala\')")',
'',
'repo_rule = repository_rule(implementation=_repo_rule_impl)',
'',
'def _module_ext_impl(ctx):',
' print("I am running the extension: " + ctx.modules[0].name)',
' repo_rule(name="hello")',
'',
'lockfile_ext = module_extension(',
' implementation=_module_ext_impl',
')',
],
)

_, _, stderr = self.RunBazel(['build', '@hello//:all'])
stderr = ''.join(stderr)
self.assertIn('I am running the extension: old_name', stderr)

# Shutdown bazel to empty cache and run with no changes
self.RunBazel(['shutdown'])
_, _, stderr = self.RunBazel(['build', '@hello//:all'])
stderr = ''.join(stderr)
self.assertNotIn('I am running the extension: old_name', stderr)

# Update module name and rerun
self.RunBazel(['shutdown'])
self.ScratchFile(
'MODULE.bazel',
[
'module(name = "new_name", version = "1.2.3", repo_name = "fixed_name")',
'lockfile_ext = use_extension("//:extension.bzl", "lockfile_ext")',
'use_repo(lockfile_ext, "hello")',
],
)
_, _, stderr = self.RunBazel(['build', '@hello//:all'])
stderr = ''.join(stderr)
self.assertIn('I am running the extension: new_name', stderr)

def testModuleExtensionRerunsOnVersionChange(self):
self.ScratchFile(
'MODULE.bazel',
[
'module(name = "old_name", version = "1.2.3")',
'lockfile_ext = use_extension("extension.bzl", "lockfile_ext")',
'use_repo(lockfile_ext, "hello")',
],
)
self.ScratchFile('BUILD.bazel')
self.ScratchFile(
'extension.bzl',
[
'def _repo_rule_impl(ctx):',
' ctx.file("BUILD", "filegroup(name=\'lala\')")',
'',
'repo_rule = repository_rule(implementation=_repo_rule_impl)',
'',
'def _module_ext_impl(ctx):',
' print("I am running the extension: " + ctx.modules[0].version)',
' repo_rule(name="hello")',
'',
'lockfile_ext = module_extension(',
' implementation=_module_ext_impl',
')',
],
)

_, _, stderr = self.RunBazel(['build', '@hello//:all'])
stderr = ''.join(stderr)
self.assertIn('I am running the extension: 1.2.3', stderr)

# Shutdown bazel to empty cache and run with no changes
self.RunBazel(['shutdown'])
_, _, stderr = self.RunBazel(['build', '@hello//:all'])
stderr = ''.join(stderr)
self.assertNotIn('I am running the extension: 1.2.3', stderr)

# Update module name and rerun
self.RunBazel(['shutdown'])
self.ScratchFile(
'MODULE.bazel',
[
'module(name = "old_name", version = "4.5.6")',
'lockfile_ext = use_extension("extension.bzl", "lockfile_ext")',
'use_repo(lockfile_ext, "hello")',
],
)
_, _, stderr = self.RunBazel(['build', '@hello//:all'])
stderr = ''.join(stderr)
self.assertIn('I am running the extension: 4.5.6', stderr)

if __name__ == '__main__':
absltest.main()

0 comments on commit 4326d90

Please sign in to comment.