strict_deps and exports

[Irina Iancu]

Hello everyone,

Let's continue the discussion regarding exports and strict_deps from #4584 in this new thread, assuming the add_dep issue is taken care of.

The problem can be summarized as follows:

In the scenario: A -> B -> C => D, trying to use D from A ends up in an error message advertising adding D to the dependencies of A. The discussions floated around whether the message is correct and if not, how to fix it. In this example fixing it means advertising adding C to the dependencies of A.

(Legend: => is exports and -> is deps)

There was a consensus of not handling this in SJD. Carmi suggested Jadep, with some caveats. 

Since the target-label is now stamped on each jar and Tom's proposal includes an API for stamping the jar, wouldn't it be enough to re-stamp the jar every time it's exported? The consumers of JavaInfo can retrieve the exported jars, re-stamp them and put them back.

[Tom Lundell]

That doesn't work because exports aren't unique. You can reach the jar via multiple paths. This is the primary reason why I've said that SJD and/or providers don't work.

[P. Oscar Boykin]

To me, it is correct to add D not C in this case.

If you add C then someone refractors C to no longer export D you have a strange breakage in my opinion.

I think exports should be the exception, not the rule, so I prefer to tell the user to use D directly. I

--
You received this message because you are subscribed to the Google Groups "Bazel/JVM Special Interest Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-sig-jv...@googlegroups.com.
To post to this group, send email to bazel-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-sig-jvm/CAPvN2rXKMs6g_to6sJ4QELYn4JXeYUen3HTHHOt76h43wiR6xw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

P. Oscar Boykin, Ph.D. | http://twitter.com/posco | http://pobox.com/~boykin

[Carmi Grushko]

> I think exports should be the exception

+1

There are some valid use-cases, like exporting plugins (there's currently no better way to do this).

I think that deps should be as dumb and immediate as possible.

For example, if you find a Java file you need, you should be able to immediately find the rule to use.

The Bazel code base's BUILD files are a good (bad) example - every time I have to add a dependency I scratch my head, because there are 4-5 options that would work.

I joined the team after most of it was written, and the various names (build? build-base? package-internal? package) mean very little to me, so I choose ~randomly.

This wastes developer time, and a bad choice means you build more than needed.

The various rules probably started as something tidy and rationale, and human-curated, which is the problem, because it means others on the team need to read a manual of which rules to use, which they won't, and the original meaning will slowly be lost.

Ok wow I got carried away, carry one.

:D

[Liam Miller-Cushon]

On Thu, Apr 19, 2018 at 8:28 PM Carmi Grushko <ca...@google.com> wrote:

> I think exports should be the exception

+1

There are some valid use-cases, like exporting plugins (there's currently no better way to do this).

Agreed. Another reasonable use-case we see is srcs-less forwarding targets that export a private implementation target (e.g. //third_party/foo exports the private target //third_party/foo/v1). But in general, exporting targets that might be needed downstream for 'convenience' makes build graphs harder to reason about maintain, and less performant.

In the other thread we discussed having the tool that fixes SJD errors do some post-build analysis to deal with exports and visibility. I think this is a good solution. It avoids the need to do expensive book-keeping during the build (like re-stamping jars, or maintaining a jar->label mapping in a provider).

My rough proposal is to use blaze query to find the path to the dependency being added, and walk export edges back until the first visible target is found.

[ittai zeidman]

The main use-case I’m trying to solve is indeed exporting private targets for convenience of internal API providers.
Remember the following:
1. Bazel coordinates are strongly coupled to paths.
2. We use package level granularity for targets.
3. We have many-repos.

1&2 mean that for many trivial refactoring a provider might break its consumers and 3 means they can’t fix it atomically without some very serious tooling.

For us the inability to have an abstraction mechanism (or alternatively have an abstraction mechanism but render add deps useless) is a big problem.

Cushon/Carmi,
Any chance you can elaborate a bit on how the tool can solve it? I’m definitely not married to the stamping labels approach but I do need the solution to be dead simple.
People just copy paste the buildozer command a continue on (and they love it)

[Carmi Grushko]

Re refactoring: 

side question out of curiosity - 

Given (2), any change in BUILD structure implies change in symbol names, so other code in other repos must be updated when refactoring, doesn't it?

Anyway, don't want to dig into this too much, if you say that's the way it is then that's the way it is.

Re new tool:

In the example above, a blaze query is used to obtain a path from A to D (we know both A and D at this point).

Any ancestor of D in this path, when only considering exports and alias(), that's also visible to A, can be used instead of D.

So the tool walks the path up and returns the furthest ancestor.

Together with filtering out rules with tags=["avoid_dep"] and deprecation= attributes, you can refactor, leave a tombstone rule, and have tools automatically use the rule you want to be used.

[ittai zeidman]

Sounds really good.

Do you suggest Bazel will output the jadep message? Out of curiosity is it in addition to the buildozer command? 

Less important questions:

1. What if C exports D and D is also visible to A? One advantage to adding C (and not D) is that it can allow library owners to keep things public for the edge cases which need fine granularity but still make refactoring easy

2. What will happen with strict-deps checks themselves in the A -> B => C scenario? They will continue to pass, right?

[Carmi Grushko]

It won't probably be part of Jadep, I think - they're too different.

> Do you suggest Bazel will output the jadep message? Out of curiosity is it in addition to the buildozer command? 

Probably instead of the buildozer commands, but Liam knows better.

> What if C exports D and D is also visible to A?

If you want automating tools to avoid D, internally we have an unwritten agreement that `tags = ["avoid_dep"]` will cause a tool to skip it, while Bazel ignores tags.