rules_scala icon indicating copy to clipboard operation
rules_scala copied to clipboard

`_suite`-like rules don't perform unused dependency checks

Open stefanobaghino-da opened this issue 3 years ago • 3 comments
trafficstars

I recently realized that scala_test_suite doesn't perform unused dependency checks.

I had a look at the code and indeed this is explicitly turned off for both scala_test_suite and scala_library_suite, with this behavior tracing back to https://github.com/bazelbuild/rules_scala/pull/579.

https://github.com/bazelbuild/rules_scala/blob/b85d1225d0ddc9c376963eb0be86d9d546f25a4a/scala/private/rules/scala_library.bzl#L107-L130

https://github.com/bazelbuild/rules_scala/blob/f782615e8f87b136045a0c5bcd4dbd7a304a4359/scala/private/rules/scala_test.bzl#L125-L146

If my intuition is correct, I suspect the problem is that the dependencies are defined for the entire suite but unused dependency checks is performed for each scala_* rule defined as part of scala_*_suite, which means that if any of the "sub-targets" don't use any dependency of the overall target, this will make the check fail. Is this a correct assessment of why _suite-like rules have turned unused dependency checks off explicitly?

Right now to perform unused dependency checks we use the following workaround:

  • temporarily turn a rule from scala_test_suite to scala_test
  • build the target
  • clean up any unused dependency
  • revert the rule back into a scala_test_suite

This is done sporadically and is completely manual, so far from being ideal, but I guess it's good enough and we (probably) don't have too many unused dependencies, but I was wondering whether there is a better way to perform unused dependency checks on _suite-like targets, either manually or perhaps to introduce it automatically with a PR.

stefanobaghino-da avatar Feb 01 '22 10:02 stefanobaghino-da

@stefanobaghino-da maybe you can wrap scala_test_suite with into another one which would also create scala_library with dep checking on? It's not optimal solution, but could work. Maybe this target can be marked manual and then built on demand.

liucijus avatar Feb 02 '22 19:02 liucijus

@stefanobaghino-da maybe you can wrap scala_test_suite with into another one which would also create scala_library with dep checking on? It's not optimal solution, but could work. Maybe this target can be marked manual and then built on demand.

Thanks for the answer and sorry for the delayed reply.

This could catch unused dependencies for existing targets, but we would have to make sure new targets follow this pattern too. I appreciate your suggestion as it has advantages over our current approach, but do you think it would be somehow feasible to find a way to perform the check automatically? I'm happy to contribute but I'd need some guidance as what a good approach could be (if feasible at all), my understanding of Bazel is unfortunately a bit shallow as I'm mostly a user.

stefanobaghino-da avatar Feb 10 '22 07:02 stefanobaghino-da

The challenge here is that existing dependency tracking is done per target, or to be more precise, per Scala compiler invocation. I don't know if it's possible to make it work efficiently with suite-like macros, but there are possible directions for research:

  • rearrange existing macros so that they can do dep tracking within single target
  • see if current dep tracking mechanism can be extended/improved to work between group of targets. Maybe there could be a special (test?) rule, which knows how to collect used/unused deps information from multiple targets and aggregate into a proper report.

liucijus avatar Feb 10 '22 15:02 liucijus