bazelbuild / rules_scala

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

Provide `select_for_scala_version` utility macro #1563

Closed aszady closed 5 months ago

aszady commented 6 months ago

Description

For later use in BUILD files of rules_scala where customization depending on Scala version is needed. End users are also encouraged to use this one.

The approach in this PR differs from the one in #1552, as with selects we will avoid the need of multiplying targets within rules_scala for each Scala version configured. This comes at a cost of not so easy access to the internal binaries for a specific, non-default version. #1552 proposed suffixing target names with the Scala version, whereas here the target built depends on a configuration – harder to specify via command-line. Possible workaround for this, if ever needed, would be to use a flag (see: https://github.com/bazelbuild/rules_scala/pull/1558#discussion_r1539128621).

The choice of matchers is quite arbitrary, but covers the usage required at the moment.

See usage in #1564.

Motivation

Originally #1290.

simuons commented 5 months ago

Hi, @aszady, would it be possible to write some tests for this macro?

I'm not sure about exposing this macro for end users right now. I mean such sophisticated matchers might be needed internally to support wide range of scala versions. But end users (presumably) would have only handful set of scala versions and could write regular selects.

Could you make this bzl private for now?

I think maybe what's missing is more coarse grained config_settings for scala version at @io_bazel_rules_scala_config ie:

That would allow to have more stable selects. wdyt?

aszady commented 5 months ago

would it be possible to write some tests for this macro?

@simuons, do you have any specific idea in mind for such tests? I think we cannot test the macro itself, as we haven't enabled multiple versions yet, so such test would be quite trivial. Other option would be to writ unit tests for some implementation details, like _matches_for_version – not sure if satisfactory? Imo the best would be to set up a separate test repository, e.g. test_cross_build and do the full analysis-phase test there. But that's also something we can't do yet, without multiple versions enabled.

I'm not sure about exposing this macro for end users right now.

Certainly it may seem that some matchers are here just for internal support, with most striking example of between…and… to use for after_2_12_13_and_before_2_13_12-specific code. My reasoning for exposing this macro was that it expresses closer the intention and could make the maintenance easier.

With the macro we say: this will/won't work for such Scala versions and we usually can state which Scala version caused the change. This information is irrelevant to currently enabled scala_versions and therefore ideally needs no updates when adding extra versions (easier Scala version change). With the pure select only: the original intention may get intermixed with the current workspace setup.

The coarser-grained Scala version settings is something in between, as it emulates the any matcher – potentially the most needed one. Though in this case again it may require extra work (per each select()) when Scala versions get added or removed. On the other hand I can imagine that most of the customization needs would revolve around Scala 2 vs. 3 and more precise selections would be quite rare and we can neglect them when estimating impact of this change.