rules_scala icon indicating copy to clipboard operation
rules_scala copied to clipboard

Target pattern attributes on `scala_toolchain` are erroneously checked

Open Wyverald opened this issue 3 years ago • 2 comments
trafficstars

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:

  • The target //foo:bar_baz erroneously satisfies the target pattern //foo:bar
  • If @foo uses rules_scala, and specifies a target pattern //bar, then while building inside @foo as the main repository, //bar:baz satisfies the target pattern; but if @foo is being used as someone else's dependency, the stringified label would be @foo//bar:baz, which no longer satisfies the specified target pattern

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

Wyverald avatar Sep 13 '22 14:09 Wyverald

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

liucijus avatar Sep 19 '22 13:09 liucijus

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.

Wyverald avatar Sep 19 '22 15:09 Wyverald