Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactor
scripts/create_repository.py, improve efficiency and corre…
…ctness, and bump Scalatest, protobuf-java, kind-projector (#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.
- Loading branch information
