From aa9a669e0cf5cb938335a6a361363ff11a49963f Mon Sep 17 00:00:00 2001 From: Xdng Yng Date: Mon, 29 Jul 2024 08:04:13 -0700 Subject: [PATCH] Add flag --incompatible_use_plus_in_repo_names Defaults to false. When set to true, we use `+` instead of `~` as the separator in canonical repo names. Some more subtle changes include: - We now officially say that the "build" part of version strings (the part that begins with a plus) is ignored and stripped. - When the flag is set to true, we effectively increase the lockfile version by 1 (see code comment in BazelLockFileModule). - When true, we no longer insert a `_main` in front of names of repos generated by module extensions hosted in the main repo. (`~abc` as a name was problematic, but `+abc` is not.) - When true, we no longer insert a `v` in front of numerical versions in canonical repo names. (`my_mod~1.1` could be a Windows short path, but `my_mod+1.1` cannot.) Work towards https://github.com/bazelbuild/bazel/issues/22865. Closes #23103. PiperOrigin-RevId: 657202616 Change-Id: I015b2a04a823b1d951015a1b2e1b99b154dcc5a2 --- MODULE.bazel | 2 +- MODULE.bazel.lock | 3 +- src/MODULE.tools | 2 +- .../devtools/build/lib/bazel/bzlmod/BUILD | 2 + .../bazel/bzlmod/BazelDepGraphFunction.java | 41 +++++++++++++------ .../lib/bazel/bzlmod/BazelDepGraphValue.java | 24 +++++------ .../lib/bazel/bzlmod/BazelLockFileModule.java | 12 ++++++ .../lib/bazel/bzlmod/BazelLockFileValue.java | 2 + .../lib/bazel/bzlmod/ModuleExtensionId.java | 2 + .../lib/bazel/bzlmod/ModuleFileFunction.java | 13 ++++-- .../build/lib/bazel/bzlmod/ModuleKey.java | 35 +++++++++++----- .../bzlmod/SingleExtensionEvalFunction.java | 32 +++++++++++++-- .../lib/bazel/bzlmod/StarlarkBazelModule.java | 2 +- .../build/lib/bazel/bzlmod/Version.java | 29 ++++++++----- .../build/lib/cmdline/RepositoryName.java | 5 ++- .../semantics/BuildLanguageOptions.java | 14 +++++++ .../lib/skyframe/BzlmodRepoRuleFunction.java | 7 +++- .../skyframe/RepositoryMappingFunction.java | 6 ++- .../lib/skyframe/RepositoryMappingValue.java | 2 +- .../lib/bazel/bzlmod/BzlmodTestUtil.java | 2 +- .../build/lib/bazel/bzlmod/FakeRegistry.java | 5 ++- .../lib/bazel/bzlmod/InterimModuleTest.java | 2 +- .../build/lib/bazel/bzlmod/ModuleTest.java | 18 +++++--- .../bazel/bzlmod/StarlarkBazelModuleTest.java | 7 ++-- .../bzlmod/modcommand/ExtensionArgTest.java | 6 +-- .../bzlmod/modcommand/ModuleArgTest.java | 6 +-- .../packages/semantics/ConsistencyTest.java | 2 + src/test/tools/bzlmod/MODULE.bazel.lock | 4 +- 28 files changed, 205 insertions(+), 82 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index d99b016087eb23..db00e2300ef9a5 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -22,7 +22,7 @@ bazel_dep(name = "stardoc", version = "0.5.6", repo_name = "io_bazel_skydoc") bazel_dep(name = "zstd-jni", version = "1.5.2-3.bcr.1") bazel_dep(name = "blake3", version = "1.5.1.bcr.1") bazel_dep(name = "sqlite3", version = "3.42.0.bcr.1") -bazel_dep(name = "zlib", version = "1.3") +bazel_dep(name = "zlib", version = "1.3.1.bcr.3") bazel_dep(name = "rules_cc", version = "0.0.9") bazel_dep(name = "rules_java", version = "7.6.5") bazel_dep(name = "rules_graalvm", version = "0.11.1") diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 11b62a7b1879fb..ae99f886ce0370 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -117,8 +117,9 @@ "https://bcr.bazel.build/modules/zlib/1.2.11/MODULE.bazel": "07b389abc85fdbca459b69e2ec656ae5622873af3f845e1c9d80fe179f3effa0", "https://bcr.bazel.build/modules/zlib/1.2.12/MODULE.bazel": "3b1a8834ada2a883674be8cbd36ede1b6ec481477ada359cd2d3ddc562340b27", "https://bcr.bazel.build/modules/zlib/1.2.13/MODULE.bazel": "aa6deb1b83c18ffecd940c4119aff9567cd0a671d7bba756741cb2ef043a29d5", + "https://bcr.bazel.build/modules/zlib/1.3.1.bcr.3/MODULE.bazel": "af322bc08976524477c79d1e45e241b6efbeb918c497e8840b8ab116802dda79", + "https://bcr.bazel.build/modules/zlib/1.3.1.bcr.3/source.json": "2be409ac3c7601245958cd4fcdff4288be79ed23bd690b4b951f500d54ee6e7d", "https://bcr.bazel.build/modules/zlib/1.3/MODULE.bazel": "6a9c02f19a24dcedb05572b2381446e27c272cd383aed11d41d99da9e3167a72", - "https://bcr.bazel.build/modules/zlib/1.3/source.json": "b6b43d0737af846022636e6e255fd4a96fee0d34f08f3830e6e0bac51465c37c", "https://bcr.bazel.build/modules/zstd-jni/1.5.2-3.bcr.1/MODULE.bazel": "cb11f12dc4c8454bede2b64855a8126b547cc89cf77838188513f647d9edd86e", "https://bcr.bazel.build/modules/zstd-jni/1.5.2-3.bcr.1/source.json": "f728c0f2384b4d047a759f4ff5d9cd05a81a78388fbe9cc3981b773e5536be38" }, diff --git a/src/MODULE.tools b/src/MODULE.tools index 82ab2044c823c0..51e51ac5e5f8e1 100644 --- a/src/MODULE.tools +++ b/src/MODULE.tools @@ -13,7 +13,7 @@ bazel_dep(name = "rules_python", version = "0.22.1") bazel_dep(name = "buildozer", version = "7.1.2") bazel_dep(name = "platforms", version = "0.0.9") bazel_dep(name = "protobuf", version = "3.19.6", repo_name = "com_google_protobuf") -bazel_dep(name = "zlib", version = "1.3") +bazel_dep(name = "zlib", version = "1.3.1.bcr.3") cc_configure = use_extension("//tools/cpp:cc_configure.bzl", "cc_configure_extension") use_repo(cc_configure, "local_config_cc", "local_config_cc_toolchains") diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 1c4ed82a6c99e0..02b3a879d57ee6 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -26,6 +26,7 @@ java_library( ], deps = [ "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/net/starlark/java/eval", "//third_party:auto_value", "//third_party:gson", @@ -250,6 +251,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java index 5770b09e1d3d74..7faf5a17e796fb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java @@ -33,7 +33,9 @@ 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.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code; +import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; @@ -42,6 +44,7 @@ import com.google.devtools.build.skyframe.SkyValue; import java.util.Map.Entry; import javax.annotation.Nullable; +import net.starlark.java.eval.StarlarkSemantics; /** * This function runs Bazel module resolution, extracts the dependency graph from it and creates a @@ -58,13 +61,14 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws BazelDepGraphFunctionException, InterruptedException { BazelModuleResolutionValue selectionResult = (BazelModuleResolutionValue) env.getValue(BazelModuleResolutionValue.KEY); + StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); if (env.valuesMissing()) { return null; } var depGraph = selectionResult.getResolvedDepGraph(); ImmutableBiMap canonicalRepoNameLookup = - computeCanonicalRepoNameLookup(depGraph); + computeCanonicalRepoNameLookup(depGraph, starlarkSemantics); ImmutableTable extensionUsagesById; try { extensionUsagesById = getExtensionUsagesById(depGraph, canonicalRepoNameLookup.inverse()); @@ -73,14 +77,17 @@ public SkyValue compute(SkyKey skyKey, Environment env) } ImmutableBiMap extensionUniqueNames = - calculateUniqueNameForUsedExtensionId(extensionUsagesById); + calculateUniqueNameForUsedExtensionId(extensionUsagesById, starlarkSemantics); return BazelDepGraphValue.create( depGraph, canonicalRepoNameLookup, depGraph.values().stream().map(AbridgedModule::from).collect(toImmutableList()), extensionUsagesById, - extensionUniqueNames.inverse()); + extensionUniqueNames.inverse(), + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES) + ? '+' + : '~'); } private static ImmutableTable @@ -126,7 +133,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) } private static ImmutableBiMap computeCanonicalRepoNameLookup( - ImmutableMap depGraph) { + ImmutableMap depGraph, StarlarkSemantics semantics) { // Find modules with multiple versions in the dep graph. Currently, the only source of such // modules is multiple_version_override. ImmutableSet multipleVersionsModules = @@ -151,13 +158,14 @@ private static ImmutableBiMap computeCanonicalRepoNam toImmutableBiMap( key -> multipleVersionsModules.contains(key.getName()) - ? key.getCanonicalRepoNameWithVersion() - : key.getCanonicalRepoNameWithoutVersion(), + ? key.getCanonicalRepoNameWithVersion(semantics) + : key.getCanonicalRepoNameWithoutVersion(semantics), key -> key)); } private ImmutableBiMap calculateUniqueNameForUsedExtensionId( - ImmutableTable extensionUsagesById) { + ImmutableTable extensionUsagesById, + StarlarkSemantics starlarkSemantics) { // Calculate a unique name for each used extension id with the following property that is // required for BzlmodRepoRuleFunction to unambiguously identify the extension that generates a // given repo: @@ -166,18 +174,23 @@ private ImmutableBiMap calculateUniqueNameForUsedExte BiMap extensionUniqueNames = HashBiMap.create(); for (ModuleExtensionId id : extensionUsagesById.rowKeySet()) { int attempt = 1; - while (extensionUniqueNames.putIfAbsent(makeUniqueNameCandidate(id, attempt), id) != null) { + while (extensionUniqueNames.putIfAbsent( + makeUniqueNameCandidate(id, attempt, starlarkSemantics), id) + != null) { attempt++; } } return ImmutableBiMap.copyOf(extensionUniqueNames); } - private static String makeUniqueNameCandidate(ModuleExtensionId id, int attempt) { + private static String makeUniqueNameCandidate( + ModuleExtensionId id, int attempt, StarlarkSemantics starlarkSemantics) { + boolean usePlus = + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES); // Ensure that the resulting extension name (and thus the repository names derived from it) do // not start with a tilde. RepositoryName repository = id.getBzlFileLabel().getRepository(); - String nonEmptyRepoPart = repository.isMain() ? "_main" : repository.getName(); + String nonEmptyRepoPart = repository.isMain() && !usePlus ? "_main" : repository.getName(); // When using a namespace, prefix the extension name with "_" to distinguish the prefix from // those generated by non-namespaced extension usages. Extension names are identified by their // Starlark identifier, which in the case of an exported symbol cannot start with "_". @@ -191,14 +204,18 @@ private static String makeUniqueNameCandidate(ModuleExtensionId id, int attempt) .map( namespace -> String.format( - "%s~_%s%s~%s~%s~%s", + usePlus ? "%s+_%s%s+%s+%s+%s" : "%s~_%s%s~%s~%s~%s", nonEmptyRepoPart, id.getExtensionName(), extensionNameDisambiguator, namespace.getModule().getName(), namespace.getModule().getVersion(), namespace.getUsageExportedName())) - .orElse(nonEmptyRepoPart + "~" + id.getExtensionName() + extensionNameDisambiguator); + .orElse( + nonEmptyRepoPart + + (usePlus ? "+" : "~") + + id.getExtensionName() + + extensionNameDisambiguator); } static class BazelDepGraphFunctionException extends SkyFunctionException { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java index 6fb4c98bf8432c..86217750a73459 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.bazel.bzlmod; -import static com.google.common.collect.ImmutableMap.toImmutableMap; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableBiMap; @@ -43,13 +42,15 @@ public static BazelDepGraphValue create( ImmutableMap canonicalRepoNameLookup, ImmutableList abridgedModules, ImmutableTable extensionUsagesTable, - ImmutableMap extensionUniqueNames) { + ImmutableMap extensionUniqueNames, + char repoNameSeparator) { return new AutoValue_BazelDepGraphValue( depGraph, ImmutableBiMap.copyOf(canonicalRepoNameLookup), abridgedModules, extensionUsagesTable, - extensionUniqueNames); + extensionUniqueNames, + repoNameSeparator); } public static BazelDepGraphValue createEmptyDepGraph() { @@ -65,22 +66,16 @@ public static BazelDepGraphValue createEmptyDepGraph() { .build(); ImmutableMap emptyDepGraph = ImmutableMap.of(ModuleKey.ROOT, root); - ImmutableMap canonicalRepoNameLookup = - emptyDepGraph.keySet().stream() - .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)); + ImmutableMap.of(RepositoryName.MAIN, ModuleKey.ROOT); return BazelDepGraphValue.create( emptyDepGraph, canonicalRepoNameLookup, ImmutableList.of(), ImmutableTable.of(), - ImmutableMap.of()); + ImmutableMap.of(), + '+'); } /** @@ -112,6 +107,9 @@ public static BazelDepGraphValue createEmptyDepGraph() { */ public abstract ImmutableMap getExtensionUniqueNames(); + /** The character to use to separate the different segments of a canonical repo name. */ + public abstract char getRepoNameSeparator(); + /** * Returns the full {@link RepositoryMapping} for the given module, including repos from Bazel * module deps and module extensions. @@ -122,7 +120,7 @@ public final RepositoryMapping getFullRepoMapping(ModuleKey key) { getExtensionUsagesTable().column(key).entrySet()) { ModuleExtensionId extensionId = extIdAndUsage.getKey(); ModuleExtensionUsage usage = extIdAndUsage.getValue(); - String repoNamePrefix = getExtensionUniqueNames().get(extensionId) + "~"; + String repoNamePrefix = getExtensionUniqueNames().get(extensionId) + getRepoNameSeparator(); for (ModuleExtensionUsage.Proxy proxy : usage.getProxies()) { for (Map.Entry entry : proxy.getImports().entrySet()) { String canonicalRepoName = repoNamePrefix + entry.getValue(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java index afb6f3ab3c4563..b615a6d51a6fd3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java @@ -117,6 +117,18 @@ public void afterCommand() { // lockfile that are still up-to-date and adding the newly resolved extension results. BazelLockFileValue newLockfile = BazelLockFileValue.builder() + // HACK: Flipping the flag `--incompatible_use_plus_in_repo_names` causes the canonical + // repo name format to change, which warrants a lockfile invalidation. We could do this + // properly by introducing another field in the lockfile, but that's very annoying since + // we'll need to 1) keep it around for a while; 2) be careful not to parse the rest of + // the lockfile to avoid triggering parsing errors ('~' will be an invalid character in + // repo names eventually). So we just increment the lockfile version if '+' is being + // used. This does mean that, while this hack exists, normal increments of the lockfile + // version need to be done by 2 at a time (i.e. keep LOCK_FILE_VERSION an odd number). + .setLockFileVersion( + depGraphValue.getRepoNameSeparator() == '+' + ? BazelLockFileValue.LOCK_FILE_VERSION + 1 + : BazelLockFileValue.LOCK_FILE_VERSION) .setRegistryFileHashes( ImmutableSortedMap.copyOf(moduleResolutionValue.getRegistryFileHashes())) .setSelectedYankedVersions(moduleResolutionValue.getSelectedYankedVersions()) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java index 60b6faf21cbfaf..ff94e4ccb9f3ab 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java @@ -35,6 +35,8 @@ @GenerateTypeAdapter public abstract class BazelLockFileValue implements SkyValue, Postable { + // NOTE: See "HACK" note in BazelLockFileModule. While this hack exists, normal increments of the + // lockfile version need to be done by 2 at a time (i.e. keep LOCK_FILE_VERSION an odd number). public static final int LOCK_FILE_VERSION = 11; @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionId.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionId.java index 3fb6eeb7ec15ac..e0b135ab6b4815 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionId.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionId.java @@ -60,6 +60,8 @@ public static IsolationKey create(ModuleKey module, String usageExportedName) { @Override public final String toString() { + // NOTE: Can't be bothered to switch this based on the flag. But DO change this to "+" by + // Bazel 8! return getModule() + "~" + getUsageExportedName(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index fd042048ff459a..8ba416e6e89038 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -156,7 +156,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) GetModuleFileResult getModuleFileResult; try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.BZLMOD, () -> "fetch module file: " + moduleKey)) { - getModuleFileResult = getModuleFile(moduleKey, moduleFileKey.getOverride(), env); + getModuleFileResult = + getModuleFile(moduleKey, moduleFileKey.getOverride(), starlarkSemantics, env); } if (getModuleFileResult == null) { return null; @@ -475,7 +476,8 @@ public static RootModuleFileValue evaluateRootModuleFile( // A module with a non-registry override always has a unique version across the // entire dep graph. name -> - ModuleKey.create(name, Version.EMPTY).getCanonicalRepoNameWithoutVersion(), + ModuleKey.create(name, Version.EMPTY) + .getCanonicalRepoNameWithoutVersion(starlarkSemantics), name -> name)); ImmutableSet moduleFilePaths = Stream.concat( @@ -544,14 +546,17 @@ private record GetModuleFileResult( @Nullable private GetModuleFileResult getModuleFile( - ModuleKey key, @Nullable ModuleOverride override, Environment env) + ModuleKey key, + @Nullable ModuleOverride override, + StarlarkSemantics starlarkSemantics, + Environment env) throws ModuleFileFunctionException, InterruptedException { // If there is a non-registry override for this module, we need to fetch the corresponding repo // first and read the module file from there. if (override instanceof NonRegistryOverride) { // A module with a non-registry override always has a unique version across the entire dep // graph. - RepositoryName canonicalRepoName = key.getCanonicalRepoNameWithoutVersion(); + RepositoryName canonicalRepoName = key.getCanonicalRepoNameWithoutVersion(starlarkSemantics); RepositoryDirectoryValue repoDir = (RepositoryDirectoryValue) env.getValue(RepositoryDirectoryValue.key(canonicalRepoName)); if (repoDir == null) { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java index 6fb5e0a378c8b2..c0ad725ffb67aa 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleKey.java @@ -16,12 +16,15 @@ package com.google.devtools.build.lib.bazel.bzlmod; import com.google.auto.value.AutoValue; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import java.util.Comparator; import java.util.List; +import net.starlark.java.eval.StarlarkSemantics; /** A module name, version pair that identifies a module in the external dependency graph. */ @AutoValue @@ -79,8 +82,13 @@ public final String toString() { * *

This method must not be called if the module has a {@link NonRegistryOverride}. */ - public RepositoryName getCanonicalRepoNameWithVersion() { - return getCanonicalRepoName(/* includeVersion= */ true); + public RepositoryName getCanonicalRepoNameWithVersion(StarlarkSemantics semantics) { + return getCanonicalRepoName(/* includeVersion= */ true, semantics); + } + + @VisibleForTesting + public RepositoryName getCanonicalRepoNameWithVersionForTesting() { + return getCanonicalRepoNameWithVersion(StarlarkSemantics.DEFAULT); } /** @@ -88,26 +96,32 @@ public RepositoryName getCanonicalRepoNameWithVersion() { * only guaranteed to be unique when there is a single version of the module in the entire dep * graph. */ - public RepositoryName getCanonicalRepoNameWithoutVersion() { - return getCanonicalRepoName(/* includeVersion= */ false); + public RepositoryName getCanonicalRepoNameWithoutVersion(StarlarkSemantics semantics) { + return getCanonicalRepoName(/* includeVersion= */ false, semantics); + } + + @VisibleForTesting + public RepositoryName getCanonicalRepoNameWithoutVersionForTesting() { + return getCanonicalRepoNameWithoutVersion(StarlarkSemantics.DEFAULT); } - private RepositoryName getCanonicalRepoName(boolean includeVersion) { + private RepositoryName getCanonicalRepoName(boolean includeVersion, StarlarkSemantics semantics) { if (WELL_KNOWN_MODULES.containsKey(getName())) { return WELL_KNOWN_MODULES.get(getName()); } if (ROOT.equals(this)) { return RepositoryName.MAIN; } + boolean usePlus = semantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES); String suffix; 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()); - // Prepend "v" to prevent canonical repo names, which form segments of file paths, from - // looking like a Windows short path. Such paths segments would incur additional file IO - // during analysis (see WindowsShortPath). - suffix = "v" + getVersion().toString(); + // When using `~` as the separator, prepend "v" to prevent canonical repo names, which form + // segments of file paths, from looking like a Windows short path. Such paths segments would + // incur additional file IO during analysis (see WindowsShortPath). + suffix = usePlus ? getVersion().toString() : "v" + getVersion().toString(); } else { // This results in canonical repository names such as `rules_foo~` for the module `rules_foo`. // This particular format is chosen since: @@ -125,7 +139,8 @@ private RepositoryName getCanonicalRepoName(boolean includeVersion) { // rarely used. suffix = ""; } - return RepositoryName.createUnvalidated(String.format("%s~%s", getName(), suffix)); + return RepositoryName.createUnvalidated( + String.format("%s%c%s", getName(), usePlus ? '+' : '~', suffix)); } public static ModuleKey fromString(String s) throws Version.ParseException { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index b1ca65d53d3b51..e452dda9e67ee8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; @@ -178,6 +179,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) extension, usagesValue, extension.getEvalFactors(), + starlarkSemantics, lockedExtension); if (singleExtensionValue != null) { return singleExtensionValue; @@ -261,7 +263,13 @@ public SkyValue compute(SkyKey skyKey, Environment env) lockFileInfo = Optional.empty(); } return createSingleExtensionValue( - generatedRepoSpecs, moduleExtensionMetadata, extensionId, usagesValue, lockFileInfo, env); + generatedRepoSpecs, + moduleExtensionMetadata, + extensionId, + usagesValue, + lockFileInfo, + starlarkSemantics, + env); } /** @@ -278,6 +286,7 @@ private SingleExtensionValue tryGettingValueFromLockFile( RunnableExtension extension, SingleExtensionUsagesValue usagesValue, ModuleExtensionEvalFactors evalFactors, + StarlarkSemantics starlarkSemantics, LockFileModuleExtension lockedExtension) throws SingleExtensionEvalFunctionException, InterruptedException, @@ -338,6 +347,7 @@ private SingleExtensionValue tryGettingValueFromLockFile( extensionId, usagesValue, Optional.of(new LockFileModuleExtension.WithFactors(evalFactors, lockedExtension)), + starlarkSemantics, env); } if (lockfileMode.equals(LockfileMode.ERROR)) { @@ -452,6 +462,7 @@ private SingleExtensionValue createSingleExtensionValue( ModuleExtensionId extensionId, SingleExtensionUsagesValue usagesValue, Optional lockFileInfo, + StarlarkSemantics starlarkSemantics, Environment env) throws SingleExtensionEvalFunctionException { Optional fixup = Optional.empty(); @@ -479,6 +490,11 @@ private SingleExtensionValue createSingleExtensionValue( } } + char separator = + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES) + ? '+' + : '~'; + return SingleExtensionValue.create( generatedRepoSpecs, generatedRepoSpecs.keySet().stream() @@ -486,7 +502,7 @@ private SingleExtensionValue createSingleExtensionValue( toImmutableBiMap( e -> RepositoryName.createUnvalidated( - usagesValue.getExtensionUniqueName() + "~" + e), + usagesValue.getExtensionUniqueName() + separator + e), Function.identity())), lockFileInfo, fixup); @@ -744,7 +760,11 @@ public RunModuleExtensionResult run( Dict kwargs = repo.tag().getAttributeValues().attributes(); // This cast should be safe since it should have been verified at tag creation time. String name = (String) kwargs.get("name"); - String prefixedName = usagesValue.getExtensionUniqueName() + "~" + name; + char separator = + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES) + ? '+' + : '~'; + String prefixedName = usagesValue.getExtensionUniqueName() + separator + name; Rule ruleInstance; AttributeValues attributesValue; try { @@ -884,9 +904,13 @@ public RunModuleExtensionResult run( ModuleExtensionId extensionId, RepositoryMapping mainRepositoryMapping) throws InterruptedException, SingleExtensionEvalFunctionException { + char separator = + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES) + ? '+' + : '~'; ModuleExtensionEvalStarlarkThreadContext threadContext = new ModuleExtensionEvalStarlarkThreadContext( - usagesValue.getExtensionUniqueName() + "~", + usagesValue.getExtensionUniqueName() + separator, extensionId.getBzlFileLabel().getPackageIdentifier(), BazelModuleContext.of(bzlLoadValue.getModule()).repoMapping(), directories, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModule.java index 90b28b87eda5b0..f15f7d4e22f8c2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModule.java @@ -139,7 +139,7 @@ public static StarlarkBazelModule create( } return new StarlarkBazelModule( module.getName(), - module.getVersion().getOriginal(), + module.getVersion().getNormalized(), new Tags(Maps.transformValues(typeCheckedTags, StarlarkList::immutableCopyOf)), module.getKey().equals(ModuleKey.ROOT)); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Version.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Version.java index 41072e01064114..42d44025679e40 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Version.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Version.java @@ -36,9 +36,9 @@ * each a sequence of "identifiers" (defined as a non-empty sequence of ASCII alphanumerical * characters and hyphens) separated by dots. The {@code RELEASE} part may not contain hyphens. * - *

Otherwise, this format is identical to SemVer, especially in terms of the comparison algorithm - * (https://semver.org/#spec-item-11). In other words, this format is intentionally looser than - * SemVer; in particular: + *

Otherwise, this format is identical to SemVer, especially in terms of the comparison algorithm. In other words, this format is + * intentionally looser than SemVer; in particular: * *

    *
  • the "release" part isn't limited to exactly 3 segments (major, minor, patch), but can be @@ -51,6 +51,13 @@ * {@code a} and {@code b} compare {@code a < b} iff the same holds when they're compared as Bazel * module versions. * + *

    Versions with a "build" part are generally accepted as input, but they're treated as if the + * "build" part is completely absent. That is, when Bazel outputs version strings, it never outputs + * the "build" part (in fact, it doesn't even store it); similarly, when Bazel accesses registries + * to request versions, the "build" part is never included. This gives us the nice property of + * "consistent with equals" natural ordering (see {@link Comparable}); that is, {@code + * a.compareTo(b) == 0} iff {@code a.equals(b)}. + * *

    The special "empty string" version can also be used, and compares higher than everything else. * It signifies that there is a {@link NonRegistryOverride} for a module. */ @@ -112,15 +119,15 @@ public final int compareTo(Identifier o) { /** Returns the "prerelease" part of the version string as a list of {@link Identifier}s. */ abstract ImmutableList getPrerelease(); - /** Returns the original version string. */ - public abstract String getOriginal(); + /** Returns the normalized version string (that is, with any "build" part stripped). */ + public abstract String getNormalized(); /** * Whether this is just the "empty string" version, which signifies a non-registry override for * the module. */ boolean isEmpty() { - return getOriginal().isEmpty(); + return getNormalized().isEmpty(); } /** @@ -164,7 +171,8 @@ public static Version parse(String version) throws ParseException { } } - return new AutoValue_Version(releaseSplit.build(), prereleaseSplit.build(), version); + String normalized = Strings.isNullOrEmpty(prerelease) ? release : release + '-' + prerelease; + return new AutoValue_Version(releaseSplit.build(), prereleaseSplit.build(), normalized); } private static final Comparator COMPARATOR = @@ -180,17 +188,18 @@ public int compareTo(Version o) { @Override public final String toString() { - return getOriginal(); + return getNormalized(); } @Override public final boolean equals(Object o) { - return this == o || (o instanceof Version && ((Version) o).getOriginal().equals(getOriginal())); + return this == o + || (o instanceof Version && ((Version) o).getNormalized().equals(getNormalized())); } @Override public final int hashCode() { - return Objects.hash("version", getOriginal().hashCode()); + return Objects.hash("version", getNormalized().hashCode()); } /** An exception encountered while trying to {@link Version#parse parse} a version. */ diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java index 5a386b3d0ef8be..ab1bf0dc46f2a1 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java @@ -42,7 +42,7 @@ public final class RepositoryName { // Repository names must not start with a tilde as shells treat unescaped paths starting with them // specially. // https://www.gnu.org/software/bash/manual/html_node/Tilde-Expansion.html - private static final Pattern VALID_REPO_NAME = Pattern.compile("|[\\w\\-.][\\w\\-.~]*"); + private static final Pattern VALID_REPO_NAME = Pattern.compile("|[\\w\\-.+][\\w\\-.~+]*"); // Must start with a letter. Can contain ASCII letters and digits, underscore, dash, and dot. private static final Pattern VALID_USER_PROVIDED_NAME = Pattern.compile("[a-zA-Z][-.\\w]*$"); @@ -218,11 +218,12 @@ public String getOwnerModuleDisplayString() { if (ownerRepoIfNotVisible.isMain()) { return "root module"; } else { + boolean hasTilde = ownerRepoIfNotVisible.getName().contains("~"); return String.format( "module '%s'", ownerRepoIfNotVisible .getName() - .substring(0, ownerRepoIfNotVisible.getName().indexOf('~'))); + .substring(0, ownerRepoIfNotVisible.getName().indexOf(hasTilde ? '~' : '+'))); } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 9575a929c7ad92..3df4f24621cd5c 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -215,6 +215,17 @@ public final class BuildLanguageOptions extends OptionsBase { + " https://bazel.build/external/overview for more information.") public boolean enableWorkspace; + @Option( + name = "incompatible_use_plus_in_repo_names", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = OptionEffectTag.LOADING_AND_ANALYSIS, + help = + "If true, uses the plus sign (+) as the separator in canonical repo names, instead of the" + + " tilde (~). This is to address severe performance issues on Windows; see" + + " https://github.com/bazelbuild/bazel/issues/22865 for more information.") + public boolean incompatibleUsePlusInRepoNames; + @Option( name = "experimental_isolated_extension_usages", defaultValue = "false", @@ -760,6 +771,7 @@ public StarlarkSemantics toStarlarkSemantics() { .setBool(EXPERIMENTAL_ENABLE_SCL_DIALECT, experimentalEnableSclDialect) .setBool(ENABLE_BZLMOD, enableBzlmod) .setBool(ENABLE_WORKSPACE, enableWorkspace) + .setBool(INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES, incompatibleUsePlusInRepoNames) .setBool(EXPERIMENTAL_ISOLATED_EXTENSION_USAGES, experimentalIsolatedExtensionUsages) .setBool( INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW, incompatibleExistingRulesImmutableView) @@ -869,6 +881,8 @@ public StarlarkSemantics toStarlarkSemantics() { public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "-experimental_enable_scl_dialect"; public static final String ENABLE_BZLMOD = "+enable_bzlmod"; public static final String ENABLE_WORKSPACE = "+enable_workspace"; + public static final String INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES = + "-incompatible_use_plus_in_repo_names"; public static final String EXPERIMENTAL_ISOLATED_EXTENSION_USAGES = "-experimental_isolated_extension_usages"; public static final String INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java index 55e4135f8a4882..3e30cb1b37c1af 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; import com.google.devtools.build.lib.packages.RuleFunction; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.skyframe.SkyFunction; @@ -113,9 +114,13 @@ public SkyValue compute(SkyKey skyKey, Environment env) } // Step 3: look for the repo from module extension evaluation results. + char separator = + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES) + ? '+' + : '~'; Optional extensionId = bazelDepGraphValue.getExtensionUniqueNames().entrySet().stream() - .filter(e -> repositoryName.getName().startsWith(e.getValue() + "~")) + .filter(e -> repositoryName.getName().startsWith(e.getValue() + separator)) .map(Entry::getKey) .findFirst(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java index d01fd4d78ea16d..2cfd3319b7ee34 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java @@ -233,7 +233,11 @@ private RepositoryMappingValue computeFromWorkspace( private static Optional maybeGetModuleExtensionForRepo( RepositoryName repositoryName, BazelDepGraphValue bazelDepGraphValue) { return bazelDepGraphValue.getExtensionUniqueNames().entrySet().stream() - .filter(e -> repositoryName.getName().startsWith(e.getValue() + "~")) + .filter( + e -> + repositoryName + .getName() + .startsWith(e.getValue() + bazelDepGraphValue.getRepoNameSeparator())) .map(Entry::getKey) .findFirst(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java index 54bfef21ca5545..d457372c934833 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingValue.java @@ -72,7 +72,7 @@ public static RepositoryMappingValue createForBzlmodRepo( return new AutoValue_RepositoryMappingValue( repositoryMapping, Optional.of(associatedModuleName), - Optional.of(associatedModuleVersion.getOriginal())); + Optional.of(associatedModuleVersion.getNormalized())); } /** diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodTestUtil.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodTestUtil.java index d27f684d94a216..6783f21937dc5b 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodTestUtil.java @@ -307,7 +307,7 @@ public static RepositoryMapping createRepositoryMapping(ModuleKey key, String... mappingBuilder.put(names[i], RepositoryName.createUnvalidated(names[i + 1])); } return RepositoryMapping.create( - mappingBuilder.buildOrThrow(), key.getCanonicalRepoNameWithoutVersion()); + mappingBuilder.buildOrThrow(), key.getCanonicalRepoNameWithoutVersionForTesting()); } public static TagClass createTagClass(Attribute... attrs) { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java index d1d20aedba16c9..1f8ba8e32d4a83 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java @@ -81,7 +81,10 @@ public RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler) { .setAttributes( AttributeValues.create( ImmutableMap.of( - "path", rootPath + "/" + key.getCanonicalRepoNameWithVersion().getName()))) + "path", + rootPath + + "/" + + key.getCanonicalRepoNameWithVersionForTesting().getName()))) .build(); eventHandler.post( RegistryFileDownloadEvent.create( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModuleTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModuleTest.java index dd65628c9bac18..d903da386e3334 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/InterimModuleTest.java @@ -39,7 +39,7 @@ public void withDepSpecsTransformed() throws Exception { DepSpec.fromModuleKey( createModuleKey( depSpec.getName() + "_new", - depSpec.getVersion().getOriginal() + ".1")))) + depSpec.getVersion().getNormalized() + ".1")))) .isEqualTo( InterimModuleBuilder.create("", "") .addDep("dep_foo", createModuleKey("foo_new", "1.0.1")) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleTest.java index d7f3041ccf499b..dd4db83801a0e7 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleTest.java @@ -46,7 +46,8 @@ public void getRepoMapping() throws Exception { module.getRepoMappingWithBazelDepsOnly( Stream.of(key, fooKey, barKey, ModuleKey.ROOT) .collect( - toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithoutVersion)))) + toImmutableMap( + k -> k, ModuleKey::getCanonicalRepoNameWithoutVersionForTesting)))) .isEqualTo( createRepositoryMapping( key, @@ -73,7 +74,9 @@ public void getRepoMapping_asMainModule() throws Exception { assertThat( module.getRepoMappingWithBazelDepsOnly( Stream.of(ModuleKey.ROOT, fooKey, barKey) - .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersion)))) + .collect( + toImmutableMap( + k -> k, ModuleKey::getCanonicalRepoNameWithVersionForTesting)))) .isEqualTo( createRepositoryMapping( ModuleKey.ROOT, @@ -89,11 +92,14 @@ public void getRepoMapping_asMainModule() throws Exception { @Test public void getCanonicalRepoName_isNotAWindowsShortPath() { - assertNotAShortPath(createModuleKey("foo", "").getCanonicalRepoNameWithoutVersion().getName()); - assertNotAShortPath(createModuleKey("foo", "1").getCanonicalRepoNameWithVersion().getName()); - assertNotAShortPath(createModuleKey("foo", "1.2").getCanonicalRepoNameWithVersion().getName()); assertNotAShortPath( - createModuleKey("foo", "1.2.3").getCanonicalRepoNameWithVersion().getName()); + createModuleKey("foo", "").getCanonicalRepoNameWithoutVersionForTesting().getName()); + assertNotAShortPath( + createModuleKey("foo", "1").getCanonicalRepoNameWithVersionForTesting().getName()); + assertNotAShortPath( + createModuleKey("foo", "1.2").getCanonicalRepoNameWithVersionForTesting().getName()); + assertNotAShortPath( + createModuleKey("foo", "1.2.3").getCanonicalRepoNameWithVersionForTesting().getName()); } private static void assertNotAShortPath(String name) { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java index 6fdb4f7afe1ee8..032488da7acad1 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java @@ -95,8 +95,8 @@ public void basic() throws Exception { extension, module.getRepoMappingWithBazelDepsOnly( ImmutableMap.of( - fooKey, fooKey.getCanonicalRepoNameWithoutVersion(), - barKey, barKey.getCanonicalRepoNameWithoutVersion())), + fooKey, fooKey.getCanonicalRepoNameWithoutVersionForTesting(), + barKey, barKey.getCanonicalRepoNameWithoutVersionForTesting())), usage); assertThat(moduleProxy.getName()).isEqualTo("foo"); @@ -143,7 +143,8 @@ public void unknownTagClass() throws Exception { abridgedModule, extension, module.getRepoMappingWithBazelDepsOnly( - ImmutableMap.of(fooKey, fooKey.getCanonicalRepoNameWithoutVersion())), + ImmutableMap.of( + fooKey, fooKey.getCanonicalRepoNameWithoutVersionForTesting())), usage)); assertThat(e).hasMessageThat().contains("does not have a tag class named blep"); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ExtensionArgTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ExtensionArgTest.java index 1b9366a6acb158..2bd1e58cb71a3d 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ExtensionArgTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ExtensionArgTest.java @@ -89,7 +89,7 @@ public void resolve_good() throws Exception { .buildOrThrow(); ImmutableMap moduleKeyToCanonicalNames = depGraph.keySet().stream() - .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersion)); + .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersionForTesting)); ImmutableBiMap baseModuleDeps = ImmutableBiMap.of("fred", key); ImmutableBiMap baseModuleUnusedDeps = ImmutableBiMap.of(); @@ -118,7 +118,7 @@ public void resolve_badLabel() throws Exception { .buildOrThrow(); ImmutableMap moduleKeyToCanonicalNames = depGraph.keySet().stream() - .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersion)); + .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersionForTesting)); ImmutableBiMap baseModuleDeps = ImmutableBiMap.of("fred", key); ImmutableBiMap baseModuleUnusedDeps = ImmutableBiMap.of(); @@ -162,7 +162,7 @@ public void resolve_noneOrtooManyModules() throws Exception { .buildOrThrow(); ImmutableMap moduleKeyToCanonicalNames = depGraph.keySet().stream() - .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersion)); + .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersionForTesting)); ImmutableBiMap baseModuleDeps = ImmutableBiMap.of("foo1", foo1, "foo2", foo2); ImmutableBiMap baseModuleUnusedDeps = ImmutableBiMap.of(); diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModuleArgTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModuleArgTest.java index fc15ea604ffb34..70d6b212f84d54 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModuleArgTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModuleArgTest.java @@ -80,7 +80,7 @@ public void converter() throws Exception { ImmutableMap moduleKeyToCanonicalNames = depGraph.keySet().stream() - .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersion)); + .collect(toImmutableMap(k -> k, ModuleKey::getCanonicalRepoNameWithVersionForTesting)); ImmutableBiMap baseModuleDeps = ImmutableBiMap.of("fred", foo2); ImmutableBiMap baseModuleUnusedDeps = ImmutableBiMap.of("fred", foo1); RepositoryMapping rootMapping = createRepositoryMapping(ModuleKey.ROOT, "fred", "foo~v2.0"); @@ -270,7 +270,7 @@ public void resolve_apparentRepoName_notFound() throws Exception { @Test public void resolve_canonicalRepoName_good() throws Exception { - var arg = CanonicalRepoName.create(foo2.getCanonicalRepoNameWithVersion()); + var arg = CanonicalRepoName.create(foo2.getCanonicalRepoNameWithVersionForTesting()); assertThat( arg.resolveToModuleKeys( @@ -311,7 +311,7 @@ public void resolve_canonicalRepoName_notFound() throws Exception { @Test public void resolve_canonicalRepoName_unused() throws Exception { - var arg = CanonicalRepoName.create(foo1.getCanonicalRepoNameWithVersion()); + var arg = CanonicalRepoName.create(foo1.getCanonicalRepoNameWithVersionForTesting()); // Without --include_unused, this doesn't resolve, as foo@1.0 has been replaced by foo@2.0. assertThat( diff --git a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java index d5b4943a3cd31b..da4b746121e0f5 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java @@ -129,6 +129,7 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep "--experimental_enable_android_migration_apis=" + rand.nextBoolean(), "--enable_bzlmod=" + rand.nextBoolean(), "--enable_workspace=" + rand.nextBoolean(), + "--incompatible_use_plus_in_repo_names=" + rand.nextBoolean(), "--experimental_isolated_extension_usages=" + rand.nextBoolean(), "--experimental_google_legacy_api=" + rand.nextBoolean(), "--experimental_platforms_api=" + rand.nextBoolean(), @@ -177,6 +178,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { BuildLanguageOptions.EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, rand.nextBoolean()) .setBool(BuildLanguageOptions.ENABLE_BZLMOD, rand.nextBoolean()) .setBool(BuildLanguageOptions.ENABLE_WORKSPACE, rand.nextBoolean()) + .setBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES, rand.nextBoolean()) .setBool(BuildLanguageOptions.EXPERIMENTAL_ISOLATED_EXTENSION_USAGES, rand.nextBoolean()) .setBool(BuildLanguageOptions.EXPERIMENTAL_GOOGLE_LEGACY_API, rand.nextBoolean()) .setBool(BuildLanguageOptions.EXPERIMENTAL_PLATFORMS_API, rand.nextBoolean()) diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index be76d812f5a353..10efc54d38ee5b 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -56,8 +56,8 @@ "https://bcr.bazel.build/modules/upb/0.0.0-20220923-a547704/source.json": "f1ef7d3f9e0e26d4b23d1c39b5f5de71f584dd7d1b4ef83d9bbba6ec7a6a6459", "https://bcr.bazel.build/modules/zlib/1.2.11/MODULE.bazel": "07b389abc85fdbca459b69e2ec656ae5622873af3f845e1c9d80fe179f3effa0", "https://bcr.bazel.build/modules/zlib/1.2.12/MODULE.bazel": "3b1a8834ada2a883674be8cbd36ede1b6ec481477ada359cd2d3ddc562340b27", - "https://bcr.bazel.build/modules/zlib/1.3/MODULE.bazel": "6a9c02f19a24dcedb05572b2381446e27c272cd383aed11d41d99da9e3167a72", - "https://bcr.bazel.build/modules/zlib/1.3/source.json": "b6b43d0737af846022636e6e255fd4a96fee0d34f08f3830e6e0bac51465c37c" + "https://bcr.bazel.build/modules/zlib/1.3.1.bcr.3/MODULE.bazel": "af322bc08976524477c79d1e45e241b6efbeb918c497e8840b8ab116802dda79", + "https://bcr.bazel.build/modules/zlib/1.3.1.bcr.3/source.json": "2be409ac3c7601245958cd4fcdff4288be79ed23bd690b4b951f500d54ee6e7d" }, "selectedYankedVersions": {}, "moduleExtensions": {