scala_export macro #1790
scala_export macro #1790
Conversation
vinnybod
requested review from
WojciechMazur,
mbland and
simuons
as code owners
mbland
left a comment
mbland
left a comment
There was a problem hiding this comment.
Overall, I think it looks mostly pretty good, modulo the few requests I've made so far, particularly around legacy WORKSPACE support. I also need to study it a bit to make sure I understand the whole thing, and I'll eventually pull the branch and run the tests locally.
That said, I'm going to play the BazelCon card and decline to commit to looking any more closely until about a week from now. (If changes come in, and I'm inclined, I may respond sooner. But no promises until next week.)
mbland
left a comment
mbland
left a comment
There was a problem hiding this comment.
One more thing: there are a number of test failures in CI.
If you haven't already, please run the test scripts locally and fix what you can, ideally getting ./test_all.sh to finish successfully once you're done. I'm happy to dig in and try to help if you get stuck. (After BazelCon.)
And FWIW, I think the Windows failure may be CRLF vs. LF related, but I haven't looked very closely yet. But check out the changes to src/java/io/bazel/rulesscala/worker/WorkerTest.java from #1724, in case it helps.
|
Looks like something in the docs jar is not reproducible. Will need to spend some more time looking at that. |
| @@ -0,0 +1,79 @@ | |||
| load("@aspect_bazel_lib//lib:diff_test.bzl", "diff_test") | |||
There was a problem hiding this comment.
Using bazel_lib instead of skilib because the skylib diff_test fails on windows with this error bazelbuild/bazel-skylib#529
2 participants
Description
This adds a drop in replacement for
java_exportthat works for scala.We've been using this internally at Confluent for some time now in our rules_jvm_external fork.
https://github.com/confluentinc/rules_jvm_external/blob/master/private/rules/scala_export.bzl
Motivation
We needed this at Confluent and figure other folks might find it useful. Originally I tried to upstream this to rules_jvm_external, but it was decided that it would be more appropriate here and would help to avoid circular dependencies between rules_scala and rules_jvm_external if rules_scala ever chose to adopt rules_jvm_external in the future.