Add empty srcs support to the scala rules #948
Conversation
Change-Id: I3f2b984129e725076ba4736700b0459d233cc3f4
Change-Id: I9a8e229e3ec19521d2e7e8d8e447949586f6b126
| change_dir = "-C " + c_dir if c_dir else "" | ||
| res_cmd = "\n{jar} uf {out} " + change_dir + " " + res_path | ||
| ijar_cmd = "" | ||
| if buildijar: |
There was a problem hiding this comment.
Why have buildijar here if you are not using it?
Also the ijar should be just an empty jar here, no?
There was a problem hiding this comment.
Yeah, I don't like it. But scala_library defines an ijar output always. I don't see how I can tell bazel that output is only generated when srcs are non-empty. So, what I do is just copy the (empty) jar to be the ijar. I do this with the flag so that we can accomodate scala_macro_library which never creates ijars.
So basically a desire for consistency with java_library and both scala_library and scala_macro_library motivated this choice.
There was a problem hiding this comment.
I mean why is there _build_nosrc_jar(ctx, buildijar) if everwhere you call it buildijar = False?
There was a problem hiding this comment.
There was a problem hiding this comment.
This is not a call to build_nosrc_jar, was it supposed to be?
There was a problem hiding this comment.
no, sorry I was not very clear:
scala_librarycalls_lib: https://github.com/bazelbuild/bazel/pull/948/files#diff-981b14fba970d9a9d0dd316467d216eeR222_libcalls_compile_or_empty: https://github.com/bazelbuild/bazel/pull/948/files#diff-981b14fba970d9a9d0dd316467d216eeR198_compile_or_emptycalls_build_nosrc_jar: https://github.com/bazelbuild/bazel/pull/948/files#diff-981b14fba970d9a9d0dd316467d216eeR110
There was a problem hiding this comment.
Sorry so I was, ijar is ignored right after being produced (buildijar can indeed be true):
https://github.com/bazelbuild/bazel/pull/948/files#diff-981b14fba970d9a9d0dd316467d216eeR112
There was a problem hiding this comment.
yes, but not ignored here:
https://github.com/bazelbuild/bazel/pull/948/files#diff-981b14fba970d9a9d0dd316467d216eeR52
where bazel it checking that an ijar war produced. There are two places where ijar identity is relevant;
- where bazel checks that all outputs are created by some rule.
- where dependant scala rules look for their compile time dependency.
So, in 2 above, I don't bother to give it the path to the copy, but I generate that path for 1 so bazel sees that it is created.
Would you prefer another approach?
There was a problem hiding this comment.
No I am fine with it. Just though the ijar wasn't actually used
|
This is LGTM but won't be merged, I will be moving the scala rules to a separate repository today, then we can merge it directly to the new repo :) |
|
@damienmg exciting. Should I open a new PR there? |
|
Yes open a PR (well or just merge it directly, you have write access :)) |
When consuming external jars, we often need to declare many compile time dependencies (especially in scala due to scala needing to read scala specific attributes: https://groups.google.com/forum/#!topic/scala-language/0B678ZjZQAw ).
A workable pattern is to make a target that only has exports and runtime_deps for large projects such as Hadoop. Then the user can depend on one target that gives the minimal needed compile-time and runtime dependencies.
This change adds support for empty srcs attribute by making an empty jar (and ijar) and adds tests that it works (along with a scala_macro_library test).