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

Logspew when building #1364

Open
ptarjan opened this issue Mar 10, 2022 · 7 comments
Open

Logspew when building #1364

ptarjan opened this issue Mar 10, 2022 · 7 comments

Comments

@ptarjan
Copy link
Contributor

ptarjan commented Mar 10, 2022

bazel commands shouldn't print anything on success. Can you suppress or fix these warnings please?

    # HEAD from 2021-11-29
    RULES_SCALA_VERSION = "1d98a8013853b73c825c727be9467e5ea15cc262"
    RULES_SCALA_SHA = "50963bf5d38dab524bffe941bf5fd68c06de6e6ee75e6b12a8610de7ca409880"
    http_archive(
        name = "io_bazel_rules_scala",
        sha256 = RULES_SCALA_SHA,
        strip_prefix = "rules_scala-%s" % RULES_SCALA_VERSION,
        type = "zip",
        url = "https://github.com/bazelbuild/rules_scala/archive/%s.zip" % RULES_SCALA_VERSION,
    )
$ bazel build @io_bazel_rules_scala//src/scala/scripts:scalapb_worker_lib
Starting local Bazel server and connecting to it...
INFO: Invocation ID: 8cd07936-f07d-4a08-a1a4-82d7dbe93bc1
INFO: Analyzed target @io_bazel_rules_scala//src/scala/scripts:scalapb_worker_lib (249 packages loaded, 6590 targets configured).
INFO: Found 1 target...
INFO: From scala @io_bazel_rules_scala//src/scala/scripts:scalapb_worker_lib:
warning: one deprecation
warning: one deprecation (since 0.9.0)
warning: two deprecations in total; re-run with -deprecation for details
three warnings found
Target @io_bazel_rules_scala//src/scala/scripts:scalapb_worker_lib up-to-date:
  bazel-bin/external/io_bazel_rules_scala/src/scala/scripts/scalapb_worker_lib.jar
INFO: Elapsed time: 36.317s, Critical Path: 8.48s
INFO: 26 processes: 2 internal, 21 darwin-sandbox, 3 worker.
INFO: Build completed successfully, 26 total actions
@johnynek
Copy link
Member

Hi @ptarjan, long time!

I am 95% sure that all but:

INFO: From scala @io_bazel_rules_scala//src/scala/scripts:scalapb_worker_lib:
warning: one deprecation
warning: one deprecation (since 0.9.0)
warning: two deprecations in total; re-run with -deprecation for details
three warnings found

are due to bazel and not related to these rules.

But the warning on deprecation should indeed not just print to screen I think.

That said if you want to be unblocked scala 2.12 and 2.13 allows configuring warnings:

scala/scala#8373

so, you silence on your end by passing the right args to the compiler in the toolchain setup I believe. If you are using scala 2.11 you can do more work (turn on a compiler plugin):

https://github.com/ghik/silencer

I agree that the "right" solution is for that not to be sent back, but I'm not 100% sure it will be easy. Maybe we just throw away the compiler output, or make it one of the build outputs, in the success case, but in the failure case send all the stdout and stderr back to bazel? I haven't thought too much about it, just thinking off the cuff now.

(note: I handed over maintenance of these rules to others 2 years ago, I'm just answering to try to be helpful since I recognized your name).

@johnynek
Copy link
Member

@ptarjan
Copy link
Contributor Author

ptarjan commented Mar 16, 2022

@johnynek Thanks Oscar. Indeed, long time, hope you're doing well. Thanks for answering, it was lovely to see your name again. I'm happy to wait to see if the current maintainer can help me out here.

I'll look at passing those command flags to scala. It does seem to be coming from the rules compiling some library for themselves, so it might be harder to inject flags but I'll try.

Your suggestion is exactly the approach they went with in rules_nodejs. Silent on success (as an optional flag with some debate to make it on by default at a later date): bazelbuild/rules_nodejs#3336

@simuons
Copy link
Collaborator

simuons commented Mar 17, 2022

Hi @ptarjan

registered scala_toolchain is used to compile @io_bazel_rules_scala//src/scala/scripts:scalapb_worker_lib so you could setup scalacopts to suppress these deprecation warnings. Ideally there should be no warnings from internal targets. Which version of scalapb you are using (if you override that)? I think rules_scala uses quite old one. I'll update scalapb and cleanup warning or suppress them on target level.

@virusdave
Copy link
Contributor

We do use a custom scala_toolchain, but only to specify specific attributes (transitive deps, no unused deps warnings, etc). We also use a custom scala_proto_toolchain which uses ZioCodeGenerator as an extra scalapb generator.

I'l update the rules to current master after the ScalaPBWorker deprecation commit and make sure that we're not seeing any more rules-related log spam; if we are, will report back.

@virusdave
Copy link
Contributor

Which version of scalapb you are using (if you override that)? I think rules_scala uses quite old one. I'll update scalapb and cleanup warning or suppress them on target level.

We currently use 0.11.5 of scalapb, which causes a deprecation warning because it updates the interface used by ScalapbWorker and deprecates the method currently called. I'm solving this currently by patching rules_scala with a 0.11.5-version specific patch to bring everything back into harmony again. If there's willingness to globally update the scalapb version, please let me know and i'll submit a change that does this (unless you specifically wish to do so).

@virusdave
Copy link
Contributor

The other source of deprecation-related log spam is from the scalapb aspect compilation itself. Scalapb creates files which simultaneously "produce code marked @deprecated" and "uses that code", which causes scalac to emit warnings. However, the warnings should really come from downstream users of those deprecated fields, not from proto-internal code which uses it.

Alas, there's not a way to add silencing lint suppression via scalacopts to the aspect currently. Thoughts on either making this cusomizable, or outright suppressing deprecation warnings here directly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants