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

Do not rerun module extensions when only imports or locations change #19253

Closed
Closed
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
24 changes: 23 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ java_library(
":common",
":inspection",
":module_extension",
":module_extension_metadata",
":registry",
":repo_rule_creator",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
Expand Down Expand Up @@ -168,7 +169,6 @@ java_library(
"Discovery.java",
"GsonTypeAdapterUtil.java",
"ModuleExtensionContext.java",
"ModuleExtensionMetadata.java",
"ModuleFileFunction.java",
"ModuleFileGlobals.java",
"Selection.java",
Expand All @@ -182,6 +182,7 @@ java_library(
":common",
":exception",
":module_extension",
":module_extension_metadata",
":registry",
":resolution",
"//src/main/java/com/google/devtools/build/docgen/annot",
Expand Down Expand Up @@ -282,3 +283,24 @@ java_library(
"//third_party:jsr305",
],
)

java_library(
name = "module_extension_metadata",
srcs = [
"ModuleExtensionMetadata.java",
],
deps = [
":common",
":module_extension",
"//src/main/java/com/google/devtools/build/docgen/annot",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/eval",
"//src/main/java/net/starlark/java/syntax",
"//third_party:auto_value",
"//third_party:gson",
"//third_party:guava",
"//third_party:jsr305",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public ImmutableList<String> getModuleExtensionDiff(
extDiff.add("One or more files the extension '" + extensionId + "' is using have changed");
}
if (!extensionUsages.equals(lockedExtensionUsages)) {
extDiff.add("The usages of the extension '" + extensionId + "' has changed");
extDiff.add("The usages of the extension '" + extensionId + "' have changed");
}
if (!envVariables.equals(lockedExtension.getEnvVariables())) {
extDiff.add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
import java.util.Optional;

/**
* This object serves as a container for the transitive digest (obtained from transitive .bzl files)
Expand All @@ -33,7 +34,8 @@ public static Builder builder() {
return new AutoValue_LockFileModuleExtension.Builder()
// TODO(salmasamy) can be removed when updating lockfile version
.setEnvVariables(ImmutableMap.of())
.setAccumulatedFileDigests(ImmutableMap.of());
.setAccumulatedFileDigests(ImmutableMap.of())
.setModuleExtensionMetadata(Optional.empty());
}

@SuppressWarnings("mutable")
Expand All @@ -45,6 +47,8 @@ public static Builder builder() {

public abstract ImmutableMap<String, RepoSpec> getGeneratedRepoSpecs();

public abstract Optional<ModuleExtensionMetadata> getModuleExtensionMetadata();

public abstract Builder toBuilder();

/** Builder type for {@link LockFileModuleExtension}. */
Expand All @@ -59,6 +63,8 @@ public abstract static class Builder {

public abstract Builder setGeneratedRepoSpecs(ImmutableMap<String, RepoSpec> value);

public abstract Builder setModuleExtensionMetadata(Optional<ModuleExtensionMetadata> value);

public abstract LockFileModuleExtension build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.stream.Collectors.joining;

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
Expand All @@ -27,6 +28,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashSet;
Expand All @@ -49,24 +51,29 @@
doc =
"Return values of this type from a module extension's implementation function to "
+ "provide metadata about the repositories generated by the extension to Bazel.")
public class ModuleExtensionMetadata implements StarlarkValue {
@Nullable private final ImmutableSet<String> explicitRootModuleDirectDeps;
@Nullable private final ImmutableSet<String> explicitRootModuleDirectDevDeps;
private final UseAllRepos useAllRepos;
@AutoValue
@GenerateTypeAdapter
public abstract class ModuleExtensionMetadata implements StarlarkValue {
@Nullable
abstract ImmutableSet<String> getExplicitRootModuleDirectDeps();

private ModuleExtensionMetadata(
@Nullable
abstract ImmutableSet<String> getExplicitRootModuleDirectDevDeps();

abstract UseAllRepos getUseAllRepos();

private static ModuleExtensionMetadata create(
@Nullable Set<String> explicitRootModuleDirectDeps,
@Nullable Set<String> explicitRootModuleDirectDevDeps,
UseAllRepos useAllRepos) {
this.explicitRootModuleDirectDeps =
return new AutoValue_ModuleExtensionMetadata(
explicitRootModuleDirectDeps != null
? ImmutableSet.copyOf(explicitRootModuleDirectDeps)
: null;
this.explicitRootModuleDirectDevDeps =
: null,
explicitRootModuleDirectDevDeps != null
? ImmutableSet.copyOf(explicitRootModuleDirectDevDeps)
: null;
this.useAllRepos = useAllRepos;
: null,
useAllRepos);
}

static ModuleExtensionMetadata create(
Expand All @@ -76,19 +83,19 @@ static ModuleExtensionMetadata create(
throws EvalException {
if (rootModuleDirectDepsUnchecked == Starlark.NONE
&& rootModuleDirectDevDepsUnchecked == Starlark.NONE) {
return new ModuleExtensionMetadata(null, null, UseAllRepos.NO);
return create(null, null, UseAllRepos.NO);
}

// When root_module_direct_deps = "all", accept both root_module_direct_dev_deps = None and
// root_module_direct_dev_deps = [], but not root_module_direct_dev_deps = ["some_repo"].
if (rootModuleDirectDepsUnchecked.equals("all")
&& rootModuleDirectDevDepsUnchecked.equals(StarlarkList.immutableOf())) {
return new ModuleExtensionMetadata(null, null, UseAllRepos.REGULAR);
return create(null, null, UseAllRepos.REGULAR);
}

if (rootModuleDirectDevDepsUnchecked.equals("all")
&& rootModuleDirectDepsUnchecked.equals(StarlarkList.immutableOf())) {
return new ModuleExtensionMetadata(null, null, UseAllRepos.DEV);
return create(null, null, UseAllRepos.DEV);
}

if (rootModuleDirectDepsUnchecked.equals("all")
Expand Down Expand Up @@ -146,8 +153,7 @@ static ModuleExtensionMetadata create(
}
}

return new ModuleExtensionMetadata(
explicitRootModuleDirectDeps, explicitRootModuleDirectDevDeps, UseAllRepos.NO);
return create(explicitRootModuleDirectDeps, explicitRootModuleDirectDevDeps, UseAllRepos.NO);
}

public void evaluate(
Expand Down Expand Up @@ -358,18 +364,18 @@ private static Optional<String> makeUseRepoCommand(

private Optional<ImmutableSet<String>> getRootModuleDirectDeps(Set<String> allRepos)
throws EvalException {
switch (useAllRepos) {
switch (getUseAllRepos()) {
case NO:
if (explicitRootModuleDirectDeps != null) {
Set<String> invalidRepos = Sets.difference(explicitRootModuleDirectDeps, allRepos);
if (getExplicitRootModuleDirectDeps() != null) {
Set<String> invalidRepos = Sets.difference(getExplicitRootModuleDirectDeps(), allRepos);
if (!invalidRepos.isEmpty()) {
throw Starlark.errorf(
"root_module_direct_deps contained the following repositories "
+ "not generated by the extension: %s",
String.join(", ", invalidRepos));
}
}
return Optional.ofNullable(explicitRootModuleDirectDeps);
return Optional.ofNullable(getExplicitRootModuleDirectDeps());
case REGULAR:
return Optional.of(ImmutableSet.copyOf(allRepos));
case DEV:
Expand All @@ -380,18 +386,19 @@ private Optional<ImmutableSet<String>> getRootModuleDirectDeps(Set<String> allRe

private Optional<ImmutableSet<String>> getRootModuleDirectDevDeps(Set<String> allRepos)
throws EvalException {
switch (useAllRepos) {
switch (getUseAllRepos()) {
case NO:
if (explicitRootModuleDirectDevDeps != null) {
Set<String> invalidRepos = Sets.difference(explicitRootModuleDirectDevDeps, allRepos);
if (getExplicitRootModuleDirectDevDeps() != null) {
Set<String> invalidRepos =
Sets.difference(getExplicitRootModuleDirectDevDeps(), allRepos);
if (!invalidRepos.isEmpty()) {
throw Starlark.errorf(
"root_module_direct_dev_deps contained the following "
+ "repositories not generated by the extension: %s",
String.join(", ", invalidRepos));
}
}
return Optional.ofNullable(explicitRootModuleDirectDevDeps);
return Optional.ofNullable(getExplicitRootModuleDirectDevDeps());
case REGULAR:
return Optional.of(ImmutableSet.of());
case DEV:
Expand All @@ -400,7 +407,7 @@ private Optional<ImmutableSet<String>> getRootModuleDirectDevDeps(Set<String> al
throw new IllegalStateException("not reached");
}

private enum UseAllRepos {
enum UseAllRepos {
NO,
REGULAR,
DEV,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ public abstract class ModuleExtensionUsage {
*/
public abstract boolean getHasNonDevUseExtension();

public abstract Builder toBuilder();

public static Builder builder() {
return new AutoValue_ModuleExtensionUsage.Builder();
}
Expand Down
Loading
Loading