Use apparent_repo_name only on repository_ctx

Repurposed `apparent_repo_name` to only work for `repository_ctx`
objects, not repository or module names in general. Removed
`generated_rule_name` from `repositories.bzl`, since it's no longer
necessary.

Technically we could eliminate `apparent_repo_name` by making
`generated_rule_name` a mandatory attribute of `_jvm_import_external`.
However, this feels ultimately clunky and unnecessary.

This update to `apparent_repo_name` required removing
`_update_external_target_path` and updating
`_target_path_by_default_prefixes` to remove
`external/<canonical_repo_name>` prefixes. This represents a breaking
change for files referencing `external/<repo_name>` paths, but the quick
fix is to delete that prefix in the code. This matches the behavior in
the same function regarding `resources/` and `java/` prefixes.

Author: mbland

scala/private/macros/bzlmod.bzl

@@ -1,37 +1,21 @@
 """Utilities for working with Bazel modules"""
 
-_repo_attr = (
-    "repo_name" if hasattr(Label("//:all"), "repo_name") else "workspace_name"
-)
-
-def apparent_repo_name(label_or_name):
-    """Return a repository's apparent repository name.
-
-    Can be replaced with a future bazel-skylib implementation, if accepted into
-    that repo.
+def apparent_repo_name(repository_ctx):
+    """Generates a repository's apparent name from a repository_ctx object.
 
     Args:
-        label_or_name: a Label or repository name string
+        repository_ctx: a repository_ctx object
 
     Returns:
-        The apparent repository name
+        An apparent repo name derived from repository_ctx.name
     """
-    repo_name = getattr(label_or_name, _repo_attr, label_or_name).lstrip("@")
-    delimiter_indices = []
+    repo_name = repository_ctx.name
 
     # Bazed on this pattern from the Bazel source:
     # com.google.devtools.build.lib.cmdline.RepositoryName.VALID_REPO_NAME
-    for i in range(len(repo_name)):
+    for i in range(len(repo_name) - 1, -1, -1):
         c = repo_name[i]
         if not (c.isalnum() or c in "_-."):
-            delimiter_indices.append(i)
-
-    if len(delimiter_indices) == 0:
-        # Already an apparent repo name, apparently.
-        return repo_name
-
-    if len(delimiter_indices) == 1:
-        # The name is for a top level module, possibly containing a version ID.
-        return repo_name[:delimiter_indices[0]]
+            return repo_name[i + 1:]
 
-    return repo_name[delimiter_indices[-1] + 1:]
+    return repo_name

scala/private/resources.bzl

@@ -1,5 +1,3 @@
-load(":macros/bzlmod.bzl", "apparent_repo_name")
-
 def paths(resources, resource_strip_prefix):
     """Return a list of path tuples (target, source) where:
         target - is a path in the archive (with given prefix stripped off)
@@ -15,13 +13,7 @@ def paths(resources, resource_strip_prefix):
 
 def _target_path(resource, resource_strip_prefix):
     path = _target_path_by_strip_prefix(resource, resource_strip_prefix) if resource_strip_prefix else _target_path_by_default_prefixes(resource)
-    return _update_external_target_path(_strip_prefix(path, "/"))
-
-def _update_external_target_path(target_path):
-    if not target_path.startswith("external/"):
-        return target_path
-    prefix, repo_name, rest = target_path.split("/")
-    return "/".join([prefix, apparent_repo_name(repo_name), rest])
+    return _strip_prefix(path, "/")
 
 def _target_path_by_strip_prefix(resource, resource_strip_prefix):
     # Start from absolute resource path and then strip roots so we get to correct short path
@@ -52,6 +44,13 @@ def _target_path_by_default_prefixes(resource):
     if rel_path:
         return rel_path
 
+    # Looking inside an external repository. Trim off both the "external/" and
+    # the repository name components. Especially important under Bzlmod, because
+    # the canonical repository name may change between versions.
+    (dir_1, dir_2, rel_path) = path.partition("external/")
+    if rel_path:
+        return rel_path[rel_path.index("/"):]
+
     # Both short_path and path have quirks we wish to avoid, in short_path there are times where
     # it is prefixed by `../` instead of `external/`. And in .path it will instead return the entire
     # bazel-out/... path, which is also wanting to be avoided. So instead, we return the short-path if

scala/scala_maven_import_external.bzl

@@ -65,22 +65,20 @@ def _jvm_import_external(repository_ctx):
     if (repository_ctx.attr.generated_linkable_rule_name and
         not repository_ctx.attr.neverlink):
         fail("Only use generated_linkable_rule_name if neverlink is set")
-    name = (
-        repository_ctx.attr.generated_rule_name or
-        apparent_repo_name(repository_ctx.name)
-    )
+    repo_name = apparent_repo_name(repository_ctx)
+    name = repository_ctx.attr.generated_rule_name or repo_name
     urls = repository_ctx.attr.jar_urls
     if repository_ctx.attr.jar_sha256:
         print("'jar_sha256' is deprecated. Please use 'artifact_sha256'")
     sha = repository_ctx.attr.jar_sha256 or repository_ctx.attr.artifact_sha256
-    path = repository_ctx.name + ".jar"
+    path = repo_name + ".jar"
     for url in urls:
         if url.endswith(".jar"):
             path = url[url.rindex("/") + 1:]
             break
     srcurls = repository_ctx.attr.srcjar_urls
     srcsha = repository_ctx.attr.srcjar_sha256
-    srcpath = repository_ctx.name + "-src.jar" if srcurls else ""
+    srcpath = repo_name + "-src.jar" if srcurls else ""
     coordinates = repository_ctx.attr.coordinates
     for url in srcurls:
         if url.endswith(".jar"):
@@ -140,7 +138,7 @@ def _jvm_import_external(repository_ctx):
         "",
         "alias(",
         "    name = \"jar\",",
-        "    actual = \"@%s\"," % apparent_repo_name(repository_ctx.name),
+        "    actual = \"@%s\"," % repo_name,
         ")",
         "",
     ]))

test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalDepTest.scala

@@ -8,7 +8,7 @@ class ScalaLibResourcesFromExternalDepTest extends SpecWithJUnit {
     "allow to load resources" >> {
 
       val expectedString = String.format("A resource%n"); //Using platform dependent newline (%n)
-      get("/external/test_new_local_repo/resource.txt") must beEqualTo(expectedString)
+      get("/resource.txt") must beEqualTo(expectedString)
 
     }
   }

test/src/main/scala/scalarules/test/resources/ScalaLibResourcesFromExternalScalaTest.scala

@@ -6,7 +6,7 @@ class ScalaLibResourcesFromExternalScalaTest extends AnyFunSuite {
 
   test("Scala library depending on resources from external resource-only jar should allow to load resources") {
     val expectedString = String.format("A resource%n"); //Using platform dependent newline (%n)
-    assert(get("/external/test_new_local_repo/resource.txt") === expectedString)
+    assert(get("/resource.txt") === expectedString)
   }
 
   private def get(s: String): String =

third_party/repositories/repositories.bzl

@@ -102,10 +102,9 @@ def repositories(
                 major_scala_version.replace(".", "_"),
             ))
 
-        generated_rule_name = apparent_repo_name(id) + suffix
+        artifact_repo_name = id + suffix
         _scala_maven_import_external(
-            name = id + suffix,
-            generated_rule_name = generated_rule_name,
+            name = artifact_repo_name,
             artifact = artifacts[id]["artifact"],
             artifact_sha256 = artifacts[id]["sha256"],
             licenses = ["notice"],
@@ -123,13 +122,13 @@ def repositories(
         # See: https://github.com/bazelbuild/rules_scala/pull/1573
         # Hopefully we can deprecate and remove it one day.
         if suffix and scala_version == SCALA_VERSION:
-            _alias_repository(name = id, target = generated_rule_name)
+            _alias_repository(name = id, target = artifact_repo_name)
 
 def _alias_repository_impl(rctx):
     """ Builds a repository containing just two aliases to the Scala Maven artifacts in the `target` repository. """
 
     format_kwargs = {
-        "name": apparent_repo_name(rctx.name),
+        "name": apparent_repo_name(rctx),
         "target": rctx.attr.target,
     }
     rctx.file("BUILD", """alias(