https://pantsbuild.org/ logo
Join Slack
Powered by
Someone with experience using `jvm_war`: Context:...
# development
p
Someone with experience using `jvm_war`: Context: I'm studying various fields to see if we could combine 
content
with 
dependencies
(actually looking at the feasibility to get rid of all of the 
SpecialCasedDependencies
fields like 
content
). If you put 
file(s)
or 
resource(s)
in the 
jvm_war(dependencies=...)
field, are they ignored today (I hope the answer is yes)? Or do they somehow end up in one of the JARs on the classpath? It's not clear to me from reading through the rules.
w
If you put 
file(s)
or 
resource(s)
in the 
jvm_war(dependencies=...)
field, are they ignored today (I hope the answer is yes)? Or do they somehow end up in one of the JARs on the classpath?
the latter. there is slightly different handling for 
content
vs 
dependencies
, but both are actually “dependencies” which should be tracked 
./pants dependencies
, 
--changed
, etc.
to be clear: when i suggested removing handling of 
SpecialCasedDependencies
, i wasn’t necessarily suggesting getting rid of having multiple dependencies fields: ideally, 
TransitiveTargets
would support collecting multiple 
Dependencies
fields
p
Maybe the field itself could have a flag: don't traverse me unless requested by a predicate that knows about me.
w
wouldn’t it be better if the predicate could drive that…?
a strawman would be something like:
Copy code
def include_dep_field(target: Target, field: Dependencies | SpecialCasedDependencies) -> bool: ...
(i mean, ideally we could just merge the two into 
Dependencies
with more subclasses: that’s a larger project, but)
p
So are you imagining a default predicate that deals with the special cased fields? So, the predicate will run for every dep whereas right now I have avoided running the predicate unless requested.
w
You could do every dep, or just every Field subclass, yea
f
for context, the reason why 
content
and 
dependencies
are different is that the 
content
is unpacked inside the WAR while jars from 
dependencies
are not
from a UX perspective, merging the two might confuse developers who can currently see which targets are being used for actual web content pages and which are just JVM code.
and from a technical perspective, content and code dependencies live in two separate subpaths of the WAR output
moreover, merging them together will make it ambiguous whether resource files/jars should be content or resources jars for the classpath
In my view, that is enough for me to veto merging 
content
and 
dependencies
together.
p
Sounds good. I'm investigating supporting multiple dependencies fields instead of merging all into one.
❤️ 1
And thank you for the feedback on usage. That's very helpful.
Well, multiple 
Dependencies
fields on 
jvm_war
would be problematic because 
tgt.get(Dependencies)
would (since we always subclass that) get the first field that subclasses 
Dependencies
with one big gotcha: The fields are sorted by alias, so 
jvm_war_tgt[Dependencies]
would return the 
content
field since that sorts before the 
dependencies
field. So, we would probably need to extend the Target interface so that you can ask for all fields that subclass a given field (
Dependencies
in this case), and then you would create a 
DependenciesRequest
for each one. Or, we need to be able to sort the subclassed fields in some other way, for example by preferring one that has the same alias as the parent class.
w
I think that a 
Target.get_all(Field)
would be reasonable... most callsites wouldn't care, but some would
@hundreds-father-404 had thought about this a bit, and might know of some other things to look out for.
p
Oh shoot. When a 
FieldSet
uses a super class type, and we allow multiple fields of that type, it might not get the right field. 😞 https://github.com/pantsbuild/pants/blob/main/src/python/pants/engine/target.py#L1234 Luckily, the 
VisibilityFieldSet
is the only 
FieldSet
I've found that has a dataclass field for 
dependencies: Dependencies
, the other field sets that require a 
Dependencies
field only include it in 
required_fields
. `VisibilityFieldSet`: https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/visibility/lint.py#L25-L28 @curved-television-6568 I wonder how we could change the lint rule that uses that field set so it can handle multiple dependencies fields per target. So far, I'm assuming that 
DependenciesRequest
will be for one target+field, so if there are multiple Dependencies fields, then we need multiple `DependenciesRequest`s.
c
Yes, I imagine that targets with multiple fields of a given type that is included as a member field of a field set would need to result in multiple field set instances, one for each such target field.
p
Oh! @witty-crayon-22786 You were suggesting getting rid of the 
include_special_cased_dependencies
flag, not eliminating 
SpecialCasedDepencies
in my PR (and therefore merging that with 
Dependencies
). OK. I think I can do that - the rest is a really big rabbit hole that I was getting lost in.
w
Yea, that is totally fine. It's a very important rabbit hole, so I appreciate the time spent on it! But yea, just the ability to filter on multiple fields would unblock.
2 Views
Open in Slack
PreviousNext
Powered by