Improve scala to java exports

[johnynek]

See: #56 (comment)

one idea could be to improve this:

https://github.com/bazelbuild/rules_scala/blob/master/scala/scala.bzl#L490

so it is a skylark rule, not a macro, that consumes targets and looks at their runtime and compile-time jars, and just exports them (right now it is not saving much effort).

Then, that could be used to test this feature. I'll add an issue.

the fix we added for #18 is still pretty mechanical: you pass output jar names. What we really want is to just list targets and get all their exports and runtime deps for use from java.

In this way, we can test that the runtime paths are correct without conflating with scala_binary.

[johnynek]

/cc @colinmarc @jcoveney

[colinmarc]

What about adding a java provider to scala_library? Will the java rules pick that up automatically?

[colinmarc]

Also, if that doesn't work, we could generate the java_import rules preemptively as foo_javaimport.

[johnynek]

There is a ticket for some API to make this work, but I think the java rules don't have a contract like this. I wouldn't want to add preemptively out of anxiety that it would slow down the build by doubling the number of targets, half of which would not be used in scala-only repos (even with mixed java-scala it is not that common to depend across languages).

[johnynek]

I don't see how to do the java_import. We can only call native rules from macros, not other skylark rules. But we can't see in a macro all the dependencies we need to add to the java_import.

I think we may just have to wait until bazelbuild/bazel#970 is solved. I can't see any non-manual way to work around this, but maybe I'm missing something.

[colinmarc]

What if we benchmarked preemptively creating java_import (assuming that actually works)? It may be that perf works fine with double the targets.

[johnynek]

Sure, that would be fine. I actually don't know how to do it though.

Can you see a way?

On Wednesday, May 25, 2016, Colin Marc notifications@github.com wrote:

What if we benchmarked preemptively creating java_import (assuming that
actually works)? It may be that perf works fine with double the targets.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#57 (comment)

P. Oscar Boykin, Ph.D. | http://twitter.com/posco | http://pobox.com/~boykin

[colinmarc]

I think we'd have to give the java_import the specified name, and then build the library jar as a separate _scalajar rule or something. I can tool around with it later.

[johnynek]

So, with the recent merge of correct deploy jars we can just do Java import
of deploy jars, bit that is not a great solution since it hides duplicate
classes on the class path, although they should be identical.

How urgent is this? What if we just hope bazel makes it possible to do Java
interoperability as is being discussed?

On Thursday, May 26, 2016, Colin Marc notifications@github.com wrote:

I think we'd have to give the java_import the specified name, and then
build the library jar as a separate _scalajar rule or something. I can
tool around with it later.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#57 (comment)

P. Oscar Boykin, Ph.D. | http://twitter.com/posco | http://pobox.com/~boykin

[ittaiz]

@sdtwigg at the risk of extending our conversation to yet another issue this one seems like the most relevant one about enabling java_* targets to depend on scala_* targets. Any update on that?
I thought issue 970 has a bit too much noise so maybe we should continue here for followup which doesn't require the Bazel people.

[sdtwigg]

Sorry for the delay here. I was ill over the weekend and have been playing catch-up since then.

There was a slight confusion in getting the necessary reviews from the bazel team but things should be in sync now. Here is what I have so far:

Regarding critical bazel contributions:
https://bazel-review.googlesource.com/#/c/9671/ - This one is critical. Otherwise, you will find it somewhat arbitrary what arguments in a java native rule accept skylark rules. This would make it so that all of the java_library|import|binary accept skylark rules for deps|exports|runtime_deps.

Regarding rules_scala changes:
See sdtwigg@8be3df5 as the minimum viable product for this working. As-is, it does work and is how the main API will ultimately probably look. Preliminary tests indicate it is working fine (especially after the other cleanup PR I did which took care of most of the other oddities that used to be in that commit).

I have not been aggressively pushing to submit due to these two pending bazel contributions:
https://bazel-review.googlesource.com/#/c/9730/ - Changes java_common.create_provider to take depsets instead of lists. API-breaking (for an unpublished, experimental API) that would break my rules_scala changes until they are adjusted. It is a simple adjustment (involving removing the to_list calls!) but still technically breaking.

https://bazel-review.googlesource.com/#/c/9670/ - This cleans things up the implementation and better restricts compile jars. Internally, it means we can just use JavaProvider primarily just like the java rules are using our java provider. Also, cleans up a glitch in current implementation where it is getting transitive compile jars from java rules rather than just the 'local' compile jars.

[sdtwigg]

Update with good news: All the referenced changes to bazel have been merged.

[ittaiz]

Awesome!!! Do you have any estimate on when you'll be able to have a PR?

…

On Tue, 23 May 2017 at 0:28 Stephen Twigg ***@***.***> wrote: Update with good news: All the referenced changes to bazel have been merged. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#57 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF_ijwcmp_dlckaXO-cbsmPYSQZS2ks5r8f4FgaJpZM4IirKL> .

[johnynek]

Thank you so much for pushing those through! On Mon, May 22, 2017 at 17:44 Ittai Zeidman <notifications@github.com> wrote:

…

Awesome!!! Do you have any estimate on when you'll be able to have a PR? On Tue, 23 May 2017 at 0:28 Stephen Twigg ***@***.***> wrote: > Update with good news: All the referenced changes to bazel have been > merged. > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > < #57 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ABUIF_ijwcmp_dlckaXO-cbsmPYSQZS2ks5r8f4FgaJpZM4IirKL > > . > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#57 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJdms3wjZc6wob6tp1KkSJTBfjjEawks5r8lYBgaJpZM4IirKL> .

[ittaiz]

@sdtwigg
Any updates? I'm mainly asking since there are some changes I'm thinking about which will probably be easier when this lands

[ittaiz]

Hey,
@sdtwigg I'm sorry to be a nuisance but would you rather someone take sdtwigg@8be3df5 and continue from there?
I don't want to stress you too much but on the other hand I think it would help with a few different issues (junit logs, javac default encoding not being UTF8 and maybe javac default debug symbols)

[ittaiz]

@johnynek this is solved, right?

[johnynek]

I think so. yes!