bazelbuild / rules_scala

Scala rules for Bazel
Apache License 2.0
360 stars 272 forks source link

Visibility issue after upgrading from 6.3.0 to 6.5.0 #1590

Closed kmate-ct closed 4 weeks ago

kmate-ct commented 1 month ago

I'm unable to upgrade from 6.3.0 to 6.5.0... I get this error:

ERROR: .../external/io_bazel_rules_scala/scala/BUILD:39:22: in toolchain rule @io_bazel_rules_scala//scala:default_toolchain: target '@io_bazel_rules_scala_config//:scala_version_2_12_18' is not visible from target '@io_bazel_rules_scala//scala:default_toolchain'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: .../external/io_bazel_rules_scala/scala/BUILD:39:22: Analysis of target '@io_bazel_rules_scala//scala:default_toolchain' failed
ERROR: .../BUILD.bazel:6:14: While resolving toolchains for target //...: invalid registered toolchain '@io_bazel_rules_scala//scala:default_toolchain':

(I replaced some sensitive paths / target names with "...", but that should not matter)

Any ideas what am I doing wrong? I did not change my setup, only bumped the version.

kmate-ct commented 1 month ago

I see that the generated target is missing visibility

config_setting(
    name = "scala_version_2_12_18",
    flag_values = {":scala_version": "2.12.18"},
)

Is this intentional?

Also, if I force it to be visible, it doesn't get much better

external/io_bazel_rules_scala/scala/private/rule_impls.bzl", line 231, column 61, in allow_security_manager
        return ["-[Djava.security](http://djava.security/).manager=allow"] if java_runtime.version >= 17 else []

Error: 'JavaRuntimeInfo' value has no field or method 'version'

still using bazel 5.3.1, unfortunately... did the minimum bazel version change?

Should we just change rules_scala to use hasattr before calling .version or something?

https://github.com/bazelbuild/rules_scala/blob/master/scala/private/rule_impls.bzl#L231

kmate-ct commented 1 month ago

I'm not sure if this is the right fix but it "makes it work"

return ["-Djava.security.manager=allow"] if hasattr(java_runtime, "version") and java_runtime.version >= 17 else []
johnynek commented 1 month ago

I guess if you are an a bazel that old you aren't also on a new jdk, so I think that fix should be fine if you send as a PR.

kmate-ct commented 1 month ago

Thanks, I will do so then. The original question was about the visibility of config_setting. Should I open a PR for that as well, or it might be something broken in my setup?

kmate-ct commented 1 month ago

I opened both PRs, we'll see I guess.

simuons commented 4 weeks ago

@kmate-ct thanks for the issue and PRs (both are merged)

kmate-ct commented 4 weeks ago

Thank you! Do you have any plan on the next release's date?

simuons commented 4 weeks ago

Hi @kmate-ct, made new release https://github.com/bazelbuild/rules_scala/releases/tag/v6.6.0

kmate-ct commented 4 weeks ago

Thank you so much!