rules_scala icon indicating copy to clipboard operation
rules_scala copied to clipboard

[Bzlmod][Twitter Scrooge] Creating a custom `twitter_scrooge` toolchain requires significant workarounds

Open FrankPortman opened this issue 5 months ago • 6 comments
trafficstars

From what I understand, there are 2 (independent? complementary?) paths to create a custom twitter_scrooge toolchain.

You can either invoke setup_scrooge_toolchain or use the tag class a la scala_deps.twitter_scrooge(libthrift = ..., ....).

In the former case, supplying an override to every param in the constructor leads to this error, due to implicit non overrideable labels being summoned deeper in the macro:

no such package '@@[unknown repo 'io_bazel_rules_scala_scopt_2_13_16' requested from @@]//': The repository '@@[unknown repo 'io_bazel_rules_scala_scopt_2_13_16' requested from @@]' could not be resolved: No repository visible as '@io_bazel_rules_scala_scopt_2_13_16' from main repository and referenced by '//tools/scala:compiler_classpath_provider'

You can work around this via something like:

scala_deps = use_extension("//scala/extensions:deps.bzl", "scala_deps")
scala_deps.scala()
scala_deps.twitter_scrooge()

SCALA_VERSION="2.13.16"
version_suffix = "_%s" % SCALA_VERSION.replace(".", "_")
use_repo(
    scala_deps,
    "io_bazel_rules_scala_javax_annotation_api" + version_suffix,
    "io_bazel_rules_scala_mustache" + version_suffix,
    ... other non overrideable deps ...
)  

but this is not obvious, and requires you to invoke scala_deps.scala() even if you had successfully obviated that call previously via a custom Scala toolchain.

In the latter case (using the tag class and providing overrides), we hit a label vs string issue:

@@rules_scala+//scala/extensions:rules_scala++scala_deps+rules_scala_toolchains: expected value of type 'string' for dict value element, but got Label("@@rules_jvm_external++maven+maven//:org_apache_thrift_libthrift") (Label)
ERROR: /private/var/tmp/_bazel_fp/2927ef64b11207240f16497f643a99d5/external/rules_scala+/scala/toolchains.bzl:228:26: Traceback (most recent call last):
	File "/private/var/tmp/_bazel_fp/2927ef64b11207240f16497f643a99d5/external/rules_scala+/scala/extensions/deps.bzl", line 227, column 21, in _scala_deps_impl
		scala_toolchains(
	File "/private/var/tmp/_bazel_fp/2927ef64b11207240f16497f643a99d5/external/rules_scala+/scala/toolchains.bzl", line 228, column 26, in scala_toolchains
		scala_toolchains_repo(
Error in repository_rule: failed to instantiate 'scala_toolchains_repo' from this module extension

In an ideal world, the preferred route of creating a custom twitter_scrooge toolchain - or pros/cons of either route - is well documented, and all overrides are self contained in one call.

FrankPortman avatar May 27 '25 00:05 FrankPortman

Slack context: https://bazelbuild.slack.com/archives/CDCKJ2KFZ/p1748133814638949

FrankPortman avatar May 27 '25 00:05 FrankPortman

@simuons Feel free to assign this one to me, too. Looks like we need to make all the setup_scrooge_toolchain deps overridable (and add those fields to scala_deps.twitter_scrooge(), too.)

Plus, I need to restore a Label in setup_scrooge_toolchain, too (after adding Label instances to fix setup_scala_toolchains and scala_cross_version_select in #1742).

As a side note, I think we could release 7.0.1 once #1745 lands. Then I can hopefully get to this issue and #1743 this week, and we can release 7.02. Or, if you prefer, you can wait for me to get to this and #1743 first, and then release 7.0.1. What do you think?

mbland avatar May 27 '25 02:05 mbland

Friendly bump here - I am happy (and think I see a path) to patch this for my company with something that gets the job done but is probably not what you'd want landing in rules_scala. So I just wanted to check in quickly to see if anybody was working on a more robust solution.

FrankPortman avatar Jun 01 '25 22:06 FrankPortman

@FrankPortman Sorry I didn't get to it last week as I'd hoped. But I'm also hoping #1740 and #1745 can land first, then this particular change should prove quick and easy. (Famous last words...)

I don't anticipate #1743 being that difficult, either, but this would be the quicker of the two to address.

mbland avatar Jun 03 '25 01:06 mbland

I have a test that reproduced both problems and fixes for both problems in my fix-1744-setup-scrooge-toolchain branch. I'll tidy up the commits, add a documentation file, and open a pull request some time tomorrow.

mbland avatar Jun 12 '25 03:06 mbland

@FrankPortman I just opened #1747. Let me know what you think of docs/twitter_scrooge.md and the comments in examples/twitter_scrooge/{BUILD,MODULE.bazel,WORKSPACE}. I'm happy to edit them to make them more clear and useful.

As for proper twitter_scrooge rules usage, that may be something you can contribute in a follow-up PR, as I'm not that knowledgable about it.

mbland avatar Jun 12 '25 18:06 mbland

This looks good to me, and I'm open to creating some followup examples, but I think I am still not understanding when to choose between:

"Builtin toolchain dependency overrides"

and

"Defining a custom toolchain"

--

Are those two ways of achieving the same thing or are there situations when we would prefer one over the other?

FrankPortman avatar Jun 19 '25 17:06 FrankPortman

This looks good to me, and I'm open to creating some followup examples, but I think I am still not understanding when to choose between:

"Builtin toolchain dependency overrides"

and

"Defining a custom toolchain"

--

Are those two ways of achieving the same thing or are there situations when we would prefer one over the other?

Yeah, thanks for calling this out. I think yes, you can largely accomplish most of the same things with either option. I think "Builtin toolchain dependency overrides" are for when:

  • You don't want to call setup_scrooge_toolchain in your own BUILD file, so you want to rely on the builtin toolchain, but...
  • you definitely still want to customize some of the JAR dependencies.

"Defining a custom toolchain" is for when

  • You want to call setup_scrooge_toolchain in your own BUILD file, to avoid relying on the builtin toolchain, and...
  • you want to supply potentially all of the dependency JARs, but...
  • if you still want to rely on some builtins, you'd need to instantiate the builtin toolchain, even if you don't use it.

@FrankPortman Let me know if that makes sense, or if you have any other questions or possibly insights to add.

@simuons @liucijus If this understanding is correct, I can update the documentation in #1747 accordingly.

mbland avatar Jun 19 '25 18:06 mbland

That distinction makes sense, I just get nervous in Bazel land when there appear to be 2 ways to do similar things given how arcane a lot of the inner workings of build rules are to most devs (including me, in many cases)

FrankPortman avatar Jun 23 '25 14:06 FrankPortman