Skip to content

Commit 8cb8279

Browse files
mblandmbland
committed
Strip Scala version from Maven artifact names
Solves the problem described in the new comments within `MavenCoordinates.new()` regarding ScalaPB artifacts flipping between `_2.13` and `_3` versions on subsequent runs. Also explicitly sets the `protoc-gen` root artifact to ensure we use the `_3` version for Scala >= 3.3. Replaces `ArtifactLabelMaker._remove_scala_version_suffix` with `MavenCoordinates.unversioned_artifact`.
1 parent baf584f commit 8cb8279

File tree

9 files changed

+91
-60
lines changed

9 files changed

+91
-60
lines changed

‎scripts/create_repository.py

+59-28
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
GRPC_LIBS = ['netty', 'protobuf', 'stub']
4141
GUAVA_VERSION = '33.3.1-jre'
4242

43+
# This should include values corresponding to `MavenCoordinates.artifact_name`,
44+
# i.e., group:artifact after stripping any Scala version suffix from artifact.
4345
EXCLUDED_ARTIFACTS = set(["com.google.guava:listenablefuture"])
4446

4547
THIS_FILE = Path(__file__)
@@ -77,14 +79,13 @@ def select_root_artifacts(scala_version, scala_major, is_scala_3) -> List[str]:
7779

7880
scala_2_version = scala_version
7981
scala_2_major = scala_major
80-
scalatest_major = scala_major
8182
scalapb_major = scala_2_major
8283

8384
if is_scala_3:
8485
scala_2_version = max_scala_2_version
8586
scala_2_major = max_scala_2_major
86-
scalatest_major = '3'
87-
scalapb_major = max_scala_2_major if minor_version < 3 else '3'
87+
scala_major = '3'
88+
scalapb_major = max_scala_2_major if minor_version < 3 else scala_major
8889

8990
scalafmt_version = SCALAFMT_VERSION
9091
scalapb_version = SCALAPB_VERSION
@@ -115,11 +116,17 @@ def select_root_artifacts(scala_version, scala_major, is_scala_3) -> List[str]:
115116
f'org.scala-lang:scala-reflect:{scala_2_version}',
116117
f'org.scala-lang:scalap:{scala_2_version}',
117118
f'org.scalameta:scalafmt-core_{scala_2_major}:{scalafmt_version}',
118-
f'org.scalatest:scalatest_{scalatest_major}:{SCALATEST_VERSION}',
119+
f'org.scalatest:scalatest_{scala_major}:{SCALATEST_VERSION}',
119120
f'org.typelevel:kind-projector_{scala_2_version}:' +
120121
KIND_PROJECTOR_VERSION,
121122
] + [f'io.grpc:grpc-{lib}:{GRPC_VERSION}' for lib in GRPC_LIBS]
122123

124+
if scala_major != '2.11':
125+
root_artifacts.append(
126+
f'com.thesamet.scalapb:protoc-gen_{scalapb_major}:' +
127+
protoc_bridge_version,
128+
)
129+
123130
if scala_version == max_scala_2_version or is_scala_3:
124131
# Since the Scala 2.13 compiler is included in Scala 3 deps.
125132
root_artifacts.append('org.jline:jline:' + JLINE_VERSION)
@@ -158,18 +165,51 @@ class MavenCoordinates:
158165
version: str
159166
coordinate: str
160167

168+
# The `artifact` with the Scala version suffix stripped
169+
unversioned_artifact: str
170+
171+
# Canonical name for comparing new and existing artifacts
172+
artifact_name: str
173+
161174
@staticmethod
162-
def new(artifact) -> Self:
175+
def new(coords) -> Self:
163176
"""Creates a new MavenCoordinates from a Maven coordinate string."""
164177
# There are Maven artifacts that contain extra components like `:jar` in
165178
# their coordinates. However, the groupId and artifactId are always the
166179
# first two components, and the version is the last.
167-
parts = artifact.split(':')
168-
return MavenCoordinates(parts[0], parts[1], parts[-1], artifact)
169-
170-
def artifact_name(self):
171-
"""Returns the name to use as a hash key for existing artifacts."""
172-
return f'{self.group}:{self.artifact}'
180+
parts = coords.split(':')
181+
group, artifact, vers = parts[0], parts[1], parts[-1]
182+
183+
# Remove any Scala version suffix from what will become the
184+
# `artifact_name`. This is to avoid consecutive runs of the script
185+
# flipping between the `_2.x` and `_3` versions of some artifacts.
186+
#
187+
# Specifically, there are ScalaPB root artifacts specified by this
188+
# script that end in `_3` yet still transitively depend on artifacts
189+
# ending in `_2.13`. However, some of these transitive dependencies are
190+
# also specified as root artifacts ending in `_3`.
191+
#
192+
# Without trimming the version suffix, the script would see the `_3`
193+
# root artifacts and the `_2.13` transitive dependency artifacts as
194+
# entirely different. However, their computed repository labels would be
195+
# the same, causing one version to replace the other on consecutive
196+
# runs.
197+
artifact_parts = artifact.rsplit('_', 1)
198+
199+
if len(artifact_parts) != 1:
200+
version_suffix = artifact_parts[-1]
201+
202+
# "Why does `'2.13'.isdecimal()` return `False`, sir?"
203+
# "Nobody knows."
204+
# See: https://youtu.be/JYqfVE-fykk (couldn't resist!)
205+
if version_suffix.split('.')[0].isdigit():
206+
del artifact_parts[-1]
207+
208+
unversioned_artifact = '_'.join(artifact_parts)
209+
artifact_name = f'{group}:{unversioned_artifact}'
210+
return MavenCoordinates(
211+
group, artifact, vers, coords, unversioned_artifact, artifact_name
212+
)
173213

174214
def is_newer_than(self, other):
175215
"""Determines if this artifact is newer than the other.
@@ -188,7 +228,7 @@ def is_newer_than(self, other):
188228
CreateRepositoryError if other doesn't match self.group and
189229
self.artifact
190230
"""
191-
if (self.group != other.group) or (self.artifact != other.artifact):
231+
if self.artifact_name != other.artifact_name:
192232
raise CreateRepositoryError(
193233
f'Expected {self.group}:{self.artifact}, ' +
194234
f'got {other.group}:{other.artifact}'
@@ -234,8 +274,7 @@ def get_label(self, coordinates) -> str:
234274
def _get_label_impl(self, coordinates) -> str:
235275
group = coordinates.group
236276
group_label = self._labelize(group)
237-
artifact = self._remove_scala_version_suffix(coordinates.artifact)
238-
artifact_label = self._labelize(artifact)
277+
artifact_label = self._labelize(coordinates.unversioned_artifact)
239278

240279
if group in self._SCALA_LANG_GROUPS:
241280
return self._get_scala_lang_label(artifact_label, coordinates)
@@ -246,7 +285,7 @@ def _get_label_impl(self, coordinates) -> str:
246285
if group in self._SCALA_PROTO_RULES_GROUPS:
247286
return self._get_scala_proto_label(artifact_label, coordinates)
248287

249-
artifact_name = f'{group}:{artifact}'
288+
artifact_name = coordinates.artifact_name
250289

251290
if artifact_name in self._SPECIAL_CASE_ARTIFACT_LABELS:
252291
return self._SPECIAL_CASE_ARTIFACT_LABELS[artifact_name]
@@ -256,14 +295,6 @@ def _get_label_impl(self, coordinates) -> str:
256295
def _labelize(s):
257296
return s.replace('.', '_').replace('-', '_')
258297

259-
@staticmethod
260-
def _remove_scala_version_suffix(artifact):
261-
"""Removes the Scala version suffix from artifact, e.g., scopt_2.13."""
262-
parts = artifact.split('_')
263-
if len(parts) != 1 and parts[-1][0].isdigit():
264-
return '_'.join(parts[:-1])
265-
return artifact
266-
267298
_ARTIFACT_LABEL_ONLY_GROUPS = set([
268299
"com.google.guava",
269300
"com.twitter",
@@ -361,9 +392,9 @@ def resolve_artifacts(
361392

362393
for artifact in artifacts_data['dependencies']:
363394
coords = MavenCoordinates.new(artifact['coord'])
364-
current = current_artifacts_map.get(coords.artifact_name())
395+
current = current_artifacts_map.get(coords.artifact_name)
365396

366-
if coords.artifact_name() in EXCLUDED_ARTIFACTS:
397+
if coords.artifact_name in EXCLUDED_ARTIFACTS:
367398
continue
368399

369400
if current is None or coords.is_newer_than(current.coordinates):
@@ -395,7 +426,7 @@ def _create_current_artifacts_map(original_artifacts):
395426

396427
for metadata in original_artifacts.values():
397428
coordinates = MavenCoordinates.new(metadata['artifact'])
398-
name = coordinates.artifact_name()
429+
name = coordinates.artifact_name
399430

400431
if name not in result and metadata.get('testonly') is not True:
401432
result[name] = ResolvedArtifact(
@@ -412,7 +443,7 @@ def _get_artifact_metadata(self, artifact) -> str:
412443
MavenCoordinates.new(d) for d in artifact['directDependencies']
413444
]
414445
metadata['deps'] = [
415-
d for d in deps if d.artifact_name() not in EXCLUDED_ARTIFACTS
446+
d for d in deps if d.artifact_name not in EXCLUDED_ARTIFACTS
416447
]
417448
with open(artifact['file'], 'rb') as f:
418449
metadata['checksum'] = hashlib.sha256(f.read()).hexdigest()
@@ -533,7 +564,7 @@ def _update_artifact_labels(artifacts, labeler):
533564
for existing_label, metadata in artifacts.items():
534565
coords = MavenCoordinates.new(metadata['artifact'])
535566

536-
if coords.artifact_name() in EXCLUDED_ARTIFACTS:
567+
if coords.artifact_name in EXCLUDED_ARTIFACTS:
537568
continue
538569

539570
label = (

‎third_party/repositories/scala_2_12.bzl

+2-2
Original file line numberDiff line numberDiff line change
@@ -823,8 +823,8 @@ artifacts = {
823823
],
824824
},
825825
"scala_proto_rules_scalapb_protoc_gen": {
826-
"artifact": "com.thesamet.scalapb:protoc-gen_2.12:0.9.7",
827-
"sha256": "81df11e24e52887515dff20eb4d1a050fd58e078200291c3c87fd04218abe53b",
826+
"artifact": "com.thesamet.scalapb:protoc-gen_2.12:0.9.8",
827+
"sha256": "65391bf190ac9cab45674dcd8063893e4bffd4f7289742cad145962b42928648",
828828
"deps": [
829829
"@io_bazel_rules_scala_scala_library",
830830
"@scala_proto_rules_scalapb_protoc_bridge",

‎third_party/repositories/scala_2_13.bzl

+2-2
Original file line numberDiff line numberDiff line change
@@ -845,8 +845,8 @@ artifacts = {
845845
],
846846
},
847847
"scala_proto_rules_scalapb_protoc_gen": {
848-
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
849-
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
848+
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.8",
849+
"sha256": "cf2b50721952cb4f10ca05a0ed36d7b01b88eb6505a9478556ee5a7af1a21775",
850850
"deps": [
851851
"@io_bazel_rules_scala_scala_library",
852852
"@scala_proto_rules_scalapb_protoc_bridge",

‎third_party/repositories/scala_3_1.bzl

+2-2
Original file line numberDiff line numberDiff line change
@@ -887,8 +887,8 @@ artifacts = {
887887
],
888888
},
889889
"scala_proto_rules_scalapb_protoc_gen": {
890-
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
891-
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
890+
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.8",
891+
"sha256": "cf2b50721952cb4f10ca05a0ed36d7b01b88eb6505a9478556ee5a7af1a21775",
892892
"deps": [
893893
"@io_bazel_rules_scala_scala_library_2",
894894
"@scala_proto_rules_scalapb_protoc_bridge",

‎third_party/repositories/scala_3_2.bzl

+2-2
Original file line numberDiff line numberDiff line change
@@ -887,8 +887,8 @@ artifacts = {
887887
],
888888
},
889889
"scala_proto_rules_scalapb_protoc_gen": {
890-
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
891-
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
890+
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.8",
891+
"sha256": "cf2b50721952cb4f10ca05a0ed36d7b01b88eb6505a9478556ee5a7af1a21775",
892892
"deps": [
893893
"@io_bazel_rules_scala_scala_library_2",
894894
"@scala_proto_rules_scalapb_protoc_bridge",

‎third_party/repositories/scala_3_3.bzl

+6-6
Original file line numberDiff line numberDiff line change
@@ -880,18 +880,18 @@ artifacts = {
880880
],
881881
},
882882
"scala_proto_rules_scalapb_protoc_bridge": {
883-
"artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.7",
884-
"sha256": "403f0e7223c8fd052cff0fbf977f3696c387a696a3a12d7b031d95660c7552f5",
883+
"artifact": "com.thesamet.scalapb:protoc-bridge_3:0.9.8",
884+
"sha256": "6e1e38e34f3aaa14c6d46defb66b819f03edbdc4d69965011955da2a4781df9c",
885885
"deps": [
886886
"@dev_dirs_directories",
887-
"@io_bazel_rules_scala_scala_library_2",
887+
"@io_bazel_rules_scala_scala_library",
888888
],
889889
},
890890
"scala_proto_rules_scalapb_protoc_gen": {
891-
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
892-
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
891+
"artifact": "com.thesamet.scalapb:protoc-gen_3:0.9.8",
892+
"sha256": "9e5eebe35ca68a884adf3c6cade094055192d3243d29a26d32ae6b5396b39e08",
893893
"deps": [
894-
"@io_bazel_rules_scala_scala_library_2",
894+
"@io_bazel_rules_scala_scala_library",
895895
"@scala_proto_rules_scalapb_protoc_bridge",
896896
],
897897
},

‎third_party/repositories/scala_3_4.bzl

+6-6
Original file line numberDiff line numberDiff line change
@@ -880,18 +880,18 @@ artifacts = {
880880
],
881881
},
882882
"scala_proto_rules_scalapb_protoc_bridge": {
883-
"artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.7",
884-
"sha256": "403f0e7223c8fd052cff0fbf977f3696c387a696a3a12d7b031d95660c7552f5",
883+
"artifact": "com.thesamet.scalapb:protoc-bridge_3:0.9.8",
884+
"sha256": "6e1e38e34f3aaa14c6d46defb66b819f03edbdc4d69965011955da2a4781df9c",
885885
"deps": [
886886
"@dev_dirs_directories",
887-
"@io_bazel_rules_scala_scala_library_2",
887+
"@io_bazel_rules_scala_scala_library",
888888
],
889889
},
890890
"scala_proto_rules_scalapb_protoc_gen": {
891-
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
892-
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
891+
"artifact": "com.thesamet.scalapb:protoc-gen_3:0.9.8",
892+
"sha256": "9e5eebe35ca68a884adf3c6cade094055192d3243d29a26d32ae6b5396b39e08",
893893
"deps": [
894-
"@io_bazel_rules_scala_scala_library_2",
894+
"@io_bazel_rules_scala_scala_library",
895895
"@scala_proto_rules_scalapb_protoc_bridge",
896896
],
897897
},

‎third_party/repositories/scala_3_5.bzl

+6-6
Original file line numberDiff line numberDiff line change
@@ -880,18 +880,18 @@ artifacts = {
880880
],
881881
},
882882
"scala_proto_rules_scalapb_protoc_bridge": {
883-
"artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.7",
884-
"sha256": "403f0e7223c8fd052cff0fbf977f3696c387a696a3a12d7b031d95660c7552f5",
883+
"artifact": "com.thesamet.scalapb:protoc-bridge_3:0.9.8",
884+
"sha256": "6e1e38e34f3aaa14c6d46defb66b819f03edbdc4d69965011955da2a4781df9c",
885885
"deps": [
886886
"@dev_dirs_directories",
887-
"@io_bazel_rules_scala_scala_library_2",
887+
"@io_bazel_rules_scala_scala_library",
888888
],
889889
},
890890
"scala_proto_rules_scalapb_protoc_gen": {
891-
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
892-
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
891+
"artifact": "com.thesamet.scalapb:protoc-gen_3:0.9.8",
892+
"sha256": "9e5eebe35ca68a884adf3c6cade094055192d3243d29a26d32ae6b5396b39e08",
893893
"deps": [
894-
"@io_bazel_rules_scala_scala_library_2",
894+
"@io_bazel_rules_scala_scala_library",
895895
"@scala_proto_rules_scalapb_protoc_bridge",
896896
],
897897
},

‎third_party/repositories/scala_3_6.bzl

+6-6
Original file line numberDiff line numberDiff line change
@@ -889,18 +889,18 @@ artifacts = {
889889
],
890890
},
891891
"scala_proto_rules_scalapb_protoc_bridge": {
892-
"artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.7",
893-
"sha256": "403f0e7223c8fd052cff0fbf977f3696c387a696a3a12d7b031d95660c7552f5",
892+
"artifact": "com.thesamet.scalapb:protoc-bridge_3:0.9.8",
893+
"sha256": "6e1e38e34f3aaa14c6d46defb66b819f03edbdc4d69965011955da2a4781df9c",
894894
"deps": [
895895
"@dev_dirs_directories",
896-
"@io_bazel_rules_scala_scala_library_2",
896+
"@io_bazel_rules_scala_scala_library",
897897
],
898898
},
899899
"scala_proto_rules_scalapb_protoc_gen": {
900-
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
901-
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
900+
"artifact": "com.thesamet.scalapb:protoc-gen_3:0.9.8",
901+
"sha256": "9e5eebe35ca68a884adf3c6cade094055192d3243d29a26d32ae6b5396b39e08",
902902
"deps": [
903-
"@io_bazel_rules_scala_scala_library_2",
903+
"@io_bazel_rules_scala_scala_library",
904904
"@scala_proto_rules_scalapb_protoc_bridge",
905905
],
906906
},

0 commit comments

Comments
 (0)