https://pantsbuild.org/ logo
Powered by
a

ancient-vegetable-10556

11/08/2021, 5:07 PM
@witty-crayon-22786 @fast-nail-55400 Running into a weirdness around Java dependencies, and wanted to know if this is a thing we’ve thought about:
There’s a test in guice (
DuplicateElementErrorTest.java
) that has an inline class 
extends AbstractModule
, and 
AbstractModule
has a method which takes 
Matcher<T>
as a parameter. 
javac
is failing to compile 
DuplicateElementErrorTest
because there’s no class file for 
Matcher
available
unlike most dependencies, I think the fact that 
Matcher
is generic means the class file needs to be present during compilation in order for the compiler for 
AbstractModule
to verify type bounds
The blunt instrument solution is to include second-degree dependencies when resolving the compilation classpath; but there’s probably smarts involving generics that we need to figure out
w

witty-crayon-22786

11/08/2021, 5:11 PM
yea, this is what i started to mention on Friday
it’s possible that 
Matcher
should be “exported” by files that use it
i’ve thrown “exported” around a few times: it was supported in v1, and is supported elsewhere: https://docs.bazel.build/versions/main/be/java.html#java_import.exports
but when a target 
exports
a dep, it is available to consumers of the target, even when they only depend on the target
i think that inference could 
export
Matcher
in this case when we see it in the 
implements $X
or 
extends $X
position
a

ancient-vegetable-10556

11/08/2021, 5:15 PM
That’s not right
w

witty-crayon-22786

11/08/2021, 5:15 PM
or whichever position. generic method in this case i guess?
a

ancient-vegetable-10556

11/08/2021, 5:15 PM
The issue is that there’s a method 
public void foo(Matcher<T> matcher) {}
` which needs the definition of 
Matcher
to be available to figure out the type bounds
So it’s actually any exposed consumption of matcher that matters here
w

witty-crayon-22786

11/08/2021, 5:17 PM
a failing testcase for this one might be worthwhile, since we’ve all encountered it at this point, and knowing how to repro it would be good.
a

ancient-vegetable-10556

11/08/2021, 5:17 PM
so as far as inference goes, for any target X that has a dependency Y which has a dependency on matcher, X needs to also depend on matcher there.
OK, let me sort one of those out.
BACK SOON
f

fast-nail-55400

11/08/2021, 5:20 PM
I also may have missed some type usage for consumed types.
a

ancient-vegetable-10556

11/08/2021, 5:21 PM
OK. Let me get this repro case working first, to see if I understand it properly
f

fast-nail-55400

11/08/2021, 5:22 PM
the 
java-dump-first-party-dep-map
debug goal will dump the dep inf mapping.
👍 1
a

ancient-vegetable-10556

11/08/2021, 5:22 PM
handy!
f

fast-nail-55400

11/08/2021, 5:22 PM
and then you can see if Matcher was marked as a consumed type
and the parser test has various Java syntax constructs; we can add a test for this there
a

ancient-vegetable-10556

11/08/2021, 7:25 PM
@fast-nail-55400 How do I see if matcher is a consumed type here?
f

fast-nail-55400

11/08/2021, 7:27 PM
for a specific file? dump the dep map into JSON, view the JSON, and search for 
Matcher
a

ancient-vegetable-10556

11/08/2021, 7:27 PM
so it’s literally just looking to see if it’s in the json?
f

fast-nail-55400

11/08/2021, 7:27 PM
yup
w

witty-crayon-22786

11/08/2021, 7:27 PM
it seems to only render the 
type_map
when i use it, rather than a consumed map
a

ancient-vegetable-10556

11/08/2021, 7:27 PM
Yeah, that
f

fast-nail-55400

11/08/2021, 7:28 PM
probably need to update the relevant 
to_json_dict
method then
a

ancient-vegetable-10556

11/08/2021, 7:29 PM
got it. Thanks
f

fast-nail-55400

11/08/2021, 7:29 PM
actually consumed types isn’t aggregated
w

witty-crayon-22786

11/08/2021, 7:29 PM
i don’t think that there is a relevant structure to actually render for consumed types, since that is per-file.
coke
a

ancient-vegetable-10556

11/08/2021, 7:29 PM
… huh. 
PackageRootedDependencyMap
only tracks the `type_……… yeah that
f

fast-nail-55400

11/08/2021, 7:29 PM
so maybe just add a debug goal to aggregate consumed types
the goal would have targets so just 
MultiGet
the 
JavaSourceDependencyAnalysis
for the targets and then output the 
.consumed_unqualified_types
field
w

witty-crayon-22786

11/08/2021, 7:31 PM
@fast-nail-55400: it doesn’t like https://github.com/toolchainlabs/guice/blob/setup_pants/pants.toml is actually using a thirdparty mapping currently… is that because it has no thirdparty deps?
f

fast-nail-55400

11/08/2021, 7:32 PM
the BUILD files have explicit third-party deps since I wrote those prior to the third party dep inference support landing
w

witty-crayon-22786

11/08/2021, 7:33 PM
ah, k. i’m off in another repo anyway. will investigate.
f

fast-nail-55400

11/08/2021, 7:33 PM
we could always port 
guice
to use third-party dep inference instead, but I haven’t had the time to do that (and probably won’t since scala dep inf is the priority for me)
w

witty-crayon-22786

11/08/2021, 7:33 PM
yea, that sounds right.
i’ll be trying it out with 
hazelcast
.
👍 1
a

ancient-vegetable-10556

11/08/2021, 7:35 PM
@witty-crayon-22786 This is probably relevant to that: https://github.com/pantsbuild/pants/pull/13531
👍 1
Powered by