Refactor scripts/create_repository.py, improve efficiency and correctness, and bump Scalatest, protobuf-java, kind-projector (bazel-contrib#1642)

* Bump to Scalatest 3.2.19, protobuf-java 4.28.3

No code changes other than the version bumps.

* Move several functions into MavenCoordinates class

Improves readability by associating several functions directly with the
`MavenCoordinates` class. Doesn't produce any changes to the
`third_party/repositories/scala_*.bzl` files.

* Extract ArtifactLabelMaker in create_repository.py

Improves readability and maintainability. No change in
`third_party/repositories/scala_*.bzl`.

* Elide `get_mavens_coordinates_from_json`

Uses a list comprehension in `get_json_dependencies` as well, instead of
the `list(map(...))` composition. No changes in
`third_party/repositories/scala_*.bzl`.

* Add third_party/repositories/scala*.bzl docstring

This will hopefully help future contributors discover how to properly
update these files via `scripts/create_repository.py`.

* Extract ArtifactResolver

Continuation of refactoring to objects to improve maintainability.

* Remove 'testonly' when resolving root artifacts

I noticed that `org.typelevel:kind-projector_2.{11.12,12.12,13.15}` was
always updated after putting `print()` statements in
`_map_to_resolved_artifacts`. Arguably if it's a root artifact (which
`kind-projector` is), or dependency of one, it shouldn't be 'testonly'.

* Remove kind-projector_2.11.12 special case

It seems version 0.13.3 is available for Scala 2.11.12, so no need to
restrain its version number.

* Extract ArtifactUpdater object

Also hoisted select_root_artifacts to the top of the file, closer to the
actual root artifact version constant declarations.

* Refactor create_repository.py objects

Also added docstrings to all public methods.

* Make scala-parser-combinators a root artifact

It turns out it's in all the existing
`third_party/repositories/scala_*.bzl` files anyway.

Also removed `EXCLUDED_ARTIFACTS` from create_repository.py.

* Add cache to ArtifactLabelMaker

Slight efficiency improvement to avoid having to recompute the same
labels over and over.

* Read the `cs fetch` JSON file only once

Avoids having `_get_json_dependencies` read the file multiple times, and
allows us to clean it up as soon as it's read.

* Update existing repo names, root artifact versions

Starting to update repo names pulled on a thread that led to adding
several new root artifacts and updating others. However, these are all
good changes that pass all tests.

* Use `cs fetch --json-output-file` directly

I noticed while running the command directly that we could use the JSON
output file to run through all resolved artifacts and their dependencies
directly. There was no need for separate a separate `cs resolve` step
and `cs fetch` steps for each resolved artifact. Nor was there a need
for a separate `_get_json_dependencies` function.

Most significantly, there was no need to download each artifact for
checksumming. `cs fetch` already downloaded them and listed their paths
in the JSON file. The original code was parsing existing file paths to
generate URLs to download them again.

This change vastly improves performance. Here are the times for creating
a fresh output dir and then updating it before this change:

```txt
$ /usr/bin/time ./scripts/create_repository.py --output_dir before

[ ...snip... ]
       66.01 real        13.25 user         9.99 sys

$ /usr/bin/time ./scripts/create_repository.py --output_dir before

[ ...snip... ]
        1.76 real         1.41 user         0.49 sys
```

Here are the times after:

```txt
$ /usr/bin/time ./scripts/create_repository.py --output_dir after

[ ...snip... ]
        1.16 real         0.84 user         0.39 sys

$ /usr/bin/time ./scripts/create_repository.py --output_dir after

[ ...snip... ]
        0.96 real         0.72 user         0.27 sys
```

Comparing the two output directories via `diff -uNr before after` shows
the later having reordered some dependencies, otherwise the output is
identical.

This change also introduces a metadata cache. It might not help much
overall, but it feels right not to recompute data if possible.

Also decided to emit the Maven coordinates for each artifact actually
updated by the script.

* Sort the third_party/repositories/scala_* dicts

It's only the right thing to do for our fellow humans.

* Refactor select_root_artifacts

Also:

- Renames `SCALAPB_RUNTIME_VERSION` to `SCALAPB_VERSION`, since it
  affects other artifacts in the ScalaPB suite.

- Updates `ArtifactLabelMaker.get_label` so that it doesn't call
  `self._get_label_impl` unless a label isn't already present. The
  previous `self._cache.setdefault` call took the result of
  `self._get_label_impl` as an argument, which defeated the purpose.

* Restore `EXCLUDED_ARTIFACTS` mechanism

The new mechanism is more thorough and robust than the version from
"Make scala-parser-combinators a root artifact". It will not only
exclude artifacts from the newly generated dict, but will remove `deps`
labels for newly excluded artifacts from the existing dict.

There are no artifacts in the `EXCLUDED_ARTIFACTS` set yet, but we'll
add `com.google.guava:listenablefuture` to it once we get to updating
gRPC and Guava. I wanted this mechanism to stand on its own for clarity.

* Use `SPECIAL_CASE_ARTIFACT_LABELS`

Replaces `SPECIAL_CASE_GROUP_LABELS`. Even though keying on the group
label worked for now, keying on the artifact seems more sound.

* Add _remove_scala_version_suffix

Adds this new private helper function to `ArtifactLabelMaker` to make
stripping Scala version suffixes from artifact names more robust.

Author: mbland

jmh/jmh.bzl

@@ -14,7 +14,7 @@ def jmh_repositories(
             "io_bazel_rules_scala_org_openjdk_jmh_jmh_generator_asm",
             "io_bazel_rules_scala_org_openjdk_jmh_jmh_generator_reflection",
             "io_bazel_rules_scala_org_openjdk_jmh_jmh_generator_reflection",
-            "io_bazel_rules_scala_org_ows2_asm_asm",
+            "io_bazel_rules_scala_org_ow2_asm_asm",
             "io_bazel_rules_scala_net_sf_jopt_simple_jopt_simple",
             "io_bazel_rules_scala_org_apache_commons_commons_math3",
         ],

scala/private/macros/scala_repositories.bzl

@@ -176,27 +176,45 @@ def rules_scala_setup(scala_compiler_srcjar = None):
     )
 
 def _artifact_ids(scala_version):
-    return [
-        "io_bazel_rules_scala_scala_library",
+    result = [
         "io_bazel_rules_scala_scala_compiler",
-        "io_bazel_rules_scala_scala_reflect",
-        "io_bazel_rules_scala_scala_xml",
-        "io_bazel_rules_scala_scala_parser_combinators",
-        "org_scalameta_semanticdb_scalac",
-    ] if scala_version.startswith("2") else [
-        "io_bazel_rules_scala_scala_asm",
-        "io_bazel_rules_scala_scala_compiler",
-        "io_bazel_rules_scala_scala_compiler_2",
-        "io_bazel_rules_scala_scala_interfaces",
         "io_bazel_rules_scala_scala_library",
-        "io_bazel_rules_scala_scala_library_2",
         "io_bazel_rules_scala_scala_parser_combinators",
-        "io_bazel_rules_scala_scala_reflect_2",
-        "io_bazel_rules_scala_scala_tasty_core",
         "io_bazel_rules_scala_scala_xml",
-        "org_scala_sbt_compiler_interface",
     ]
 
+    if scala_version.startswith("2."):
+        result.extend([
+            "io_bazel_rules_scala_scala_reflect",
+            "org_scalameta_semanticdb_scalac",
+        ])
+
+    if scala_version.startswith("2.13.") or scala_version.startswith("3."):
+        # Since the Scala 2.13 compiler is included in Scala 3 deps.
+        result.extend([
+            "io_github_java_diff_utils_java_diff_utils",
+            "net_java_dev_jna_jna",
+            "org_jline_jline",
+        ])
+
+    if scala_version.startswith("3."):
+        result.extend([
+            "io_bazel_rules_scala_scala_asm",
+            "io_bazel_rules_scala_scala_compiler_2",
+            "io_bazel_rules_scala_scala_interfaces",
+            "io_bazel_rules_scala_scala_library_2",
+            "io_bazel_rules_scala_scala_reflect_2",
+            "io_bazel_rules_scala_scala_tasty_core",
+            "org_jline_jline_native",
+            "org_jline_jline_reader",
+            "org_jline_jline_terminal",
+            "org_jline_jline_terminal_jna",
+            "org_scala_sbt_compiler_interface",
+            "org_scala_sbt_util_interface",
+        ])
+
+    return result
+
 def rules_scala_toolchain_deps_repositories(
         maven_servers = _default_maven_server_urls(),
         overriden_artifacts = {},

scala/scalafmt/scalafmt_repositories.bzl

@@ -36,8 +36,6 @@ _SCALAFMT_DEPS = [
     "com_lihaoyi_fansi",
     "com_lihaoyi_fastparse",
     "com_lihaoyi_sourcecode",
-    "com_thesamet_scalapb_lenses",
-    "com_thesamet_scalapb_scalapb_runtime",
     "com_typesafe_config",
     "org_scala_lang_modules_scala_collection_compat",
     "org_scala_lang_scalap",
@@ -47,6 +45,8 @@ _SCALAFMT_DEPS = [
     "org_scalameta_scalameta",
     "org_scalameta_trees",
     "org_typelevel_paiges_core",
+    "scala_proto_rules_scalapb_lenses",
+    "scala_proto_rules_scalapb_runtime",
 ]
 
 _SCALAFMT_DEPS_2_11 = [

scala_proto/BUILD

@@ -82,8 +82,8 @@ declare_deps_provider(
     visibility = ["//visibility:public"],
     deps = [
         "@com_google_protobuf//:protobuf_java",
-        "@scala_proto_rules_protoc_bridge",
-        "@scala_proto_rules_scalapb_plugin",
+        "@scala_proto_rules_scalapb_compilerplugin",
+        "@scala_proto_rules_scalapb_protoc_bridge",
     ],
 )
 

scala_proto/default/default_deps.bzl

@@ -7,37 +7,37 @@
 # transitive path to be used without such a facility.
 #
 DEFAULT_SCALAPB_COMPILE_DEPS = [
-    "@scala_proto_rules_scalapb_runtime",
+    "//scala/private/toolchain_deps:scala_library_classpath",
     "@com_google_protobuf//:protobuf_java",
+    "@com_lihaoyi_fastparse",
     "@scala_proto_rules_scalapb_lenses",
-    "@scala_proto_rules_scalapb_fastparse",
-    "//scala/private/toolchain_deps:scala_library_classpath",
+    "@scala_proto_rules_scalapb_runtime",
 ]
 
 DEFAULT_SCALAPB_GRPC_DEPS = [
+    "@io_bazel_rules_scala_guava",
+    "@scala_proto_rules_disruptor",
     "@scala_proto_rules_grpc_api",
-    "@scala_proto_rules_perfmark_api",
-    "@scala_proto_rules_scalapb_runtime_grpc",
+    "@scala_proto_rules_grpc_context",
     "@scala_proto_rules_grpc_core",
-    "@scala_proto_rules_grpc_stub",
-    "@scala_proto_rules_grpc_protobuf",
     "@scala_proto_rules_grpc_netty",
-    "@scala_proto_rules_grpc_context",
-    "@scala_proto_rules_guava",
-    "@scala_proto_rules_opencensus_api",
-    "@scala_proto_rules_opencensus_impl",
-    "@scala_proto_rules_disruptor",
-    "@scala_proto_rules_opencensus_impl_core",
-    "@scala_proto_rules_opencensus_contrib_grpc_metrics",
-    "@scala_proto_rules_google_instrumentation",
+    "@scala_proto_rules_grpc_protobuf",
+    "@scala_proto_rules_grpc_stub",
+    "@scala_proto_rules_instrumentation_api",
+    "@scala_proto_rules_netty_buffer",
     "@scala_proto_rules_netty_codec",
     "@scala_proto_rules_netty_codec_http",
     "@scala_proto_rules_netty_codec_http2",
     "@scala_proto_rules_netty_codec_socks",
-    "@scala_proto_rules_netty_handler",
-    "@scala_proto_rules_netty_buffer",
-    "@scala_proto_rules_netty_transport",
-    "@scala_proto_rules_netty_resolver",
     "@scala_proto_rules_netty_common",
+    "@scala_proto_rules_netty_handler",
     "@scala_proto_rules_netty_handler_proxy",
+    "@scala_proto_rules_netty_resolver",
+    "@scala_proto_rules_netty_transport",
+    "@scala_proto_rules_opencensus_api",
+    "@scala_proto_rules_opencensus_contrib_grpc_metrics",
+    "@scala_proto_rules_opencensus_impl",
+    "@scala_proto_rules_opencensus_impl_core",
+    "@scala_proto_rules_perfmark_api",
+    "@scala_proto_rules_scalapb_runtime_grpc",
 ]

scala_proto/default/repositories.bzl

@@ -6,36 +6,38 @@ def scala_proto_default_repositories(
         overriden_artifacts = {}):
     repositories(
         for_artifact_ids = [
-            "scala_proto_rules_scalapb_plugin",
-            "scala_proto_rules_protoc_bridge",
-            "scala_proto_rules_scalapb_runtime",
-            "scala_proto_rules_scalapb_runtime_grpc",
-            "scala_proto_rules_scalapb_lenses",
-            "scala_proto_rules_scalapb_fastparse",
-            "scala_proto_rules_grpc_core",
+            "com_google_protobuf_protobuf_java",
+            "com_lihaoyi_fastparse",
+            "com_lihaoyi_sourcecode",
+            "io_bazel_rules_scala_guava",
+            "scala_proto_rules_disruptor",
+            "scala_proto_rules_instrumentation_api",
             "scala_proto_rules_grpc_api",
-            "scala_proto_rules_grpc_stub",
-            "scala_proto_rules_grpc_protobuf",
-            "scala_proto_rules_grpc_netty",
             "scala_proto_rules_grpc_context",
-            "scala_proto_rules_perfmark_api",
-            "scala_proto_rules_guava",
-            "scala_proto_rules_google_instrumentation",
+            "scala_proto_rules_grpc_core",
+            "scala_proto_rules_grpc_netty",
+            "scala_proto_rules_grpc_protobuf",
+            "scala_proto_rules_grpc_stub",
+            "scala_proto_rules_netty_buffer",
             "scala_proto_rules_netty_codec",
             "scala_proto_rules_netty_codec_http",
-            "scala_proto_rules_netty_codec_socks",
             "scala_proto_rules_netty_codec_http2",
-            "scala_proto_rules_netty_handler",
-            "scala_proto_rules_netty_buffer",
-            "scala_proto_rules_netty_transport",
-            "scala_proto_rules_netty_resolver",
+            "scala_proto_rules_netty_codec_socks",
             "scala_proto_rules_netty_common",
+            "scala_proto_rules_netty_handler",
             "scala_proto_rules_netty_handler_proxy",
+            "scala_proto_rules_netty_resolver",
+            "scala_proto_rules_netty_transport",
             "scala_proto_rules_opencensus_api",
+            "scala_proto_rules_opencensus_contrib_grpc_metrics",
             "scala_proto_rules_opencensus_impl",
-            "scala_proto_rules_disruptor",
             "scala_proto_rules_opencensus_impl_core",
-            "scala_proto_rules_opencensus_contrib_grpc_metrics",
+            "scala_proto_rules_perfmark_api",
+            "scala_proto_rules_scalapb_compilerplugin",
+            "scala_proto_rules_scalapb_lenses",
+            "scala_proto_rules_scalapb_protoc_bridge",
+            "scala_proto_rules_scalapb_runtime",
+            "scala_proto_rules_scalapb_runtime_grpc",
         ],
         maven_servers = maven_servers,
         fetch_sources = True,

scalatest/scalatest.bzl

@@ -12,18 +12,22 @@ def scalatest_repositories(
         repositories(
             scala_version = scala_version,
             for_artifact_ids = [
+                "io_bazel_rules_scala_scalactic",
                 "io_bazel_rules_scala_scalatest",
                 "io_bazel_rules_scala_scalatest_compatible",
                 "io_bazel_rules_scala_scalatest_core",
+                "io_bazel_rules_scala_scalatest_diagrams",
                 "io_bazel_rules_scala_scalatest_featurespec",
                 "io_bazel_rules_scala_scalatest_flatspec",
                 "io_bazel_rules_scala_scalatest_freespec",
-                "io_bazel_rules_scala_scalatest_funsuite",
                 "io_bazel_rules_scala_scalatest_funspec",
+                "io_bazel_rules_scala_scalatest_funsuite",
                 "io_bazel_rules_scala_scalatest_matchers_core",
-                "io_bazel_rules_scala_scalatest_shouldmatchers",
                 "io_bazel_rules_scala_scalatest_mustmatchers",
-                "io_bazel_rules_scala_scalactic",
+                "io_bazel_rules_scala_scalatest_propspec",
+                "io_bazel_rules_scala_scalatest_refspec",
+                "io_bazel_rules_scala_scalatest_shouldmatchers",
+                "io_bazel_rules_scala_scalatest_wordspec",
             ],
             maven_servers = maven_servers,
             fetch_sources = fetch_sources,