Use java_import rules instead of exports_files to reference jars.

[chaoren]

This allows aspects on scala rules to propagate and pick up the jars.
And makes it easy for other tools to recognize the jars from
arbitrary files.

[bazel-io]

Can one of the admins verify this patch?

[johnynek]

test this please

[johnynek]

This is going to break people who (like us at Stripe) are wiring in their own scala sdk.

Can you outline how this is an improvement? Is it the case that this fixed some bug for you? Can we add a test to demonstrate that bug?

We really need to solve #170 to make it clear how to configure scala.

[chaoren]

test this please

The current test suite passes. These jars are used in every rule. It should be covered by the current test suite.

This is going to break people who (like us at Stripe) are wiring in their own scala sdk.

Please elaborate. I don't see how.

Can you outline how this is an improvement? Is it the case that this fixed some bug for you?

I'm working on an IDE plugin for scala with Bazel, and I'm using an aspect to gather up all the scala rules, their dependencies, and everything needed to resolve the source files. That includes these jars. attr_aspects doesn't propagate through entries of exports_files (probably because they're not actual rules), but does for filegroup.

Can we add a test to demonstrate that bug?

I guess you can check in an aspect with attr_aspects = ["_scalalib"], print out all the targets processed, and see that the scalalib is missing.

[johnynek]

(sorry, "test this please" was a message to the the ci bot).

The way this breaks users with a custom sdk is that they do not call scala_repositories since that fixes a bunch of jars. Instead, they manually replace things at the labels that the rules expect. I want to change this so that the rules expects bind names (though there are some folks that want to remove bind and instead have everyone standardize on external names for jars, which I worry is not actually practical outside of monolithic orgs).

So, we will have not added this change that you have, and when we bump the git-sha we will just see a failure of the build (since it won't find the filegroups you have added). This isn't a showstopper, I'm just saying that this is a cost of migration for each user with their own sdk configuration.

It would be great to add any kind of test that fails without this patch to prevent any regressions here.

[chaoren]

This isn't a showstopper, I'm just saying that this is a cost of migration for each user with their own sdk configuration.

I agree there's a cost, but it seems relatively small, considering:

1. I would imagine that the majority of users don't use custom SDKs.
2. You can preemptively fix it by adding the filegroup rules to your custom SDK before this change lands.
3. Anyone who relies on rule implementation internals should kind of expect this sort of thing.

It would be great to add any kind of test that fails without this patch to prevent any regressions here.

Done. Please take a look.

[chaoren]

Hmm, actually java_import might work nicer than filegroups. It helps our plugin recognize these as jars, instead of having to manually recognize the name.

[johnynek]

java_import seems totally fine to me as well. That should work. #

[chaoren]

Done. PTAL.

[chaoren]

I added a second commit to cleanup everywhere that uses the exported_files, please let me know if you're okay with that.

[sdtwigg]

This would actually be really helpful in my efforts on getting the rules to consistently work internally. There is an annoying discrepancy where our SDK pieces are all java_import or java_library. This requires some adjustment on imports. It also exposes potential discrepancies between when I test in our repositories versus test in an isolated repository (see the JDK issue) that I have been trying to avoid.

This should also yield improvements to the external rules. Having the items be sourced by java|scala_library|import instead of direct files or filegroups should make it easier for others to replace with their own modified libraries (it also makes src/scala/BUILD somewhat redundant). For example, a user could custom-build their own scala-library and retarget to that.

Testing is a bit tricky. I had an idea regarding toolchains. You would setup a toolchain that declares all the current 'jars' as a java_library rule with no sources but just exports of the real jars. This would test that the rules are properly grabbing transitive runtime dependencies of rules (and not relying on the direct dependencies themselves supplying anything meaningful in files). Amusingly, in theory, this could also lead to using toolchains to bootstrap building custom scala toolchains while still using scala_library. I'm unsure how well this usage model fits with the Bazel vision for toolchains though.

[johnynek]

Thanks for taking the time here. No objections to merging it seems, so I will.