strict deps labels are incorrect when using non java_* rules

[ittaiz]

Description of the problem / feature request:

Strict deps labels are incorrect when using scala_import
I suspect the problem is in the java rules and not scala_import since java_rules seems to perform their own heuristics which breaks if a different "actor" is in play and scala_import here is accidental. I might be incorrect in my analysis but a few months ago when I debugged it this is what I saw (don't have links to bazel code now, sorry :( )

Feature requests: what underlying problem are you trying to solve with this feature?

Trying to use java_library or java_common.compile with strict deps warnings

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

clone https://github.com/ittaiz/bazel-strict-deps-labels-issue
run bazel build --strict_java_deps=error //...
see:

TransitiveUser.java:1: error: [strict] Using type com.google.common.cache.Cache from an indirect dependency (TOOL_INFO: "@com_google_guava_guava//jar:guava-19.0.jar"). See command below **
import com.google.common.cache.Cache;
                              ^
 ** Please add the following dependencies:
  @com_google_guava_guava//jar:guava-19.0.jar  to //:transitive_user
 ** You can use the following buildozer command:
buildozer 'add deps @com_google_guava_guava//jar:guava-19.0.jar ' //:transitive_user

INFO: Elapsed time: 0.279s, Critical Path: 0.14s
FAILED: Build did NOT complete successfully

expected at the very least @com_google_guava_guava//jar and actually I'd like it to be //:import since the rule effectively exports that file

What operating system are you running Bazel on?

os x

What's the output of bazel info release?

release 0.8.1-homebrew

[ittaiz]

cc @iirina @dbabkin

[iirina]

@ittaiz Could you point me to your code where you collect the jars and labels for strict deps?

[ittaiz]

This is for Scala import
https://github.com/bazelbuild/rules_scala/blob/master/scala/scala_import.bzl
Do you need also the “regular” Scala rules? There it’s a bit more complicated (though not very complicated)

[iirina]

No, I get the idea. Thanks! Just to be sure, JavaInfo(originating_label = ..., ...) + dep[JavaInfo].originating_label would make this easier for you?

[ittaiz]

I think we have 3 issues here actually: 1. Collecting labels with the new API to pass to the compiler plugin-Yes 2. Java scala rules sandwich with respect to labels- No. The additional needed feature for it is that the provider will keep and expose the transitive labels since currently rules Scala just pass a custom dict which java rules don’t. 3. Java_common.compile resolves transitive labels incorrectly- this will be resolved if we have 2 and the java rules will also use it. Btw, Just to make sure- AFAIU if I “export” a dependency then when someone asks for its label they will actually get mine, right?

…

On Thu, 8 Feb 2018 at 13:30 Irina Iancu ***@***.***> wrote: No, I get the idea. Thanks! Just to be sure, JavaInfo(originating_label = ..., ...) + dep[JavaInfo].originating_label would make this easier for you? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4584 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF8fiuTEsfNH75MQCv9NTy7cyYn9uks5tStrWgaJpZM4R5xhL> .

[iirina]

Just to make sure- AFAIU if I “export” a dependency then when someone asks for its label they will actually get mine, right?

They should get the label of the rule where the provider was created. The "exports" providers will just be passed around to the depending targets, so the label will be from where they originated.

[iirina]

The additional needed feature for it is that the provider will keep and expose the transitive labels.

I assume you need it only from the compile-time transitive closure, right? I don't like that for almost everything exposed there is a "compile-time" and a "runtime" version. But I don't see any alternative, so I will just expose the labels for the targets needed at compile-time for now.

expose the transitive labels since currently rules Scala just pass a custom dict which java rules don’t.

A dict is different than a set of transitive labels. How will you use the transitive labels collected from JavaInfo in the sandwich?

[ittaiz]

Not sure these are the correct semantics- I tried reading about exports in the documentation and I think (but not sure) that the correct semantics are that if A depends on B, B depends on C and C exports D then given A transitively uses D it should get a warning to add C to its direct dependencies.
How this needs to be translated to the specific mechanics of providers is a good question but I think we should first agree on the semantics. WDYT?

[iirina]

If you have A -> B -> C => D (where => is exports and -> normal dependency) and you try to use D from A you would get a warning/error to add D to the dependencies of A.

[iirina]

In this situation:

     A
  /     \
 C  --   B
//
D

where A uses D, there would be no error/warning raised.

[ittaiz]

Why do you think I should get a warning about D ? It makes more sense to me that I should get a warning about C since I think from Cs point of view D should be somewhat hidden

[ittaiz]

A dict is different than a set of transitive labels. How will you use the transitive labels collected from JavaInfo in the sandwich?

Sorry missed your message. I might be wrong but I think we use a dict now since that's what the compiler plugin needs, right? It needs to know the label of a specific jar and hence we need a dict.