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] Omit unique module versions from canonical repo names #21316

Merged
merged 2 commits into from
Feb 13, 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
4 changes: 3 additions & 1 deletion BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ pkg_tar(
write_file(
name = "gen_maven_repo_name",
out = "MAVEN_CANONICAL_REPO_NAME",
content = [get_canonical_repo_name("@maven")],
# TODO: Use this instead after building with Bazel 7.1.0 or later.
# content = [get_canonical_repo_name("@maven")],
content = ["rules_jvm_external~~maven~maven"],
)

# The @maven repository is created by maven_install from rules_jvm_external.
Expand Down
8 changes: 4 additions & 4 deletions MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions scripts/bootstrap/compile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ EOF
link_file "${PWD}/src/MODULE.tools" "${BAZEL_TOOLS_REPO}/MODULE.bazel"
new_hash=$(shasum -a 256 "${BAZEL_TOOLS_REPO}/MODULE.bazel" | awk '{print $1}')
sed -i.bak "/\"bazel_tools\":/s/\"[a-f0-9]*\"/\"$new_hash\"/" MODULE.bazel.lock
# TODO: Temporary hack for lockfile version mismatch, remove these lines after updating to 7.1.0
sed -i.bak 's/"lockFileVersion": 3/"lockFileVersion": 4/' MODULE.bazel.lock
# Replace canonical repository names and parts thereof of the form rules_foo~1.2.3 with rules_foo~
sed -i.bak -E 's/([a-z]([a-z0-9._-]*[a-z0-9]){0,1})~[a-zA-Z0-9.]{1,}(-[0-9.-]{1,}){0,1}(\+[0-9.-]{1,}){0,1}/\1/g' MODULE.bazel.lock
rm MODULE.bazel.lock.bak

mkdir -p "${BAZEL_TOOLS_REPO}/src/conditions"
Expand Down
6 changes: 3 additions & 3 deletions site/en/external/mod-command.md
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ use_repo(toolchains, my_jdk="remotejdk17_linux")
## [email protected]:
# <builtin>
http_archive(
name = "rules_cc~0.0.1",
name = "rules_cc~",
urls = ["https://bcr.bazel.build/test-mirror/github.com/bazelbuild/rules_cc/releases/download/0.0.1/rules_cc-0.0.1.tar.gz", "https://github.com/bazelbuild/rules_cc/releases/download/0.0.1/rules_cc-0.0.1.tar.gz"],
integrity = "sha256-Tcy/0iwN7xZMj0dFi9UODHFI89kgAs20WcKpamhJgkE=",
strip_prefix = "",
Expand All @@ -408,7 +408,7 @@ use_repo(toolchains, my_jdk="remotejdk17_linux")
## stardoc:
# <builtin>
http_archive(
name = "stardoc~0.5.0",
name = "stardoc~",
urls = ["https://bcr.bazel.build/test-mirror/github.com/bazelbuild/stardoc/releases/download/0.5.0/stardoc-0.5.0.tar.gz", "https://github.com/bazelbuild/stardoc/releases/download/0.5.0/stardoc-0.5.0.tar.gz"],
integrity = "sha256-yXlNzIAmow/2fPfPkeviRcopSyCwcYRdEsGSr+JDrXI=",
strip_prefix = "",
Expand Down Expand Up @@ -538,7 +538,7 @@ use_repo(toolchains, my_jdk="remotejdk17_linux")
## @remote_java_tools:
# <builtin>
http_archive(
name = "rules_java~5.0.0~toolchains~remote_java_tools",
name = "rules_java~~toolchains~remote_java_tools",
urls = ["https://mirror.bazel.build/bazel_java_tools/releases/java/v11.5/java_tools-v11.5.zip", "https://github.com/bazelbuild/java_tools/releases/download/java_v11.5/java_tools-v11.5.zip"],
sha256 = "b763ee80e5754e593fd6d5be6d7343f905bc8b73d661d36d842b024ca11b6793",
)
Expand Down
29 changes: 22 additions & 7 deletions site/en/external/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,19 +190,34 @@ flexibility.

## Repository names and strict deps

The [canonical name](/external/overview#canonical-repo-name) of a repo backing a
module is `{{ "<var>" }}module_name{{ "</var>" }}~{{ "<var>" }}version{{
"</var>" }}` (for example, `bazel_skylib~1.0.3`). For modules with a
non-registry override, replace the `{{ "<var>" }}version{{ "</var>" }}` part
with the string `override`. Note that the canonical name format is not an API
you should depend on and is subject to change at any time.

The [apparent name](/external/overview#apparent-repo-name) of a repo backing a
module to its direct dependents defaults to its module name, unless the
`repo_name` attribute of the [`bazel_dep`](/rules/lib/globals/module#bazel_dep)
directive says otherwise. Note that this means a module can only find its direct
dependencies. This helps prevent accidental breakages due to changes in
transitive dependencies.

The [canonical name](/external/overview#canonical-repo-name) of a repo backing a
module is either `{{ "<var>" }}module_name{{ "</var>" }}~{{ "<var>" }}version{{
"</var>" }}` (for example, `bazel_skylib~1.0.3`) or `{{ "<var>" }}module_name{{
"</var>" }}~` (for example, `bazel_features~`), depending on whether there are
multiple versions of the module in the entire dependency graph (see
[`multiple_version_override`](/rules/lib/globals/module#multiple_version_override)).
Note that **the canonical name format** is not an API you should depend on and
**is subject to change at any time**. Instead of hard-coding the canonical name,
use a supported way to get it directly from Bazel:
* In BUILD and `.bzl` files, use
[`Label.repo_name`](/rules/lib/builtins/Label#repo_name) on a `Label` instance
constructed from a label string given by the apparent name of the repo, e.g.,
`Label("@bazel_skylib").repo_name`.
* When looking up runfiles, use
[`$(rlocationpath ...)`](https://bazel.build/reference/be/make-variables#predefined_label_variables)
or one of the runfiles libraries in
`@bazel_tools//tools/{bash,cpp,java}/runfiles` or, for a ruleset `rules_foo`,
in `@rules_foo//foo/runfiles`.
* When interacting with Bazel from an external tool such as an IDE or language
server, use the `bazel mod dump_repo_mapping` command to get the mapping from
apparent names to canonical names for a given set of repositories.

[Module extensions](/external/extension) can also introduce additional repos
into the visible scope of a module.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package com.google.devtools.build.lib.bazel.bzlmod;

import com.google.auto.value.AutoValue;
import com.google.devtools.build.lib.cmdline.RepositoryName;

/**
* An abridged version of a {@link Module}, with a reduced set of information available, used for
Expand All @@ -30,10 +29,6 @@ public abstract class AbridgedModule {

public abstract ModuleKey getKey();

public final RepositoryName getCanonicalRepoName() {
return getKey().getCanonicalRepoName();
}

public static AbridgedModule from(Module module) {
return new AutoValue_AbridgedModule(module.getName(), module.getVersion(), module.getKey());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ java_library(
deps = [
":common",
":module_extension",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@
package com.google.devtools.build.lib.bazel.bzlmod;

import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.collect.ImmutableBiMap.toImmutableBiMap;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toSet;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand All @@ -31,6 +35,7 @@
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.LabelConverter;
import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
Expand All @@ -44,6 +49,7 @@
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -113,13 +119,11 @@ public SkyValue compute(SkyKey skyKey, Environment env)
depGraph = selectionResult.getResolvedDepGraph();
}

ImmutableMap<RepositoryName, ModuleKey> canonicalRepoNameLookup =
depGraph.keySet().stream()
.collect(toImmutableMap(ModuleKey::getCanonicalRepoName, key -> key));

ImmutableBiMap<RepositoryName, ModuleKey> canonicalRepoNameLookup =
computeCanonicalRepoNameLookup(depGraph);
ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> extensionUsagesById;
try {
extensionUsagesById = getExtensionUsagesById(depGraph);
extensionUsagesById = getExtensionUsagesById(depGraph, canonicalRepoNameLookup.inverse());
} catch (ExternalDepsException e) {
throw new BazelDepGraphFunctionException(e, Transience.PERSISTENT);
}
Expand Down Expand Up @@ -214,13 +218,23 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
public static ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
getExtensionUsagesById(ImmutableMap<ModuleKey, Module> depGraph)
throws ExternalDepsException {
return getExtensionUsagesById(depGraph, computeCanonicalRepoNameLookup(depGraph).inverse());
}

private static ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
getExtensionUsagesById(
ImmutableMap<ModuleKey, Module> depGraph,
ImmutableMap<ModuleKey, RepositoryName> moduleKeyToRepositoryNames)
throws ExternalDepsException {
ImmutableTable.Builder<ModuleExtensionId, ModuleKey, ModuleExtensionUsage>
extensionUsagesTableBuilder = ImmutableTable.builder();
for (Module module : depGraph.values()) {
RepositoryMapping repoMapping =
module.getRepoMappingWithBazelDepsOnly(moduleKeyToRepositoryNames);
LabelConverter labelConverter =
new LabelConverter(
PackageIdentifier.create(module.getCanonicalRepoName(), PathFragment.EMPTY_FRAGMENT),
module.getRepoMappingWithBazelDepsOnly());
PackageIdentifier.create(repoMapping.ownerRepo(), PathFragment.EMPTY_FRAGMENT),
module.getRepoMappingWithBazelDepsOnly(moduleKeyToRepositoryNames));
for (ModuleExtensionUsage usage : module.getExtensionUsages()) {
ModuleExtensionId moduleExtensionId;
try {
Expand Down Expand Up @@ -249,6 +263,37 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
return extensionUsagesTableBuilder.buildOrThrow();
}

private static ImmutableBiMap<RepositoryName, ModuleKey> computeCanonicalRepoNameLookup(
ImmutableMap<ModuleKey, Module> depGraph) {
// Find modules with multiple versions in the dep graph. Currently, the only source of such
// modules is multiple_version_override.
Set<String> multipleVersionsModules =
depGraph.keySet().stream()
.collect(groupingBy(ModuleKey::getName, counting()))
.entrySet()
.stream()
.filter(entry -> entry.getValue() > 1)
.map(Entry::getKey)
.collect(toSet());

// If there is a unique version of this module in the entire dep graph, we elide the version
// from the canonical repository name. This has a number of benefits:
// * It prevents the output base from being polluted with repository directories corresponding
// to outdated versions of modules, which can be large and would otherwise only be cleaned
// up by the discouraged bazel clean --expunge.
// * It improves cache hit rates by ensuring that a module update doesn't e.g. cause the paths
// of all toolchains provided by its extensions to change, which would result in widespread
// cache misses on every update.
return depGraph.keySet().stream()
.collect(
toImmutableBiMap(
key ->
multipleVersionsModules.contains(key.getName())
? key.getCanonicalRepoNameWithVersion()
: key.getCanonicalRepoNameWithoutVersion(),
key -> key));
}

private ImmutableBiMap<String, ModuleExtensionId> calculateUniqueNameForUsedExtensionId(
ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> extensionUsagesById) {
// Calculate a unique name for each used extension id with the following property that is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.common.collect.ImmutableMap.toImmutableMap;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableTable;
Expand Down Expand Up @@ -45,7 +46,7 @@ public static BazelDepGraphValue create(
ImmutableMap<ModuleExtensionId, String> extensionUniqueNames) {
return new AutoValue_BazelDepGraphValue(
depGraph,
canonicalRepoNameLookup,
ImmutableBiMap.copyOf(canonicalRepoNameLookup),
abridgedModules,
extensionUsagesTable,
extensionUniqueNames);
Expand All @@ -67,7 +68,12 @@ public static BazelDepGraphValue createEmptyDepGraph() {

ImmutableMap<RepositoryName, ModuleKey> canonicalRepoNameLookup =
emptyDepGraph.keySet().stream()
.collect(toImmutableMap(ModuleKey::getCanonicalRepoName, key -> key));
.collect(
toImmutableMap(
// All modules in the empty dep graph (just the root module) have an empty
// version, so the choice of including it in the canonical repo name does not
// matter.
ModuleKey::getCanonicalRepoNameWithoutVersion, key -> key));

return BazelDepGraphValue.create(
emptyDepGraph,
Expand All @@ -83,8 +89,8 @@ public static BazelDepGraphValue createEmptyDepGraph() {
*/
public abstract ImmutableMap<ModuleKey, Module> getDepGraph();

/** A mapping from a canonical repo name to the key of the module backing it. */
public abstract ImmutableMap<RepositoryName, ModuleKey> getCanonicalRepoNameLookup();
/** A mapping from a canonical repo name to the key of the module backing it and back. */
public abstract ImmutableBiMap<RepositoryName, ModuleKey> getCanonicalRepoNameLookup();

/** All modules in the same order as {@link #getDepGraph}, but with limited information. */
public abstract ImmutableList<AbridgedModule> getAbridgedModules();
Expand Down Expand Up @@ -124,7 +130,7 @@ public final RepositoryMapping getFullRepoMapping(ModuleKey key) {
}
return getDepGraph()
.get(key)
.getRepoMappingWithBazelDepsOnly()
.getRepoMappingWithBazelDepsOnly(getCanonicalRepoNameLookup().inverse())
.withAdditionalMappings(mapping.buildOrThrow());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
@GenerateTypeAdapter
public abstract class BazelLockFileValue implements SkyValue, Postable {

public static final int LOCK_FILE_VERSION = 3;
public static final int LOCK_FILE_VERSION = 4;

@SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
AugmentedModule::getName,
Collectors.mapping(AugmentedModule::getKey, toImmutableSet()))));

return BazelModuleInspectorValue.create(depGraph, modulesIndex, extensionToRepoInternalNames);
return BazelModuleInspectorValue.create(
depGraph,
modulesIndex,
extensionToRepoInternalNames,
depGraphValue.getCanonicalRepoNameLookup().inverse());
}

public static ImmutableMap<ModuleKey, AugmentedModule> computeAugmentedGraph(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.ImmutableSortedMap;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.skyframe.SkyKey;
Expand All @@ -42,9 +43,10 @@ public abstract class BazelModuleInspectorValue implements SkyValue {
public static BazelModuleInspectorValue create(
ImmutableMap<ModuleKey, AugmentedModule> depGraph,
ImmutableMap<String, ImmutableSet<ModuleKey>> modulesIndex,
ImmutableSetMultimap<ModuleExtensionId, String> extensionToRepoInternalNames) {
ImmutableSetMultimap<ModuleExtensionId, String> extensionToRepoInternalNames,
ImmutableMap<ModuleKey, RepositoryName> moduleKeyToCanonicalNames) {
return new AutoValue_BazelModuleInspectorValue(
depGraph, modulesIndex, extensionToRepoInternalNames);
depGraph, modulesIndex, extensionToRepoInternalNames, moduleKeyToCanonicalNames);
}

/**
Expand All @@ -69,6 +71,9 @@ public static BazelModuleInspectorValue create(
*/
public abstract ImmutableSetMultimap<ModuleExtensionId, String> getExtensionToRepoInternalNames();

/** A mapping from a module key to the canonical repository name of the module repository. */
public abstract ImmutableMap<ModuleKey, RepositoryName> getModuleKeyToCanonicalNames();

/**
* A wrapper for {@link Module}, augmented with references to dependants (and also those who are
* not used in the final dep graph).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,25 @@ public abstract class Module extends ModuleBase {
* Returns a {@link RepositoryMapping} with only Bazel module repos and no repos from module
* extensions. For the full mapping, see {@link BazelDepGraphValue#getFullRepoMapping}.
*/
public final RepositoryMapping getRepoMappingWithBazelDepsOnly() {
public final RepositoryMapping getRepoMappingWithBazelDepsOnly(
ImmutableMap<ModuleKey, RepositoryName> moduleKeyToRepositoryNames) {
ImmutableMap.Builder<String, RepositoryName> mapping = ImmutableMap.builder();
// If this is the root module, then the main repository should be visible as `@`.
if (getKey().equals(ModuleKey.ROOT)) {
mapping.put("", RepositoryName.MAIN);
}
// Every module should be able to reference itself as @<module repo name>.
// If this is the root module, this perfectly falls into @<module repo name> => @
RepositoryName owner = moduleKeyToRepositoryNames.get(getKey());
if (!getRepoName().isEmpty()) {
mapping.put(getRepoName(), getCanonicalRepoName());
mapping.put(getRepoName(), owner);
}
for (Map.Entry<String, ModuleKey> dep : getDeps().entrySet()) {
// Special note: if `dep` is actually the root module, its ModuleKey would be ROOT whose
// canonicalRepoName is the empty string. This perfectly maps to the main repo ("@").
mapping.put(dep.getKey(), dep.getValue().getCanonicalRepoName());
mapping.put(dep.getKey(), moduleKeyToRepositoryNames.get(dep.getValue()));
}
return RepositoryMapping.create(mapping.buildOrThrow(), getCanonicalRepoName());
return RepositoryMapping.create(mapping.buildOrThrow(), owner);
}

/**
Expand Down
Loading
Loading