Add reverted 'isolate' changes (#18928)

bazelbuild · Jul 13, 2023 · 76be3a2 · 76be3a2
1 parent 287e179
commit 76be3a2
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 5 deletions.
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
@@ -222,7 +222,9 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
         try {
           moduleExtensionId =
               ModuleExtensionId.create(
-                  labelConverter.convert(usage.getExtensionBzlFile()), usage.getExtensionName(), usage.getIsolationKey());
+                  labelConverter.convert(usage.getExtensionBzlFile()),
+                  usage.getExtensionName(),
+                  usage.getIsolationKey());
         } catch (LabelSyntaxException e) {
           throw ExternalDepsException.withCauseAndMessage(
               Code.BAD_MODULE,
@@ -252,12 +254,31 @@ private ImmutableBiMap<String, ModuleExtensionId> calculateUniqueNameForUsedExte
       // not start with a tilde.
       RepositoryName repository = id.getBzlFileLabel().getRepository();
       String nonEmptyRepoPart = repository.isMain() ? "_main" : repository.getName();
-      String bestName = nonEmptyRepoPart + "~" + id.getExtensionName();
+      // 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 "_".
+      // We also include whether the isolated usage is a dev usage as well as its index in the
+      // MODULE.bazel file to ensure that canonical repository names don't change depending on
+      // whether dev dependencies are ignored. This removes potential for confusion and also
+      // prevents unnecessary refetches when --ignore_dev_dependency is toggled.
+      String bestName =
+          id.getIsolationKey()
+              .map(
+                  namespace ->
+                      String.format(
+                          "%s~_%s~%s~%s~%s%d",
+                          nonEmptyRepoPart,
+                          id.getExtensionName(),
+                          namespace.getModule().getName(),
+                          namespace.getModule().getVersion(),
+                          namespace.isDevUsage() ? "dev" : "",
+                          namespace.getIsolatedUsageIndex()))
+              .orElse(nonEmptyRepoPart + "~" + id.getExtensionName());
       if (extensionUniqueNames.putIfAbsent(bestName, id) == null) {
         continue;
       }
       int suffix = 2;
-      while (extensionUniqueNames.putIfAbsent(bestName + suffix, id) != null) {
+      while (extensionUniqueNames.putIfAbsent(bestName + "~" + suffix, id) != null) {
         suffix++;
       }
     }

diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java
@@ -292,7 +292,7 @@ public void createValue_moduleExtensions() throws Exception {
             maven, "rules_jvm_external~1.0~maven",
             pip, "rules_python~2.0~pip",
             myext, "dep~2.0~myext",
-            myext2, "dep~2.0~myext2");
+            myext2, "dep~2.0~myext~2");
 
     assertThat(value.getFullRepoMapping(ModuleKey.ROOT))
         .isEqualTo(
@@ -323,7 +323,7 @@ public void createValue_moduleExtensions() throws Exception {
                 "oneext",
                 "dep~2.0~myext~myext",
                 "twoext",
-                "dep~2.0~myext2~myext"));
+                "dep~2.0~myext~2~myext"));
   }
 
   @Test