Use zipper rather than jdk/jar in the thrift rules #286
Conversation
| @@ -135,8 +148,7 @@ thrift_library = rule( | |||
| "absolute_prefixes": attr.string_list(), | |||
| # This is a list of JARs which only contain Thrift files | |||
| "external_jars": attr.label_list(), | |||
| "_jar": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/jdk:jar"), allow_files=True), | |||
| "_jdk": attr.label(default=Label("//tools/defaults:jdk"), allow_files=True), | |||
| "_zipper": attr.label(executable=True, cfg="host", default=Label("@bazel_tools//tools/zip:zipper"), allow_files=True) | |||
There was a problem hiding this comment.
TIL this exsited! This kind of thing needs better publicity. I've cargo culted around the restamped archive code in like every archive rule I've written
|
I tested this final version on our largest internal repo and it seemed to work (and get rid of an annoying warning we saw when we had targets with no thrifts that only had external jars). |
|
@johnynek looks good. |
|
@ittaiz if we squash, then the 2 branches that depend on this will likely have a painful merge process. I would really like to just regular merge these as is to avoid that. Then the other two can be easily squash merged into the repo saving precious time. |
|
oh, wait, you did merge regular, so I think I can squash merge this now into one commit in the github UI. Trying. Thanks! |
|
yeah, that worked: ecbe5e7 |
|
Glad to hear. Are you saying you chose |
|
Great. I might be confusing things here but can we use the zipper to
replace the JarCreator used by ScalacProcessor?
…On Sat, Sep 30, 2017 at 10:06 PM P. Oscar Boykin ***@***.***> wrote:
@ittaiz <https://github.com/ittaiz> I agree: #288
<#288>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#286 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF8V9Ql8bBWzwBD-2u0_FvcRXLazOks5snpElgaJpZM4PpTdZ>
.
|
|
@ittaiz I don't think we can because each action needs to make one output. We use JarCreator to produce single outputs from actions, but maybe I'm wrong. Maybe the action can write to a directory, and a subsequent action can read that directory. That said, I don't see the big win there. That java code was also from bazel, it works and can be compiled with JIT so it is fast. I think spinning up a jvm to make an empty jar is definitely a bummer. |
|
You're right on both counts. I agree. |
Bazel has a built in command for building zips deterministically. Use that rather than the find + timestamp setting trick.
closes #282