bazelbuild / rules_scala

Scala rules for Bazel
Apache License 2.0
363 stars 277 forks source link

Target pattern attributes on `scala_toolchain` are erroneously checked #1420

Open Wyverald opened 2 years ago

Wyverald commented 2 years ago

scala_toolchain has two attributes that specify target patterns: https://github.com/bazelbuild/rules_scala/blob/17791a18aa966cdf2babb004822e6c70a7decc76/scala/scala_toolchain.bzl#L120-L127

These are implemented as string lists, and directly used to compare with a stringified label's prefix: https://github.com/bazelbuild/rules_scala/blob/b85d1225d0ddc9c376963eb0be86d9d546f25a4a/scala/private/phases/phase_dependency.bzl#L82-L91

This is very brittle and often doesn't work in practice. Breaking scenarios include:

A proper way to fix this would be to parse the target patterns, and make use of ctx.label.relative to relativize the input (example: https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl;l=622-626;drc=f6a99afbd61c965b6e6888239ae432c4bb9a564a)

An immediate breakage that blocks my work on https://github.com/bazelbuild/bazel/issues/16196 is that labels in the main repo get stringified with a starting '@', which means that @//foo:bar doesn't even satisfy //foo anymore. To work around this, we should disable the test https://github.com/bazelbuild/rules_scala/blob/b85d1225d0ddc9c376963eb0be86d9d546f25a4a/test/shell/test_strict_dependency.sh#L57

liucijus commented 2 years ago

@Wyverald is there a backwards compatible way to turn off incompatible_unambiguous_label_stringification on older Bazel versions?

Wyverald commented 2 years ago

Not that I know of. It does seem a reasonable request to want to put something in your .bazelrc file and have it work for older versions too. I'll ask around in the team.