Fix cases where transitivity is required despite strict_deps; enable within the repo for Java
Review Request #3125 - Created Nov. 14, 2015 and submitted
|benjyw, jsirois, nhoward_tw, zundel|
This patch fixes two cases where transitive deps are required at compile time despite
targetaliases: the direct deps search now walks through aliases (but not fully transitively). In particular, when depending on the alias
3rdparty:protobuf-java, it is resolved to
- "compiler plugins": for code that runs in the compiler, compiletime is actually runtime, so transitive deps must be provided
The patch also enabled
strict_depsfor Java code within the repo, and applies it explicitly to zinc.
There is one open question represented by a few TODOs in the patch: should it be possible for a target
A, depended on by target
B, to add additional deps for
B? Some potential usecases represented by TODOs in the patch:
- @Annotation interfaces seem to be required in cases where a class in package
Binherits from an annotated class in package
- In codegen, it will frequently be the case that it is impossible to use the generated code in target
Awithout importing a runtime library in target
- Also in codegen, there are cases where deps are directly inserted into the build graph without existing on disk (see
wire-runtime), and where writing them out to disk (anywhere other than in
BUILD.tools) would make it very likely for them to go out of sync with the in memory declaration.
First of all, I'm fine if we want to enable this feature in the pants source code, but its going to make reproducing errors in repos that don't have this a little bit tougher. It seems like a bit of an unusual configuration to me, but from what I've heard, this is the desired state at Twitter and Foursquare.
I've brought up the idea of explicitly declaring all dependencies in all BUILD files and have met with tepid enthusiam at Square. The pushback is that it seems like a lot of manual configuration (and the potential to leave unused deps lying around, we don't have a team of folks working to keep our build hygenic) for what benefit? I'd like to see some numbers on how how much time it saves in a compile of a big Java target. I guess Square might be the only one who has that so I'd have to check for myself.
We do make a lot of use of
target()style aliasing for lots of different reasons. I went through some scenarios in our code base and it looks like we can make it work with your change adding
Just to be clear, this isn't a wire specific issue, aren't all code generation tools supposed to be able to work like this?
Why do you have to explicitly turn this on here? The other settings seem to indicate that you want strict_deps on by default in pants.ini and individual targets are turning it off.
Add one additional dep to the shader test.
Revision 2 (+107 -20)