Skip to content

Commit

Permalink
Bump protoc-bridge to 0.9.8 for Scala >= 2.12 (#1688)
Browse files Browse the repository at this point in the history
This required updates to `scripts/create_repository.py` to accommodate
comparisons between `_2.13` and `_3` versions of ScalaPB artifacts. The
`_3` versions of the most recent ScalaPB artifacts are only available
for Scala 3.3 and later, and `compilerplugin_3` currently still depends
on `protoc-gen_2.13`.

The `MavenCoordinates` dataclass in `scripts/create_repository.py` has
new `unversioned_artifact`, `scala_version`, and `artifact_name`
members. Updates to `MavenCoordinates.is_newer_than()` now compare
objects with matching `artifact_name` values, and take the
`scala_version` into account. These changes then precipitated:

- Replacing the `artifact_name()` call with `artifact_name` accesses.
- Replacing `ArtifactLabelMaker._remove_scala_version_suffix` with
    `MavenCoordinates.unversioned_artifact`.
- Extracting `__compare_versions` from `is_newer_than` to reuse the same
  code for comparing Scala versions and artifact versions. (Properly
  reversed the use of `lhs` and `rhs` in the process.)

This change also removes an unnecessary `try`/`catch` block from
`scripts.ScalaPbCodeGenerator.process()` for Scala 2.12 and above.
The block is still required for the Scala 2.11 implementation of
`scripts.ScalaPbCodeGenerator.run()` and in
`scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator.run()`.

Finally, this change adds notes in a few places to indicate where
support remains for Scala 2.11. This supporting code can ultimately be
removed if we ever decide to drop Scala 2.11 support.

---

Tested by setting the `protobuf` dependencies to a versions I know will
break `ScalaPB` 0.11.17:

- `abseil-cpp`: 20220623.1 => 20240722.0
- `protobuf`: v21.7 => v26.1

I temporarily removed the `try`/`catch` block for `Throwable` from
`scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator.run`.
I then ran `bazel shutdown` to stop persistent `ProtoScalaPBRule`
workers.

After that, building with Bazel 7.4.1 (since Bazel 6.5.0 can't build
`abseil-cpp` 20240722.0 without setting C++14 compiler flags) crashed as
expected, instead of hanging. Undoing the `abseil-cpp` and `protobuf`
changes and building then produced a working build.

```txt
$ USE_BAZEL_VERSION=7.4.1 bazel build //third_party/test/proto:scala

[ ...snip... ]

ERROR: third_party/test/proto/BUILD.bazel:4:14:
  ProtoScalaPBRule
  third_party/test/proto/proto_jvm_extra_protobuf_generator_scalapb.srcjar
  failed: (Exit 1):
  scalapb_worker failed: error executing ProtoScalaPBRule command
  (from target //third_party/test/proto:proto)
  bazel-out/darwin_arm64-opt-exec-ST-a828a81199fe/bin/src/scala/scripts/scalapb_worker
    ... (remaining 2 arguments skipped)
--jvm_extra_protobuf_generator_out: java.lang.VerifyError: Cannot inherit from final class
    at java.base/java.lang.ClassLoader.defineClass1(Native Method)
    at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1022)
    at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:174)
    at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:800)
    at java.base/jdk.internal.loader.BuiltinClassLoader$4.run(BuiltinClassLoader.java:711)
    at java.base/jdk.internal.loader.BuiltinClassLoader$4.run(BuiltinClassLoader.java:706)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:719)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:621)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:579)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:527)
    at scalapb.options.Scalapb.<clinit>(Scalapb.java:24835)
    at scalapb.options.compiler.Scalapb$.registerAllExtensions(Scalapb.scala:8)
    at scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator$.run(ExtraProtobufGenerator.scala:52)
    at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:48)
    at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:113)
    at protocbridge.frontend.SocketBasedPluginFrontend.$anonfun$prepare$2(SocketBasedPluginFrontend.scala:31)
    at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
    at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1$$anon$2.block(ExecutionContextImpl.scala:75)
    at java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3118)
    at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1.blockOn(ExecutionContextImpl.scala:87)
    at scala.concurrent.package$.blocking(package.scala:146)
    at protocbridge.frontend.SocketBasedPluginFrontend.$anonfun$prepare$1(SocketBasedPluginFrontend.scala:23)
    at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
    at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:659)
    at scala.util.Success.$anonfun$map$1(Try.scala:255)
    at scala.util.Success.map(Try.scala:213)
    at scala.concurrent.Future.$anonfun$map$1(Future.scala:292)
    at scala.concurrent.impl.Promise.$anonfun$transform$1(Promise.scala:42)
    at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:74)
    at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426)
    at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
    at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
    at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
    at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
    at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

java.lang.RuntimeException: Exit with code 1
    at scala.sys.package$.error(package.scala:30)
    at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44)
    at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96)
    at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49)
    at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39)
    at scripts.ScalaPBWorker.main(ScalaPBWorker.scala)

Target //third_party/test/proto:scala failed to build

ERROR: third_party/test/proto/BUILD.bazel:4:14
  scala @//third_party/test/proto:proto failed:
  (Exit 1): scalapb_worker failed:
  error executing ProtoScalaPBRule command
  (from target //third_party/test/proto:proto)
  bazel-out/darwin_arm64-opt-exec-ST-a828a81199fe/bin/src/scala/scripts/scalapb_worker
    ... (remaining 2 arguments skipped)
```

---

The initial motivation for this change was to try to eliminate the need
for the custom `scripts.ScalaPbCodeGenerator` implementation by
upgrading to `protoc-bridge` 0.9.8. This version contains a change to
`catch` any `Throwable` objects raised by a generator implementation in
`protocbridge.frontend.PluginFrontend.runWithInputStream()`.

- https://github.com/scalapb/protoc-bridge/blob/c574d50eaee5b800fd54493fe25c4e0eed3b9def/bridge/src/main/scala/protocbridge/frontend/PluginFrontend.scala#L107-L122

This change originally came from:

- https://github.com/scalapb/protoc-bridge/blob/d0d56f635d13f7efaa2755ed0d2d66bdef18b588/bridge/src/main/scala/protocbridge/frontend/PluginFrontend.scala#L107-L122
- scalapb/protoc-bridge#367

I closed scalapb/ScalaPB#1771 as a result, and reexamined the
`try`/`catch` blocks added in #1630 and #1637, and the
`scripts.ScalaPBCodeGenerator` implementations added in #1648.

However, the last `protoc-bridge` version to support Scala 2.11 is
0.7.14. Scala 2.11 won't be able to use `protoc-bridge` 0.9.8, so we
still need to `catch` any `Throwable`s ourselves. This includes the
`catch` blocks from the Scala 2.11 `scripts.ScalaPbCodeGenerator` and
`scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator.run`.

---

In `protoc-bridge` 0.9.7 and 0.9.8, `protocbridge.ProtocCodeGenerator`
declares a `run()` method with no implementation (and no `try`/`catch`
block).

- https://github.com/scalapb/protoc-bridge/blob/v0.9.7/bridge/src/main/scala/protocbridge/ProtocCodeGenerator.scala#L5

In `protoc-gen` 0.9.7 and 0.9.8, `protogen.CodeGenApp` implements
`protocbridge.ProtocCodeGenerator`, and `CodeGenApp.run()` wraps a call
to `CodeGenApp.process()` inside a `try`/`catch` block.

- https://github.com/scalapb/protoc-bridge/blob/v0.9.7/protoc-gen/src/main/scala/protocgen/CodeGenApp.scala#L41-L56

`scalapb.ScalaPbCodeGenerator` from `compilerplugin` 0.11.17 extends
`CodeGenApp` and implements `process()`. For this version of
`ScalaPbCodeGenerator`, available in Scala 2.12 and later, there's no
need for a `try`/`catch` wrapper on our end.

- https://github.com/scalapb/ScalaPB/blob/v0.11.17/compiler-plugin/src/main/scala/scalapb/ScalaPbCodeGenerator.scala#L11

However, the last available `compilerplugin` version for Scala 2.11 is
0.9.8. `scalapb.ScalaPbCodeGenerator` from that version implements
`protocbridge.ProtocCodeGenerator` and has a `try`/`catch` in its
`run()` method. However, the `Scalapb.registerAllExtensions(registry)`
call on line 14 lies outside this block, producing the crash described
in #1648 (commit 23ae356). This is why
we need the Scala 2.11 implementation of `scripts.ScalaPbCodeGenerator`.

- https://github.com/scalapb/ScalaPB/blob/v0.9.8/compiler-plugin/src/main/scala/scalapb/ScalaPbCodeGenerator.scala#L14

`scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator`,
also extends `ProtocCodeGenerator` directly, instead of implementing
`CodeGenApp` (so it also builds with Scala 2.11). This is why
`ExtraProtobufGenerator.run()` hangs when we remove the `try`/`catch`
block and run with `protoc-bridge` 0.9.7 and `protobuf` > v25.5, even
with later Scala versions.

---

Since `scalapb.ScalaPbCodeGenerator` from `compilerplugin` 0.11.17
implements `CodeGenApp`, `scripts.ScalaPbCodeGenerator` for Scala 2.12
should be unnecessary. The only reason we need it is to maintain the
same interface as the Scala 2.11 implementation.

---

Before the `MavenCoordinates` changes, `scripts/create_repository.py`
would ScalaPB flip ScalaPB artifacts between their `_2.13` and `_3`
versions on subsequent runs (e.g., `protoc-bridge_2.13` vs.
`protoc-bridge_3`). See the comments added within
`MavenCoordinates.new()` for details.

---

Building Scala 3.3 and later versions with `compilerplugin_3` and
`protoc-bridge_3` artifacts (instead of `protoc-bridge_2.13` artifacts)
produces the following build failure:

```txt
$ RULES_SCALA_TEST_ONLY="test_scala_version 3.6.2" \
    ./test_thirdparty_version.sh

[ ...snip... ]

ERROR: third_party/test/proto/BUILD.bazel:4:14:
  ProtoScalaPBRule
  third_party/test/proto/proto_jvm_extra_protobuf_generator_scalapb.srcjar
  failed: (Exit 1): scalapb_worker failed: error executing command
  (from target //third_party/test/proto:proto)
  bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/src/scala/scripts/scalapb_worker
    ... (remaining 2 arguments skipped)

--scala_out: java.lang.NoClassDefFoundError:
  Could not initialize class scalapb.ScalaPbCodeGenerator$
    at scripts.ScalaPbCodeGenerator$.process(ScalaPbCodeGeneratorWrapper.scala:8)
    at protocgen.CodeGenApp.run(CodeGenApp.scala:48)
    at protocgen.CodeGenApp.run$(CodeGenApp.scala:41)
    at scripts.ScalaPbCodeGenerator$.run(ScalaPbCodeGeneratorWrapper.scala:5)
    at protocgen.CodeGenApp.run(CodeGenApp.scala:33)
    at protocgen.CodeGenApp.run$(CodeGenApp.scala:32)
    at scripts.ScalaPbCodeGenerator$.run(ScalaPbCodeGeneratorWrapper.scala:5)
    at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:48)
    at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:113)
    at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1$$anonfun$1(SocketBasedPluginFrontend.scala:31)
    at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1$$anonfun$adapted$1(SocketBasedPluginFrontend.scala:37)
    at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1$$anon$2.block(ExecutionContextImpl.scala:60)
    at java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3118)
    at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1.blockOn(ExecutionContextImpl.scala:71)
    at scala.concurrent.package$.blocking(package.scala:124)
    at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$1(SocketBasedPluginFrontend.scala:37)
    at protocbridge.frontend.SocketBasedPluginFrontend.prepare$$anonfun$adapted$1(SocketBasedPluginFrontend.scala:38)
    at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:687)
    at scala.concurrent.impl.Promise$Transformation.run(Promise.scala:467)
    at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426)
    at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
    at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
    at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
    at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
    at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

java.lang.RuntimeException: Exit with code 1
    at scala.sys.package$.error(package.scala:27)
    at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44)
    at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96)
    at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49)
    at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39)
    at scripts.ScalaPBWorker.main(ScalaPBWorker.scala)

ERROR: third_party/test/proto/BUILD.bazel:4:14 Building source jar
  third_party/test/proto/proto_scalapb-src.jar failed: (Exit 1):
  scalapb_worker failed: error executing command
  (from target //third_party/test/proto:proto)
  bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/src/scala/scripts/scalapb_worker
    ... (remaining 2 arguments skipped)
```
  • Loading branch information
mbland authored Feb 3, 2025
1 parent c71c00a commit 870d952
Show file tree
Hide file tree
Showing 13 changed files with 209 additions and 142 deletions.
7 changes: 7 additions & 0 deletions scala_proto/scala_proto_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ scala_proto_toolchain = rule(
default = Label("//src/scala/scripts:scalapb_worker"),
allow_files = True,
),
# `scripts.ScalaPbCodeGenerator` and `_main_generator_dep` are currently
# necessary to support protoc-bridge < 0.9.8, specifically 0.7.14
# required by Scala 2.11. See #1647 and scalapb/ScalaPB#1771.
#
# If we drop 2.11 support, restore `scalapb.ScalaPbCodeGenerator` here,
# remove `_main_generator_dep`, and delete
# `//src/scala/scripts:scalapb_codegenerator_wrapper` and its files.
"main_generator": attr.string(
default = "scripts.ScalaPbCodeGenerator",
),
Expand Down
132 changes: 95 additions & 37 deletions scripts/create_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@
PROTOBUF_JAVA_VERSION = "4.29.0"
JLINE_VERSION = '3.27.1'
SCALAPB_VERSION = '0.11.17'
PROTOC_BRIDGE_VERSION = '0.9.7'
PROTOC_BRIDGE_VERSION = '0.9.8'
GRPC_VERSION = '1.68.1'
GRPC_COMMON_PROTOS_VERSION = '2.48.0'
GRPC_LIBS = ['netty', 'protobuf', 'stub']
GUAVA_VERSION = '33.3.1-jre'

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

THIS_FILE = Path(__file__)
Expand Down Expand Up @@ -73,15 +75,25 @@ def select_root_artifacts(scala_version, scala_major, is_scala_3) -> List[str]:
v for v in ROOT_SCALA_VERSIONS if v.startswith('2.')
)
max_scala_2_major = '.'.join(max_scala_2_version.split('.')[:2])
minor_version = int(scala_version.split('.')[1])

scala_2_version = scala_version
scala_2_major = scala_major
scalatest_major = scala_major
scalapb_major = scala_2_major

if is_scala_3:
scala_2_version = max_scala_2_version
scala_2_major = max_scala_2_major
scalatest_major = '3'
scala_major = '3'
scalapb_major = scala_2_major if minor_version < 3 else scala_major

# For some reason, com.thesamet.scalapb:compilerplugin_3:0.11.17 depends on
# com.thesamet.scalapb:protoc-gen_2.13:0.9.7. Trying to use
# com.thesamet.scalapb:protoc-gen_3:0.9.8 causes a crash, saying:
# `java.lang.NoClassDefFoundError: Could not initialize class
# scalapb.ScalaPbCodeGenerator$`, even though that class is definitely in
# that jar. So we stick with protoc-gen_2.13 for now.
protoc_bridge_major = scala_2_major

scalafmt_version = SCALAFMT_VERSION
scalapb_version = SCALAPB_VERSION
Expand All @@ -97,13 +109,13 @@ def select_root_artifacts(scala_version, scala_major, is_scala_3) -> List[str]:
GRPC_COMMON_PROTOS_VERSION,
f'com.google.guava:guava:{GUAVA_VERSION}',
f'com.google.protobuf:protobuf-java:{PROTOBUF_JAVA_VERSION}',
f'com.thesamet.scalapb:compilerplugin_{scala_2_major}:' +
f'com.thesamet.scalapb:compilerplugin_{scalapb_major}:' +
scalapb_version,
f'com.thesamet.scalapb:protoc-bridge_{scala_2_major}:' +
f'com.thesamet.scalapb:protoc-bridge_{protoc_bridge_major}:' +
protoc_bridge_version,
f'com.thesamet.scalapb:scalapb-runtime_{scala_2_major}:' +
f'com.thesamet.scalapb:scalapb-runtime_{scalapb_major}:' +
scalapb_version,
f'com.thesamet.scalapb:scalapb-runtime-grpc_{scala_2_major}:' +
f'com.thesamet.scalapb:scalapb-runtime-grpc_{scalapb_major}:' +
scalapb_version,
f'org.scala-lang.modules:scala-parser-combinators_{scala_2_major}:' +
PARSER_COMBINATORS_VERSION,
Expand All @@ -112,11 +124,17 @@ def select_root_artifacts(scala_version, scala_major, is_scala_3) -> List[str]:
f'org.scala-lang:scala-reflect:{scala_2_version}',
f'org.scala-lang:scalap:{scala_2_version}',
f'org.scalameta:scalafmt-core_{scala_2_major}:{scalafmt_version}',
f'org.scalatest:scalatest_{scalatest_major}:{SCALATEST_VERSION}',
f'org.scalatest:scalatest_{scala_major}:{SCALATEST_VERSION}',
f'org.typelevel:kind-projector_{scala_2_version}:' +
KIND_PROJECTOR_VERSION,
] + [f'io.grpc:grpc-{lib}:{GRPC_VERSION}' for lib in GRPC_LIBS]

if scala_major != '2.11':
root_artifacts.append(
f'com.thesamet.scalapb:protoc-gen_{protoc_bridge_major}:' +
protoc_bridge_version,
)

if scala_version == max_scala_2_version or is_scala_3:
# Since the Scala 2.13 compiler is included in Scala 3 deps.
root_artifacts.append('org.jline:jline:' + JLINE_VERSION)
Expand Down Expand Up @@ -156,18 +174,62 @@ class MavenCoordinates:
version: str
coordinate: str

# The `artifact` with the Scala version suffix stripped
unversioned_artifact: str

# The Scala version suffix stripped from `unversioned_artifact`
scala_version: str

# Canonical name for comparing new and existing artifacts
artifact_name: str

@staticmethod
def new(artifact) -> Self:
def new(coords) -> Self:
"""Creates a new MavenCoordinates from a Maven coordinate string."""
# There are Maven artifacts that contain extra components like `:jar` in
# their coordinates. However, the groupId and artifactId are always the
# first two components, and the version is the last.
parts = artifact.split(':')
return MavenCoordinates(parts[0], parts[1], parts[-1], artifact)

def artifact_name(self):
"""Returns the name to use as a hash key for existing artifacts."""
return f'{self.group}:{self.artifact}'
parts = coords.split(':')
group, artifact, vers = parts[0], parts[1], parts[-1]

# Remove any Scala version suffix from what will become the
# `artifact_name`. This is to avoid consecutive runs of the script
# flipping between the `_2.x` and `_3` versions of some artifacts.
#
# Specifically, there are ScalaPB root artifacts specified by this
# script that end in `_3` yet still transitively depend on artifacts
# ending in `_2.13`. However, some of these transitive dependencies are
# also specified as root artifacts ending in `_3`.
#
# Without trimming the version suffix, the script would see the `_3`
# root artifacts and the `_2.13` transitive dependency artifacts as
# entirely different. However, their computed repository labels would be
# the same, causing one version to replace the other on consecutive
# runs.
artifact_parts = artifact.rsplit('_', 1)
scala_version = ''

if len(artifact_parts) != 1:
version_suffix = artifact_parts[-1]

# "Why does `'2.13'.isdecimal()` return `False`, sir?"
# "Nobody knows."
# See: https://youtu.be/JYqfVE-fykk (couldn't resist!)
if version_suffix.split('.')[0].isdigit():
scala_version = version_suffix
del artifact_parts[-1]

unversioned_artifact = '_'.join(artifact_parts)
artifact_name = f'{group}:{unversioned_artifact}'
return MavenCoordinates(
group,
artifact,
vers,
coords,
unversioned_artifact,
scala_version,
artifact_name,
)

def is_newer_than(self, other):
"""Determines if this artifact is newer than the other.
Expand All @@ -186,23 +248,28 @@ def is_newer_than(self, other):
CreateRepositoryError if other doesn't match self.group and
self.artifact
"""
if (self.group != other.group) or (self.artifact != other.artifact):
if self.artifact_name != other.artifact_name:
raise CreateRepositoryError(
f'Expected {self.group}:{self.artifact}, ' +
f'got {other.group}:{other.artifact}'
)
return (
self.__compare_versions(other.scala_version, self.scala_version) or
self.__compare_versions(other.version, self.version)
)

lhs_parts = self.version.split(".")
rhs_parts = other.version.split(".")
def __compare_versions(self, lhs, rhs):
lhs_parts = lhs.split('.')
rhs_parts = rhs.split('.')

for lhs_part, rhs_part in zip(lhs_parts, rhs_parts):
if lhs_part == rhs_part:
continue
if lhs_part.isdecimal() and rhs_part.isdecimal():
return int(rhs_part) < int(lhs_part)
return rhs_part < lhs_part
return int(lhs_part) < int(rhs_part)
return lhs_part < rhs_part

return len(rhs_parts) < len(lhs_parts)
return len(lhs_parts) < len(rhs_parts)


@dataclass
Expand Down Expand Up @@ -232,8 +299,7 @@ def get_label(self, coordinates) -> str:
def _get_label_impl(self, coordinates) -> str:
group = coordinates.group
group_label = self._labelize(group)
artifact = self._remove_scala_version_suffix(coordinates.artifact)
artifact_label = self._labelize(artifact)
artifact_label = self._labelize(coordinates.unversioned_artifact)

if group in self._SCALA_LANG_GROUPS:
return self._get_scala_lang_label(artifact_label, coordinates)
Expand All @@ -244,7 +310,7 @@ def _get_label_impl(self, coordinates) -> str:
if group in self._SCALA_PROTO_RULES_GROUPS:
return self._get_scala_proto_label(artifact_label, coordinates)

artifact_name = f'{group}:{artifact}'
artifact_name = coordinates.artifact_name

if artifact_name in self._SPECIAL_CASE_ARTIFACT_LABELS:
return self._SPECIAL_CASE_ARTIFACT_LABELS[artifact_name]
Expand All @@ -254,14 +320,6 @@ def _get_label_impl(self, coordinates) -> str:
def _labelize(s):
return s.replace('.', '_').replace('-', '_')

@staticmethod
def _remove_scala_version_suffix(artifact):
"""Removes the Scala version suffix from artifact, e.g., scopt_2.13."""
parts = artifact.split('_')
if len(parts) != 1 and parts[-1][0].isdigit():
return '_'.join(parts[:-1])
return artifact

_ARTIFACT_LABEL_ONLY_GROUPS = set([
"com.google.guava",
"com.twitter",
Expand Down Expand Up @@ -359,9 +417,9 @@ def resolve_artifacts(

for artifact in artifacts_data['dependencies']:
coords = MavenCoordinates.new(artifact['coord'])
current = current_artifacts_map.get(coords.artifact_name())
current = current_artifacts_map.get(coords.artifact_name)

if coords.artifact_name() in EXCLUDED_ARTIFACTS:
if coords.artifact_name in EXCLUDED_ARTIFACTS:
continue

if current is None or coords.is_newer_than(current.coordinates):
Expand Down Expand Up @@ -393,7 +451,7 @@ def _create_current_artifacts_map(original_artifacts):

for metadata in original_artifacts.values():
coordinates = MavenCoordinates.new(metadata['artifact'])
name = coordinates.artifact_name()
name = coordinates.artifact_name

if name not in result and metadata.get('testonly') is not True:
result[name] = ResolvedArtifact(
Expand All @@ -410,7 +468,7 @@ def _get_artifact_metadata(self, artifact) -> str:
MavenCoordinates.new(d) for d in artifact['directDependencies']
]
metadata['deps'] = [
d for d in deps if d.artifact_name() not in EXCLUDED_ARTIFACTS
d for d in deps if d.artifact_name not in EXCLUDED_ARTIFACTS
]
with open(artifact['file'], 'rb') as f:
metadata['checksum'] = hashlib.sha256(f.read()).hexdigest()
Expand Down Expand Up @@ -531,7 +589,7 @@ def _update_artifact_labels(artifacts, labeler):
for existing_label, metadata in artifacts.items():
coords = MavenCoordinates.new(metadata['artifact'])

if coords.artifact_name() in EXCLUDED_ARTIFACTS:
if coords.artifact_name in EXCLUDED_ARTIFACTS:
continue

label = (
Expand Down
7 changes: 7 additions & 0 deletions src/scala/scripts/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ scala_binary(
],
)

# Used by `scala_proto_toolchain()` to support protoc-bridge < 0.9.8,
# specifically 0.7.14 required by Scala 2.11. See #1647 and
# scalapb/ScalaPB#1771.
#
# If we drop 2.11 support, delete this target and its files, and update
# `scala_proto_toolchain()` (in `scala_proto/scala_proto_toolchain.bzl`) to use
# `scalapb.ScalaPbCodeGenerator` as its `main_generator`.
scala_library(
name = "scalapb_codegenerator_wrapper",
srcs = select_for_scala_version(
Expand Down
13 changes: 2 additions & 11 deletions src/scala/scripts/ScalaPbCodeGeneratorWrapper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,6 @@ package scripts
import protocgen.{CodeGenApp,CodeGenRequest,CodeGenResponse}

object ScalaPbCodeGenerator extends CodeGenApp {
def process(request: CodeGenRequest): CodeGenResponse = {
try {
scalapb.ScalaPbCodeGenerator.process(request)

} catch {
case e: Throwable =>
val stackStream = new java.io.ByteArrayOutputStream
e.printStackTrace(new java.io.PrintStream(stackStream))
CodeGenResponse.fail(stackStream.toString())
}
}
def process(request: CodeGenRequest): CodeGenResponse =
scalapb.ScalaPbCodeGenerator.process(request)
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ class CustomProtobufGenerator(

}


object ExtraProtobufGenerator extends ProtocCodeGenerator {
override def run(req: Array[Byte]): Array[Byte] = {
val b = CodeGeneratorResponse.newBuilder
Expand All @@ -58,7 +57,12 @@ object ExtraProtobufGenerator extends ProtocCodeGenerator {
case e: Throwable =>
// Yes, we want to catch _all_ errors and send them back to the
// requestor. Otherwise uncaught errors will cause the generator to
// die and the worker invoking it to hang.
// die and the worker invoking it to hang under protoc-bridge < 0.9.8.
// See #1647 and scalapb/ScalaPB#1771.
//
// Scala 2.11 is stuck at protoc-bridge 0.7.14. If/when we drop
// Scala 2.11 support, we can remove this `catch` block (and elide the
// `ProtobufAdapter` implementation and delete its files).
val stackStream = new java.io.ByteArrayOutputStream
e.printStackTrace(new java.io.PrintStream(stackStream))
b.setError(stackStream.toString())
Expand Down
8 changes: 4 additions & 4 deletions third_party/repositories/scala_2_12.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -815,16 +815,16 @@ artifacts = {
],
},
"scala_proto_rules_scalapb_protoc_bridge": {
"artifact": "com.thesamet.scalapb:protoc-bridge_2.12:0.9.7",
"sha256": "6d039a28d29253ac78aec0e3102f6423d269e65203c114a17f0d52a91d4876f4",
"artifact": "com.thesamet.scalapb:protoc-bridge_2.12:0.9.8",
"sha256": "4af997be5176753aa480ce40cbe9aab89ba659740a1ca6dae660afffb7bb343a",
"deps": [
"@dev_dirs_directories",
"@io_bazel_rules_scala_scala_library",
],
},
"scala_proto_rules_scalapb_protoc_gen": {
"artifact": "com.thesamet.scalapb:protoc-gen_2.12:0.9.7",
"sha256": "81df11e24e52887515dff20eb4d1a050fd58e078200291c3c87fd04218abe53b",
"artifact": "com.thesamet.scalapb:protoc-gen_2.12:0.9.8",
"sha256": "65391bf190ac9cab45674dcd8063893e4bffd4f7289742cad145962b42928648",
"deps": [
"@io_bazel_rules_scala_scala_library",
"@scala_proto_rules_scalapb_protoc_bridge",
Expand Down
8 changes: 4 additions & 4 deletions third_party/repositories/scala_2_13.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -837,16 +837,16 @@ artifacts = {
],
},
"scala_proto_rules_scalapb_protoc_bridge": {
"artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.7",
"sha256": "403f0e7223c8fd052cff0fbf977f3696c387a696a3a12d7b031d95660c7552f5",
"artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.8",
"sha256": "0b3827da2cd9bca867d6963c2a821e7eaff41f5ac3babf671c4c00408bd14a9b",
"deps": [
"@dev_dirs_directories",
"@io_bazel_rules_scala_scala_library",
],
},
"scala_proto_rules_scalapb_protoc_gen": {
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.8",
"sha256": "cf2b50721952cb4f10ca05a0ed36d7b01b88eb6505a9478556ee5a7af1a21775",
"deps": [
"@io_bazel_rules_scala_scala_library",
"@scala_proto_rules_scalapb_protoc_bridge",
Expand Down
8 changes: 4 additions & 4 deletions third_party/repositories/scala_3_1.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -887,16 +887,16 @@ artifacts = {
],
},
"scala_proto_rules_scalapb_protoc_bridge": {
"artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.7",
"sha256": "403f0e7223c8fd052cff0fbf977f3696c387a696a3a12d7b031d95660c7552f5",
"artifact": "com.thesamet.scalapb:protoc-bridge_2.13:0.9.8",
"sha256": "0b3827da2cd9bca867d6963c2a821e7eaff41f5ac3babf671c4c00408bd14a9b",
"deps": [
"@dev_dirs_directories",
"@io_bazel_rules_scala_scala_library_2",
],
},
"scala_proto_rules_scalapb_protoc_gen": {
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.7",
"sha256": "f9943ce49261aad80a063c2ce55b01fb62cfd9487ffa2d36a2eade467bc16b23",
"artifact": "com.thesamet.scalapb:protoc-gen_2.13:0.9.8",
"sha256": "cf2b50721952cb4f10ca05a0ed36d7b01b88eb6505a9478556ee5a7af1a21775",
"deps": [
"@io_bazel_rules_scala_scala_library_2",
"@scala_proto_rules_scalapb_protoc_bridge",
Expand Down
Loading

0 comments on commit 870d952

Please sign in to comment.