Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump ScalaPB, protoc-bridge, gRPC, and Guava deps, and add ProtobufAdapters and ScalaPBCodeGenerator wrappers #1648

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Commits on Nov 12, 2024

  1. Bump ScalaPB and gRPC, exclude listenablefuture

    Adds these versions to `scripts/create_repository.py`.
    
    There are breakages still that future commits will address, but the
    following are fixed in this one.
    
    Removes the unnecessary `true` parameter from `.getPlaintext(true)` in
    `test/TestServer.scala` to fix this error after the gRPC bump:
    
    ```txt
    ERROR: .../test/BUILD:676:14:
      scala @//test:lib_with_scala_proto_dep failed:
      (Exit 1): scalac failed: error executing command
      (from target //test:lib_with_scala_proto_dep)
      bazel-bin/src/java/io/bazel/rulesscala/scalac/scalac
        @bazel-out/darwin_arm64-fastbuild/bin/test/lib_with_scala_proto_dep.jar-0.params
    
    test/TestServer.scala:70: error: no arguments allowed for nullary method usePlaintext: ()?0
        val channel = ManagedChannelBuilder.forAddress(host, port).usePlaintext(true).build
                                                                                ^
    ```
    
    We exclude `com.google.guava:listenablefuture` because trying to
    download it breaks the build with an HTTP 404 (everything builds fine
    without it anyway):
    
    ```txt
    INFO: repository @io_bazel_rules_scala_listenablefuture' used the
      following cache hits instead of downloading the corresponding file.
      * Hash '...' for https://repo.maven.apache.org/maven2/com/google/guava/listenablefuture/9999.0-empty-to-avoid-conflict-with-guava/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jar
    
    WARNING: Download from
      https://repo.maven.apache.org/maven2/com/google/guava/listenablefuture/9999.0-empty-to-avoid-conflict-with-guava/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava-sources.jar
      failed: class java.io.FileNotFoundException GET returned 404 Not Found
    
    WARNING: Download from
      https://maven-central.storage-download.googleapis.com/maven2/com/google/guava/listenablefuture/9999.0-empty-to-avoid-conflict-with-guava/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava-sources.jar
      failed: class java.io.FileNotFoundException GET returned 404 Not Found
    
    WARNING: Download from
      https://mirror.bazel.build/repo1.maven.org/maven2/com/google/guava/listenablefuture/9999.0-empty-to-avoid-conflict-with-guava/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava-sources.jar
      failed: class java.io.FileNotFoundException GET returned 404 Not Found
    
    WARNING: Download from
    https://jcenter.bintray.com/com/google/guava/listenablefuture/9999.0-empty-to-avoid-conflict-with-guava/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava-sources.jar
      failed: class java.io.FileNotFoundException GET returned 404 Not Found
    
    ERROR: An error occurred during the fetch of repository 'io_bazel_rules_scala_listenablefuture':
      Traceback (most recent call last):
        File ".../scala/scala_maven_import_external.bzl",
        line 130, column 32, in _jvm_import_external
          repository_ctx.download(srcurls, srcpath, srcsha, auth = _get_auth(repository_ctx, srcurls))
    
    Error in download: java.io.IOException: Error downloading
      [ ...4 URLs from WARNINGs above... ]
    to .../external/io_bazel_rules_scala_listenablefuture/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava-src.jar:
    GET returned 404 Not Found
    
    ERROR: /Users/mbland/src/bazelbuild/rules_scala/WORKSPACE:75:25:
      fetching jvm_import_external rule
      //external:io_bazel_rules_scala_listenablefuture:
      [ ...Traceback, java.io.IOException, and HTTP 404 from above... ]
    
    ERROR: .../external/io_bazel_rules_scala_guava/BUILD:9:13:
      @io_bazel_rules_scala_guava//:io_bazel_rules_scala_guava depends on
      @io_bazel_rules_scala_listenablefuture//:io_bazel_rules_scala_listenablefuture
      in repository @io_bazel_rules_scala_listenablefuture
      which failed to fetch. no such package
      '@io_bazel_rules_scala_listenablefuture//':
         [ ...java.io.IOException and HTTP 404 from above... ]
    
    ERROR: Analysis of target
      '//test/proto/custom_generator:failing_scala_proto_deps_toolchain_def'
      failed; build aborted:
    ```
    mbland committed Nov 12, 2024
    Configuration menu
    Copy the full SHA
    521adce View commit details
    Browse the repository at this point in the history
  2. Update scala_proto repositories for Scala 2.11

    Since Scala 2.11 is stuck on ScalaPB 0.9.8, its dependencies don't
    include the `dev.dirs:directories` artifact. Before this change,
    building with `--repo_env=SCALA_VERSION=2.11.12 would die with:
    
    ```txt
    $ bazel build --repo_env=SCALA_VERSION=2.11.12 \
      //src/... //test/...  //third_party/... //scala_proto/...
    
    ERROR: Traceback (most recent call last):
      File "/Users/mbland/src/bazelbuild/rules_scala/WORKSPACE",
      line 75, column 25, in <toplevel>
        scala_proto_repositories()
    
      File "/Users/mbland/src/bazelbuild/rules_scala/scala_proto/scala_proto.bzl",
      line 17, column 37, in scala_proto_repositories
        scala_proto_default_repositories(**kwargs)
    
      File "/Users/mbland/src/bazelbuild/rules_scala/scala_proto/default/repositories.bzl",
      line 7, column 17, in scala_proto_default_repositories
        repositories(
    
      File "/Users/mbland/src/bazelbuild/rules_scala/third_party/repositories/repositories.bzl",
      line 107, column 17, in repositories
        fail("artifact %s not in third_party/repositories/scala_%s.bzl" % (
    
    Error in fail: artifact dev_dirs_directories not in
      third_party/repositories/scala_2_11.bzl
    
    ERROR: Error computing the main repository mapping:
      Encountered error while reading extension file 'go/deps.bzl':
      no such package '@io_bazel_rules_go//go':
      error loading package 'external': Could not load //external package
    ```
    
    This is a preview of scala_proto toolchainization, given the addition of
    `scala_version` and `register_toolchains` parameters to
    `scala_proto_default_repositories`.
    mbland committed Nov 12, 2024
    Configuration menu
    Copy the full SHA
    8060da1 View commit details
    Browse the repository at this point in the history
  3. Add scalapb:compilerplugin to root artifacts

    Without this, the `com.thesamet.scalapb:compilerplugin` artifact is out
    of sync with other ScalaPB artifacts, leading to this error:
    
    ```txt
    $ bazel build //test/proto_cross_repo_boundary/...
    
    ERROR: .../external/proto_cross_repo_boundary/BUILD.bazel:3:14:
      scala @proto_cross_repo_boundary//:sample_proto failed:
      (Exit 1): scalac failed: error executing command
      (from target @proto_cross_repo_boundary//:sample_proto)
    
    bazel-out/.../bin/src/java/io/bazel/rulesscala/scalac/scalac
      @bazel-out/.../bin/external/proto_cross_repo_boundary/sample_proto_scalapb.jar-0.params
    
    bazel-out/.../bin/external/proto_cross_repo_boundary/_scalac/sample_proto/sources/1_sample_proto_scala_scalapb.srcjar/sample/sample/Sample.scala:11:
      error: class Any needs to be a trait to be mixed in
        ) extends scalapb.GeneratedMessage with scalapb.Message[Sample] with scalapb.lenses.Updatable[Sample] {
                                                        ^
    Target //test/proto_cross_repo_boundary:sample_scala_proto failed to build
    ```
    mbland committed Nov 12, 2024
    Configuration menu
    Copy the full SHA
    01ac301 View commit details
    Browse the repository at this point in the history
  4. Fix scalapb-runtime 0.11.17 errors

    There were API changes from scalapb-runtime 0.9.7 causing the following
    errors:
    
    ```txt
    ERROR: .../test/src/main/scala/scalarules/test/extra_protobuf_generator/BUILD:3:14:
      scala @//test/src/main/scala/scalarules/test/extra_protobuf_generator:extra_protobuf_generator
      failed: (Exit 1): scalac failed: error executing Scalac command
      (from target //test/src/main/scala/scalarules/test/extra_protobuf_generator:extra_protobuf_generator)
      bazel-out/darwin_arm64-opt-exec-ST-a828a81199fe/bin/src/java/io/bazel/rulesscala/scalac/scalac
        ... (remaining 1 argument skipped)
    
    test/src/main/scala/scalarules/test/extra_protobuf_generator/ExtraProtobufGenerator.scala:20:
      error: value nameSymbol is not a member of com.google.protobuf.Descriptors.Descriptor
          .add(s"final case object Custom${message.nameSymbol}{}")
                                                   ^
    test/src/main/scala/scalarules/test/extra_protobuf_generator/ExtraProtobufGenerator.scala:34:
      error: value fileDescriptorObjectName is not a member of com.google.protobuf.Descriptors.FileDescriptor
        b.setName(file.scalaDirectory + "/Custom" + file.fileDescriptorObjectName + ".scala")
                                                         ^
    test/src/main/scala/scalarules/test/extra_protobuf_generator/ExtraProtobufGenerator.scala:66:
      error: value FileDescriptorPimp is not a member of scalapb.compiler.DescriptorImplicits
              import implicits.FileDescriptorPimp
                     ^
    ```
    
    Since Scala 2.11 can't advance past scalapb-runtime 0.9.8, we use
    `select_for_scala_version` to select the appropriate `ProtobufAdapter`
    to maintain API compatibility.
    
    Also, `import implicits.FileDescriptorPimp` turned out to be unnecessary
    after all, even under Scala 2.11.
    mbland committed Nov 12, 2024
    Configuration menu
    Copy the full SHA
    876a594 View commit details
    Browse the repository at this point in the history
  5. Add null as dummy TestMessage parameter

    Fixes the `test_demonstrate_INCORRECT_scala_proto_library_stamp` test
    case from `test/shell/test_strict_dependency.sh` after the ScalaPB
    0.11.17 bump.
    
    Somehow, bumping that library (presumably, instead of gRPC, et. al.)
    changed the parameter list of `TestMessage` from `String` to
    `String,UnknownFieldSet`. The test then failed because the following
    command produced this unexpected error:
    
    ```txt
    $ bazel build --verbose_failures \
      //test_expect_failure/missing_direct_deps/scala_proto_deps:uses_transitive_scala_proto \
      --extra_toolchains=//test/toolchains:ast_plus_one_deps_strict_deps_error
    
    ERROR: .../test_expect_failure/missing_direct_deps/scala_proto_deps/BUILD:25:13:
      Building test_expect_failure/missing_direct_deps/scala_proto_deps/libtransitive.jar
      (1 source file) failed: (Exit 1): java failed: error executing command
      (from target //test_expect_failure/missing_direct_deps/scala_proto_deps:transitive)
    
      (cd ...
        [ ...snip... ]
      external/remotejdk11_macos_aarch64/bin/java -XX:-CompactStrings
        [ ...snip... ]
      external/remote_java_tools/java_tools/JavaBuilder_deploy.jar
      @bazel-out/.../bin/test_expect_failure/missing_direct_deps/scala_proto_deps/libtransitive.jar-0.params
      @bazel-out/.../bin/test_expect_failure/missing_direct_deps/scala_proto_deps/libtransitive.jar-1.params)
    
    test_expect_failure/missing_direct_deps/scala_proto_deps/UseTestMessage.java:7:
      error: constructor TestMessage in class TestMessage
      cannot be applied to given types;
    
      private final TestMessage m = new TestMessage("");
                                    ^
      required: String,UnknownFieldSet
      found: String
      reason: actual and formal argument lists differ in length
    ```
    
    After this change, the same command produces the following failure that
    the test is expecting to catch:
    
    ```txt
    ERROR: .../test_expect_failure/missing_direct_deps/scala_proto_deps/BUILD:18:14:
      scala @//test_expect_failure/missing_direct_deps/scala_proto_deps:uses_transitive_scala_proto
      failed: (Exit 1): scalac failed: error executing command
      (from target //test_expect_failure/missing_direct_deps/scala_proto_deps:uses_transitive_scala_proto)
    
      (cd .../execroot/io_bazel_rules_scala && \
      exec env - \
      bazel-out/.../bin/src/java/io/bazel/rulesscala/scalac/scalac
      @bazel-out/.../bin/test_expect_failure/missing_direct_deps/scala_proto_deps/uses_transitive_scala_proto.jar-0.params)
    
    test_expect_failure/missing_direct_deps/scala_proto_deps/UseScalaProtoIndirectly.scala:6:
      error: Target '@//test_expect_failure/missing_direct_deps/scala_proto_deps:some_proto'
      is used but isn't explicitly declared, please add it to the deps.
    
    You can use the following buildozer command:
      buildozer 'add deps
        @//test_expect_failure/missing_direct_deps/scala_proto_deps:some_proto'
        //test_expect_failure/missing_direct_deps/scala_proto_deps:uses_transitive_scala_proto
    
      val foo: TestMessage = new UseTestMessage().getM
               ^
    one error found
    Build failure with errors.
    Target //test_expect_failure/missing_direct_deps/scala_proto_deps:uses_transitive_scala_proto
      failed to build
    ```
    mbland committed Nov 12, 2024
    Configuration menu
    Copy the full SHA
    c61501d View commit details
    Browse the repository at this point in the history
  6. Remove unnecessary .getPlaintext() parameter

    Already did this for `test/TestServer.scala` as part of the gRPC bump.
    `test_version/version_specific_tests_dir/TestServer.scala` gets the same
    treatment now to fix `test_scala_version 2.11.12` from
    `test_version.sh`, which failed with:
    
    ```
    ERROR: .../test_version/test_scala_version_1731027515/BUILD:161:14:
      scala @//:lib_with_scala_proto_dep failed: (Exit 1): scalac failed:
      error executing command (from target //:lib_with_scala_proto_dep)
    
    bazel-out/.../bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac
      @bazel-out/.../bin/lib_with_scala_proto_dep.jar-0.params
    
    TestServer.scala:70: error: too many arguments for method usePlaintext: ()?0
        val channel = ManagedChannelBuilder.forAddress(host, port).usePlaintext(true).build
                                                                               ^
    ```
    mbland committed Nov 12, 2024
    Configuration menu
    Copy the full SHA
    f25a555 View commit details
    Browse the repository at this point in the history
  7. Add org_scala_lang_modules_scala_collection_compat

    More specifically, add it to the list of core scala repository imports.
    It had been generated before, but somehow hadn't been required until
    now.
    
    With this change, all tests now pass after the ScalaPB 0.11.17, gRPC
    1.68.1, and Guava 33.3.1-jre version bumps.
    
    Fixes the following error in `test_scala_version 2.12.20` from
    `test_version.sh`:
    
    ```txt
    ERROR: .../external/scala_proto_rules_scalapb_compilerplugin/BUILD:9:13:
      no such package '@org_scala_lang_modules_scala_collection_compat//':
    
      The repository '@org_scala_lang_modules_scala_collection_compat'
      could not be resolved:
    
      Repository '@org_scala_lang_modules_scala_collection_compat'
      is not defined and referenced by
      '@scala_proto_rules_scalapb_compilerplugin//:scala_proto_rules_scalapb_compilerplugin'
    
    ERROR: Analysis of target '//proto:test_proto_nogrpc' failed; build aborted:
    ```
    mbland committed Nov 12, 2024
    Configuration menu
    Copy the full SHA
    5be9951 View commit details
    Browse the repository at this point in the history
  8. Wrap ScalaPbCodeGenerator, expose Scala 2.11 error

    Wraps `scalapb.ScalaPbCodeGenerator` to catch and return all errors.
    Without this, any errors escaping from `scalapb.ScalaPbCodeGenerator`
    will cause the `scala_proto` aspect workers to hang.
    
    Also deletes some `@io_bazel_rules_scala` references, replacing them
    with `Label` instances as appropriate.
    
    The `test_scala_version 2.11.12` case from `test_version.sh` would
    previously hang given the combination of ScalaPB 0.9.8 (the newest
    available for Scala 2.11) and Protobuf v28.3. Running the following in a
    `test_version/test_scala_version_*` repo generated by `test_version.sh`
    reproduced the hang:
    
    ```txt
    $ bazel build --repo_env=SCALA_VERSION=2.11.12 //proto:test_proto
    
    [ ...wait 10 seconds, then CTRL-C... ]
    
    INFO: Analyzed target //proto:test_proto (2 packages loaded, 22 targets configured).
    INFO: Found 1 target...
    [1,038 / 1,047] 4 actions running
        ProtoScalaPBRule proto/test_service_scala_scalapb.srcjar; 10s worker
        ProtoScalaPBRule proto/test2_scala_scalapb.srcjar; 10s worker
    Target //proto:test_proto failed to build
    ```
    
    With this change, the command now exposes the exact incompatibility
    between these versions:
    
    ```txt
    $ bazel build --repo_env=SCALA_VERSION=2.11.12 //proto:test_proto
    
    ERROR: .../test_version/test_scala_version_1731169107/proto/BUILD:14:14:
      ProtoScalaPBRule proto/test3_scala_scalapb.srcjar failed: (Exit 1):
      scalapb_worker failed: error executing command
      (from target //proto:test3)
    
    bazel-out/.../bin/external/io_bazel_rules_scala/src/scala/scripts/scalapb_worker
      ... (remaining 2 arguments skipped)
    
    --scala_out: java.lang.NoSuchMethodError:
      'void com.google.protobuf.Descriptors$FileDescriptor.internalBuildGeneratedFileFrom(java.lang.String[],
        com.google.protobuf.Descriptors$FileDescriptor[],
        com.google.protobuf.Descriptors$FileDescriptor$InternalDescriptorAssigner)'
          at scalapb.options.compiler.Scalapb.<clinit>(Scalapb.java:10592)
          at scalapb.ScalaPbCodeGenerator$.run(ScalaPbCodeGenerator.scala:14)
          at scalapb.ScalaPbCodeGenerator$.run(ScalaPbCodeGenerator.scala:10)
          at scripts.ScalaPbCodeGenerator$.run(ScalaPbCodeGeneratorWrapper_2_11.scala:8)
          at protocbridge.frontend.PluginFrontend$$anonfun$runWithBytes$2.apply(PluginFrontend.scala:52)
          at protocbridge.frontend.PluginFrontend$$anonfun$runWithBytes$2.apply(PluginFrontend.scala:52)
          at scala.util.Try$.apply(Try.scala:192)
          at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:51)
          at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:103)
          at protocbridge.frontend.PosixPluginFrontend$$anonfun$prepare$1.apply$mcV$sp(PosixPluginFrontend.scala:33)
          at protocbridge.frontend.PosixPluginFrontend$$anonfun$prepare$1.apply(PosixPluginFrontend.scala:31)
          at protocbridge.frontend.PosixPluginFrontend$$anonfun$prepare$1.apply(PosixPluginFrontend.scala:31)
          at scala.concurrent.impl.Future$PromiseCompletingRunnable.liftedTree1$1(Future.scala:24)
          at scala.concurrent.impl.Future$PromiseCompletingRunnable.run(Future.scala:24)
          at scala.concurrent.impl.ExecutionContextImpl$AdaptedForkJoinTask.exec(ExecutionContextImpl.scala:121)
          at scala.concurrent.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260)
          at scala.concurrent.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339)
          at scala.concurrent.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979)
          at scala.concurrent.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107)
    
    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)
    
    Target //proto:test_proto failed to build
    ```
    mbland committed Nov 12, 2024
    Configuration menu
    Copy the full SHA
    de8214b View commit details
    Browse the repository at this point in the history

Commits on Nov 17, 2024

  1. Extract scala_proto_artifact_ids

    This will support the upcoming schema whereby `scala_toolchains` will
    import all artifact IDs via macros like this, and invoke `repositories`
    itself. This will avoid instantiating the same artifact repository
    multiple times for different frameworks that depend on it.
    
    Under `WORKSPACE`, this isn't a problem, but it's an error under Bzlmod,
    as a module extension can only instantiate a repository once. The commit
    message to toolchainize Scalafmt will provide more details on this.
    mbland committed Nov 17, 2024
    Configuration menu
    Copy the full SHA
    e8d1f8b View commit details
    Browse the repository at this point in the history