bazel-deps
bazel-deps copied to clipboard
Make indirect dependencies have restricted visibility
Right now, all the generated targets have visibility = ["//visibility:public"]
. This allows a kind of strict dependency violation where a build target could depend on a maven artifact not mentioned in the yaml file.
The targets for the indrect dependencies should have visibility = ["//${thirdPartyDirectory}:__subpackages__"]
so that they can only be referenced by targets under the third party directory.
Originally https://github.com/johnynek/bazel-deps/issues/98#issuecomment-349928980
Hacked together an initial POC in #107.
We will probably have to do:
- make a flag to disable/enable the behaviour (it can be quite disruptive for existing users)
- also add the visibility to
dirname("path/to/workspace.bzl")
since it does not have to be located in the same directory as generated BUILD files - ~~maybe add an option to expose transitive dependencies to a custom target (writing custom rules that depend on transitive depdendencies, not sure about this one)~~
EDIT: Thinking about this again 3. is probably not useful
adding the co-ordinates for 400 dependencies by hand is not fun. is this really really necessary ? Bazel has a query tool and there is a lock file under source control. I don't see what this layer adds.
Using deps like spring boot and dropwizard is completely infeasible with strict deps -- I spent 90 minutes openening up deps but there is no end in sight. Like 70% of the deps in the transitive closure are needed to create a web application and they don't all neatly fit in a modules
list. The standard jetty / jersey stack alone is insane (glassfish, javax.*, jetty, hk2, jersey, jackson, swagger).
Realistically I'd have to buildozer and open the visibility after the regenerating the deps.
I'm open to a PR to add an option, but I'm in the middle of a big push on rules_scala and won't have time this week, and I'm on vacation next week.
This didn't burn us internally at stripe and actually solved several problems. If you are manually depending on these things, it seems like only O(1) work to declare them (you don't need to mention the version, just declaring the language is enough).
I am still not convinced we don't want to be strict, but I am willing to add a non-default option to be lax.