https://pantsbuild.org/ logo
Join Slack
Powered by
<@U06A03HV1> <@U0N6C2Q9F> Running into a weirdness...
# development
a
@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
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
That’s not right
w
or whichever position. generic method in this case i guess?
a
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
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
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
I also may have missed some type usage for consumed types.
a
OK. Let me get this repro case working first, to see if I understand it properly
f
the 
java-dump-first-party-dep-map
debug goal will dump the dep inf mapping.
👍 1
a
handy!
f
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
@fast-nail-55400 How do I see if matcher is a consumed type here?
f
for a specific file? dump the dep map into JSON, view the JSON, and search for 
Matcher
a
so it’s literally just looking to see if it’s in the json?
f
yup
w
it seems to only render the 
type_map
when i use it, rather than a consumed map
a
Yeah, that
f
probably need to update the relevant 
to_json_dict
method then
a
got it. Thanks
f
actually consumed types isn’t aggregated
w
i don’t think that there is a relevant structure to actually render for consumed types, since that is per-file.
coke
a
… huh. 
PackageRootedDependencyMap
only tracks the `type_……… yeah that
f
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
@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
the BUILD files have explicit third-party deps since I wrote those prior to the third party dep inference support landing
w
ah, k. i’m off in another repo anyway. will investigate.
f
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
yea, that sounds right.
i’ll be trying it out with 
hazelcast
.
👍 1
a
@witty-crayon-22786 This is probably relevant to that: https://github.com/pantsbuild/pants/pull/13531
👍 1
Open in Slack
PreviousNext
Powered by