Refactor scripts/create_repository.py, fix some issues

[mbland]

Description

Reformats and refactors scripts/create_repository.py, prevents artifact downgrades, and improves performance. Part of #1482.

This change is comprised of several commits:

• The first is a pure reformatting and refactoring that produces the same output as before.
• The second is further reformatting and refactoring that still produces the same output.
• The third is further reformatting and refactoring still, but with a performance improvement, and prevents downgrading protobuf-java.
• The fourth is a small refactoring, but one I thought worth separating from previous ones.

Commits after that proceed to resolve various issues and improve performance:

• Ensure Scala 2 libs get "_2" repo names in Scala 3
• Bump Scalafmt to 3.8.3 in to match the version set in Update to Scalafmt 3.8.3 #1631
• Prevent jar downgrades by comparing new and existing versions
• Write all artifact deps to repo files, not just ones already existing in the original repository file

If this is too much for one pull request, I'm happy to tease it into as many smaller ones as requested.

I tested along the way by first restoring previous repository files at specific $COMMIT values via:

for f in third_party/repositories/scala_*.bzl; do git show $COMMIT:$f > $f; done

then I'd:

• run the script the first time to ensure the repository files remained the same as before (for the pure refactorings), or that the output was as expected
• git add third_party/repositories/scala_*.bzl if the updates were as expected
• run the script a second time to ensure no new changes appeared in the working directory

Motivation

Some of the trickiest, most time consuming work I've done while resolving #1482 has been in resolving repository versions by hand. This script is now starting to save me soooo much time. However, it needed some extra love not to undo some of the changes I've needed to make.

The easiest way for me to do that was to start with some pure reformatting and easy refactoring. After a couple iterations, I understood the script better, and could see where to add or update the logic to resolve some issues.

There's more I could do with this script to improve it further, I think, but that's for future pull requests. Many thanks and kudos to @lm1nrt for contributing the original in #1608.

[WojciechMazur]

Nice improvements!
I've thought about 1 additional thing that might be included as I was trying to use this script to make currently broken dangerous_test_thirdparty_version.sh work that is to allow updating/boostrapping single version of Scala using argument.

Below there's a patch to old repo version that would handle that, you might include it now or I can create a follow-up pr later.
It include 2 additional small fixes:

• running script from the root directory (previously it was requiring to run it from ./scritps)
• printing cs resolve stderr outputs only when command fails

diff --git a/scripts/create_repository.py b/scripts/create_repository.py
index a0bb4df..aaa0339 100644
--- a/scripts/create_repository.py
+++ b/scripts/create_repository.py
@@ -11,6 +11,7 @@ import ast
 import copy
 import glob
 import os
+import argparse
 
 root_scala_versions = ["2.11.12", "2.12.20", "2.13.15", "3.1.3", "3.2.2", "3.3.4", "3.4.3", "3.5.2"]
 scala_test_version = "3.2.9"
@@ -105,7 +106,9 @@ def map_to_resolved_artifacts(output) -> List[ResolvedArtifact]:
 def resolve_artifacts_with_checksums_and_direct_dependencies(root_artifacts) -> List[ResolvedArtifact]:
   command = f'cs resolve {' '.join(root_artifacts)}'
   proc = subprocess.run(command, capture_output=True, text=True, shell=True)
-  print(proc.stderr)
+  if proc.returncode > 0:
+    print("Resolving artifacts failed for command: ", command)
+    print(proc.stderr)
   output = proc.stdout.splitlines()
   return map_to_resolved_artifacts(output)
 
@@ -159,8 +162,8 @@ def write_to_file(artifact_dict, version, file):
     data.write('\nartifacts = ')
     data.write(f'{get_with_trailing_commas(json.dumps(artifact_dict, indent=4).replace('true', 'True').replace('false', 'False'))}\n')
 
-def create_file(version):
-  path = os.getcwd().replace('/scripts', '/third_party/repositories')
+def create_or_update_repository_file(version):
+  path = os.getcwd().replace('/scripts', '') + '/third_party/repositories'
   file = Path(f'{path}/{'scala_' + "_".join(version.split(".")[:2]) + '.bzl'}')
 
   if not file.exists():
@@ -196,5 +199,18 @@ def create_file(version):
 
   write_to_file(original_artifact_dict, version, file)
 
-for version in root_scala_versions:
-  create_file(version)
\ No newline at end of file
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(description="Setups repository configurations for given Scala version.")
+    parser.add_argument(
+        "--version", 
+        type=str, 
+        help="Scala version to bootstrap or update repositiry. If not provided, updates all default versions."
+    )
+    
+    args = parser.parse_args()
+    
+    if args.version is None:
+      for version in root_scala_versions:
+        create_or_update_repository_file(version)
+    else:
+      create_or_update_repository_file(args.version)

[mbland]

Nice improvements! I've thought about 1 additional thing that might be included as I was trying to use this script to make currently broken dangerous_test_thirdparty_version.sh work that is to allow updating/boostrapping single version of Scala using argument.

Thanks! 🙂

Below there's a patch to old repo version that would handle that, you might include it now or I can create a follow-up pr later. It include 2 additional small fixes:

• running script from the root directory (previously it was requiring to run it from ./scritps)
• printing cs resolve stderr outputs only when command fails

Heh, this branch already includes a change to allow it to run from the repo root. I'll see about patching in the stderr change, as well as the --version flag, right now.

[mbland]

@WojciechMazur OK, just pushed a new commit incorporating your suggestions. I added a new run_command wrapper around subprocess.run to make sure all the commands only emit output on failure.

[simuons]

Thanks @mbland! To be fair I didn't look very deeply into this PR (it's a bit big and this is fine, no need for many small PRs) I trust you and @WojciechMazur ;) Thank you both.