Skip to content

Commit

Permalink
Inline old ModuleKey methods
Browse files Browse the repository at this point in the history
`create`, `getName`, and `getVersion` are just wrappers (around `new`, `name`, and `version`, respectively) now that ModuleKey is a record.

PiperOrigin-RevId: 676559957
Change-Id: I381ee94cfb7cbb377e62e75a05bf950ea794b625
  • Loading branch information
Wyverald authored and copybara-github committed Sep 19, 2024
1 parent ce64b1a commit f301171
Show file tree
Hide file tree
Showing 20 changed files with 69 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private static ImmutableBiMap<RepositoryName, ModuleKey> computeCanonicalRepoNam
// modules is multiple_version_override.
ImmutableSet<String> multipleVersionsModules =
depGraph.keySet().stream()
.collect(groupingBy(ModuleKey::getName, counting()))
.collect(groupingBy(ModuleKey::name, counting()))
.entrySet()
.stream()
.filter(entry -> entry.getValue() > 1)
Expand All @@ -150,7 +150,7 @@ private static ImmutableBiMap<RepositoryName, ModuleKey> computeCanonicalRepoNam
.collect(
toImmutableBiMap(
key ->
multipleVersionsModules.contains(key.getName())
multipleVersionsModules.contains(key.name())
? key.getCanonicalRepoNameWithVersion()
: key.getCanonicalRepoNameWithoutVersion(),
key -> key));
Expand Down Expand Up @@ -188,8 +188,8 @@ private static String makeUniqueNameCandidate(ModuleExtensionId id, int attempt)
id.getBzlFileLabel().getRepository().getName(),
id.getExtensionName(),
extensionNameDisambiguator,
isolationKey.getModule().getName(),
isolationKey.getModule().getVersion(),
isolationKey.getModule().name(),
isolationKey.getModule().version(),
isolationKey.getUsageExportedName()))
.orElse(
id.getBzlFileLabel().getRepository().getName()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ public static ImmutableMap<ModuleKey, AugmentedModule> computeAugmentedGraph(
}

ResolutionReason reason = ResolutionReason.ORIGINAL;
if (!key.getVersion().equals(originalKey.getVersion())) {
ModuleOverride override = overrides.get(key.getName());
if (!key.version().equals(originalKey.version())) {
ModuleOverride override = overrides.get(key.name());
if (override != null) {
if (override instanceof SingleVersionOverride) {
reason = ResolutionReason.SINGLE_VERSION_OVERRIDE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ public boolean isUsed() {
/** Returns a new {@link AugmentedModule.Builder} with {@code key} set. */
public static AugmentedModule.Builder builder(ModuleKey key) {
return new AutoValue_BazelModuleInspectorValue_AugmentedModule.Builder()
.setName(key.getName())
.setVersion(key.getVersion())
.setName(key.name())
.setVersion(key.version())
.setKey(key)
.setLoaded(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ private static ImmutableMap<ModuleKey, String> checkNoYankedVersions(
// No yanked version information available for this module.
continue;
}
String yankedInfo = yankedVersionsValue.yankedVersions().get().get(key.getVersion());
String yankedInfo = yankedVersionsValue.yankedVersions().get().get(key.version());
if (yankedInfo == null) {
// The selected version is not yanked.
continue;
Expand Down Expand Up @@ -384,7 +384,7 @@ private static ImmutableMap<ModuleKey, Module> computeFinalDepGraph(
entry.getKey(),
InterimModule.toModule(
entry.getValue(),
overrides.get(entry.getKey().getName()),
overrides.get(entry.getKey().name()),
remoteRepoSpecs.get(entry.getKey())));
}
return finalDepGraph.buildOrThrow();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,7 @@ private Optional<byte[]> doGrabFile(
public Optional<ModuleFile> getModuleFile(ModuleKey key, ExtendedEventHandler eventHandler)
throws IOException, InterruptedException {
String url =
constructUrl(
getUrl(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel");
constructUrl(getUrl(), "modules", key.name(), key.version().toString(), "MODULE.bazel");
Optional<byte[]> maybeContent = grabFile(url, eventHandler, /* useChecksum= */ true);
return maybeContent.map(content -> ModuleFile.create(content, url));
}
Expand Down Expand Up @@ -341,7 +340,7 @@ public RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler)

private String getSourceJsonUrl(ModuleKey key) {
return constructUrl(
getUrl(), "modules", key.getName(), key.getVersion().toString(), SOURCE_JSON_FILENAME);
getUrl(), "modules", key.name(), key.version().toString(), SOURCE_JSON_FILENAME);
}

private Optional<BazelRegistryJson> getBazelRegistryJson(ExtendedEventHandler eventHandler)
Expand Down Expand Up @@ -432,8 +431,8 @@ private RepoSpec createArchiveRepoSpec(
constructUrl(
getUrl(),
"modules",
key.getName(),
key.getVersion().toString(),
key.name(),
key.version().toString(),
"patches",
entry.getKey()),
entry.getValue());
Expand All @@ -455,8 +454,8 @@ private RepoSpec createArchiveRepoSpec(
constructUrl(
getUrl(),
"modules",
key.getName(),
key.getVersion().toString(),
key.name(),
key.version().toString(),
"overlay",
entry.getKey())))));

Expand Down Expand Up @@ -529,7 +528,7 @@ public Optional<YankedVersionsValue> tryGetYankedVersionsFromLockfile(
// access if the set of yanked versions has not changed, but the set allowed versions has.
return Optional.of(
YankedVersionsValue.create(
Optional.of(ImmutableMap.of(selectedModuleKey.getVersion(), yankedInfo))));
Optional.of(ImmutableMap.of(selectedModuleKey.version(), yankedInfo))));
}
if (knownFileHashes.containsKey(getSourceJsonUrl(selectedModuleKey))) {
// If the source.json hash is recorded in the lockfile, we know that the module was selected
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ public static DepSpec create(String name, Version version, int maxCompatibilityL
}

public static DepSpec fromModuleKey(ModuleKey key) {
return create(key.getName(), key.getVersion(), -1);
return create(key.name(), key.version(), -1);
}

public final ModuleKey toModuleKey() {
return ModuleKey.create(getName(), getVersion());
return new ModuleKey(getName(), getVersion());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,14 +205,14 @@ public SkyValue compute(SkyKey skyKey, Environment env)
env.getListener().handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack()));
throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for %s", moduleKey);
}
if (!module.getName().equals(moduleKey.getName())) {
if (!module.getName().equals(moduleKey.name())) {
throw errorf(
Code.BAD_MODULE,
"the MODULE.bazel file of %s declares a different name (%s)",
moduleKey,
module.getName());
}
if (!moduleKey.getVersion().isEmpty() && !module.getVersion().equals(moduleKey.getVersion())) {
if (!moduleKey.version().isEmpty() && !module.getVersion().equals(moduleKey.version())) {
throw errorf(
Code.BAD_MODULE,
"the MODULE.bazel file of %s declares a different version (%s)",
Expand Down Expand Up @@ -472,8 +472,7 @@ public static RootModuleFileValue evaluateRootModuleFile(
toImmutableMap(
// A module with a non-registry override always has a unique version across the
// entire dep graph.
name ->
ModuleKey.create(name, Version.EMPTY).getCanonicalRepoNameWithoutVersion(),
name -> new ModuleKey(name, Version.EMPTY).getCanonicalRepoNameWithoutVersion(),
name -> name));
ImmutableSet<PathFragment> moduleFilePaths =
Stream.concat(
Expand Down Expand Up @@ -572,14 +571,14 @@ private GetModuleFileResult getModuleFile(
}

// Otherwise, we should get the module file from a registry.
if (key.getVersion().isEmpty()) {
if (key.version().isEmpty()) {
// Print a friendlier error message if the user forgets to specify a version *and* doesn't
// have a non-registry override.
throw errorf(
Code.MODULE_NOT_FOUND,
"bad bazel_dep on module '%s' with no version. Did you forget to specify a version, or a"
+ " non-registry override?",
key.getName());
key.name());
}
ImmutableSet<String> registries = Objects.requireNonNull(REGISTRIES.get(env));
if (override instanceof RegistryOverride registryOverride) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.errorprone.annotations.InlineMe;
import java.util.Comparator;
import java.util.List;

/** A module name, version pair that identifies a module in the external dependency graph. */
/**
* A module name, version pair that identifies a node in the external dependency graph.
*
* @param name The name of the module. Must be empty for the root module.
* @param version The version of the module. Must be empty iff the module has a {@link
* NonRegistryOverride}.
*/
@AutoCodec
public record ModuleKey(String name, Version version) {

Expand Down Expand Up @@ -54,39 +59,18 @@ public record ModuleKey(String name, Version version) {
public static final ModuleKey ROOT = new ModuleKey("", Version.EMPTY);

public static final Comparator<ModuleKey> LEXICOGRAPHIC_COMPARATOR =
Comparator.comparing(ModuleKey::getName).thenComparing(ModuleKey::getVersion);

/**
* @deprecated Call the constructor directly.
*/
@InlineMe(
replacement = "new ModuleKey(name, version)",
imports = "com.google.devtools.build.lib.bazel.bzlmod.ModuleKey")
@Deprecated
public static ModuleKey create(String name, Version version) {
return new ModuleKey(name, version);
}

/** The name of the module. Must be empty for the root module. */
public String getName() {
return name();
}

/** The version of the module. Must be empty iff the module has a {@link NonRegistryOverride}. */
public Version getVersion() {
return version();
}
Comparator.comparing(ModuleKey::name).thenComparing(ModuleKey::version);

@Override
public final String toString() {
public String toString() {
if (this.equals(ROOT)) {
return "<root>";
}
return getName() + "@" + (getVersion().isEmpty() ? "_" : getVersion().toString());
return name + "@" + (version.isEmpty() ? "_" : version.toString());
}

/** Returns a string such as "root module" or "module [email protected]" for display purposes. */
public final String toDisplayString() {
public String toDisplayString() {
if (this.equals(ROOT)) {
return "root module";
}
Expand All @@ -113,8 +97,8 @@ public RepositoryName getCanonicalRepoNameWithoutVersion() {
}

private RepositoryName getCanonicalRepoName(boolean includeVersion) {
if (WELL_KNOWN_MODULES.containsKey(getName())) {
return WELL_KNOWN_MODULES.get(getName());
if (WELL_KNOWN_MODULES.containsKey(name)) {
return WELL_KNOWN_MODULES.get(name);
}
if (ROOT.equals(this)) {
return RepositoryName.MAIN;
Expand All @@ -123,8 +107,8 @@ private RepositoryName getCanonicalRepoName(boolean includeVersion) {
if (includeVersion) {
// getVersion().isEmpty() is true only for modules with non-registry overrides, which enforce
// that there is a single version of the module in the dep graph.
Preconditions.checkState(!getVersion().isEmpty());
suffix = getVersion().toString();
Preconditions.checkState(!version.isEmpty());
suffix = version.toString();
} else {
// This results in canonical repository names such as `rules_foo+` for the module `rules_foo`.
// This particular format is chosen since:
Expand All @@ -142,7 +126,7 @@ private RepositoryName getCanonicalRepoName(boolean includeVersion) {
// rarely used.
suffix = "";
}
return RepositoryName.createUnvalidated(String.format("%s+%s", getName(), suffix));
return RepositoryName.createUnvalidated(String.format("%s+%s", name, suffix));
}

public static ModuleKey fromString(String s) throws Version.ParseException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public void addOverride(String moduleName, ModuleOverride override) throws EvalE
public InterimModule buildModule(@Nullable Registry registry) throws EvalException {
// Add builtin modules as default deps of the current module.
for (String builtinModule : builtinModules.keySet()) {
if (module.getKey().getName().equals(builtinModule)) {
if (module.getKey().name().equals(builtinModule)) {
// The built-in module does not depend on itself.
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ static ModuleNameAndCompatibilityLevel create(String moduleName, int compatibili
ImmutableList<Version> allowedVersions = ((MultipleVersionOverride) override).getVersions();
for (Version allowedVersion : allowedVersions) {
InterimModule allowedVersionModule =
depGraph.get(ModuleKey.create(moduleName, allowedVersion));
depGraph.get(new ModuleKey(moduleName, allowedVersion));
if (allowedVersionModule == null) {
throw ExternalDepsException.withMessage(
Code.VERSION_RESOLUTION_ERROR,
Expand Down Expand Up @@ -198,10 +198,10 @@ private static SelectionGroup computeSelectionGroup(
if (allowedVersionSet == null) {
// This means that this module has no multiple-version override.
return SelectionGroup.create(
module.getKey().getName(), module.getCompatibilityLevel(), Version.EMPTY);
module.getKey().name(), module.getCompatibilityLevel(), Version.EMPTY);
}
return SelectionGroup.create(
module.getKey().getName(),
module.getKey().name(),
module.getCompatibilityLevel(),
// We use the `ceiling` method here to quickly locate the lowest allowed version that's
// still no lower than this module's version.
Expand Down Expand Up @@ -248,7 +248,7 @@ private static ImmutableList<ModuleKey> computePossibleResolutionResultsForOneDe
Comparators::min))
.values()
.stream()
.map(v -> ModuleKey.create(depSpec.getName(), v))
.map(v -> new ModuleKey(depSpec.getName(), v))
.collect(toImmutableList());
}

Expand Down Expand Up @@ -320,7 +320,7 @@ public static Result run(
for (Map.Entry<ModuleKey, SelectionGroup> entry : selectionGroups.entrySet()) {
ModuleKey key = entry.getKey();
SelectionGroup selectionGroup = entry.getValue();
selectedVersions.merge(selectionGroup, key.getVersion(), Comparators::max);
selectedVersions.merge(selectionGroup, key.version(), Comparators::max);
}

// Compute the possible list of ModuleKeys that each DepSpec could resolve to.
Expand Down Expand Up @@ -431,8 +431,7 @@ void visit(
@Nullable ModuleKey from,
HashMap<String, ExistingModule> moduleByName)
throws ExternalDepsException {
ModuleOverride override = overrides.get(key.getName());
if (override instanceof MultipleVersionOverride) {
if (overrides.get(key.name()) instanceof MultipleVersionOverride override) {
if (selectionGroups.get(key).getTargetAllowedVersion().isEmpty()) {
// This module has no target allowed version, which means that there's no allowed version
// higher than its version at the same compatibility level.
Expand All @@ -444,8 +443,8 @@ void visit(
+ " which allows only [%s]",
from,
key,
key.getName(),
JOINER.join(((MultipleVersionOverride) override).getVersions()));
key.name(),
JOINER.join(override.getVersions()));
}
} else {
ExistingModule existingModuleWithSameName =
Expand Down Expand Up @@ -486,7 +485,7 @@ void visit(
depSpec.toModuleKey(),
repoName,
previousRepoName,
key.getName());
key.name());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ private static boolean parseModuleKeysFromString(
pieces[1]);
}

allowedYankedVersionBuilder.add(ModuleKey.create(pieces[0], version));
allowedYankedVersionBuilder.add(new ModuleKey(pieces[0], version));
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ private String toId(ModuleKey key) {
return "\"<root>\"";
}
return String.format(
"\"%s@%s\"",
key.getName(), key.getVersion().equals(Version.EMPTY) ? "_" : key.getVersion());
"\"%s@%s\"", key.name(), key.version().equals(Version.EMPTY) ? "_" : key.version());
}

private String toId(ModuleExtensionId id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ JsonObject printModule(
AugmentedModule module = depGraph.get(key);
JsonObject json = new JsonObject();
json.addProperty("key", printKey(key));
if (!key.getName().equals(module.getName())) {
if (!key.name().equals(module.getName())) {
json.addProperty("name", module.getName());
}
if (!key.getVersion().equals(module.getVersion())) {
if (!key.version().equals(module.getVersion())) {
json.addProperty("version", module.getVersion().toString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -531,8 +531,8 @@ private void displayExtension(ModuleExtensionId extension, ImmutableSet<ModuleKe
}

private boolean isBuiltin(ModuleKey key) {
return key.equals(ModuleKey.create("bazel_tools", Version.EMPTY))
|| key.equals(ModuleKey.create("local_config_platform", Version.EMPTY));
return key.equals(new ModuleKey("bazel_tools", Version.EMPTY))
|| key.equals(new ModuleKey("local_config_platform", Version.EMPTY));
}

/** A node representing a module that forms the result graph. */
Expand Down
Loading

0 comments on commit f301171

Please sign in to comment.