https://pantsbuild.org/ logo
Join Slack
Powered by
Big oof. The unification of file arguments and gen...
# development
h
Big oof. The unification of file arguments and generated subtargets breaks when you are a) not using dep inference and b) a file depends on other files in the original owning target
Let’s say you have the target 
:utils
. 
strutil.py
depends on 
dirutil.py
. No dep inference. Then, 
./pants dependencies strutil.py
will not end up including 
dirutil.py
or 
:utils
, and we won’t include 
dirutil.py
anywhere in the chroot
I’m not sure how to resolve this. Possibly, when using a generated subtarget, we should no matter what create a dependency on every other file in the target? I’m thinking how this impacts targets with no dep inference like 
resources
. When you use a file arg on a 
resources
target, what should 
dependencies
show? -- Possibly, the unification of file args and generated subtargets was a bad idea? -- Possibly, we should only use generated subtargets for file args if 
--dependency-inference
is enabled? But, that doesn’t solve the first question in this message about target types without inference.
w
commented on the ticket, but: aiming to use the same underlying code/validation uniformly (regardless of how it is being consumed) might help? happy to shipit if you want to defer thinking about it for now though.
h
These are distinct issues, but similar scenarios
The PR is solely about the case of 
!
validation. This thread is entirely about false negatives, that we are going to leave off dependencies on files within the original owning target that really do exist.
w
ah, looking more closely. sorry.
Possibly, we should only use generated subtargets for file args if 
--dependency-inference
 is enabled? But, that doesn’t solve the first question in this message about target types without inference.
or perhaps the inverse? a file Address is always subject to inference? it doesn’t resolve the resources issue… but could perhaps address that by never generating subtargets in that case.
h
Possibly that would solve this problem for strictly Python targets, I think. But it could be surprising because you would possibly resolve targets that you otherwise wouldn’t have resolved. That would result in inconsistent behavior, e.g. that you only have a cycle when using file args And I think it would be confusing for dep inference to change how things behave. For example, I’m thinking about how this problem will impact the example bash plugin repo.
Now that we’ve had generated subtargets for two weeks, I do think it’s worth taking a step back and determining if we truly want to stick with feature or not. It is trickier than I expected. While we were able to encapsulate it from downstream rules, it is a lot of new concepts and special cases for end users. You get a pretty different experience whether you use file args or address args, and that seems confusing / complex. Ignores are difficult to reason about. There are now sometimes files in 
dependencies
, but also some addresses; and those files aren’t truly files.
w
my personal feeling is that it is still worthwhile, but that we’ll need to continue to be open to semantics changes in order to finish and polish it. it’s a pretty large project, and i think that the edge cases are understandable at this phase
if you feel like summarizing the open issues somewhere, I'd be happy to try to think through it tomorrow
h
How about open issues + pros/cons/weird things that may trip up users?
w
sure, that works. just anything to get a big picture view of remaining issues/confusions
no need to open new tickets if they don’t already exist yet though.
h
cc @witty-crayon-22786. This thread describes the problem and https://github.com/pantsbuild/pants/issues/10423 includes an explanation
w
I’m not sure how to resolve this. Possibly, when using a generated subtarget, we should no matter what create a dependency on every other file in the target?
this is what i was thinking, yea.
not sure if that’s maybe conditional on inference being enabled, or what. but yea, open question for sure.
h
If it were conditional on inference, then I think that we would need to introspect UnionMembership to determine if there is an inference rule relevant to that target Otherwise, yeah, I don’t see any other solution. We will never have inference on things like 
files
targets, so the best that we can do is assume it depends on every other file in the original base target.
w
because to get uniformity, you need to run for the subtargets if that is what was requested.
h
What do you mean?
w
Otherwise, yeah, I don’t see any other solution. We will never have inference on things like 
files
 targets, so the best that we can do is assume it depends on every other file in the original base target.
so, actually: i responded to this on your other issue: i don’t think that this is the case. we will probably never infer a dep into a files target
and if someone had a literal dep into a files target (or if that was the result of a spec match), then we could match exactly that file just fine, i think.
h
If you have a files target owning two JSON files, what should 
./pants dependencies f1.json
show? I argue 
f2.json
And this is btw a big reason I think we should consider reverting the file arguments -> generated subtargets thing
Only have “file level dependencies”
w
What do you mean?
@hundreds-father-404: just agreeing with the issue in this thread. if i run 
./pants test $file
, it’s dependencies need to be present, even without inference
👍 1
@hundreds-father-404: re: 
dependencies f1.json
: i think that i disagree, but not strongly… ie, i think that it would be fine either way
so i wonder if “what dependencies subtargets get when they split” is a target-type by target-type consideration
or whether we should just consistently do it. that might be fine too.
👍 1
@hundreds-father-404: there is a big consideration present in this thread, which is that “how many Addresses CLI specs expand to” is different from “what resolving an Address gets you”
Specs can expand to 0 or more Addresses.
but an an Address gets you exactly one Target
(and those statements still feel true)
having said that… if the Spec is ~literal (is expected to match one thing) it probably should. so maybe that doesn’t add any flexibility.
h
but an an Address gets you exactly one Target
I would love to keep this a thing. It makes it much easier to reason through how Pants internals work.
I think you’re commenting on my proposal to no longer use generated subtargets with file args, right? Let me collect my thoughts there and post a new thread. It solves my biggest concerns I think with generated subtargets.
w
um. i think that file args should continue to use generated subtargets if possible.
sorry. i was commenting on ways to reasonably preserve that.
one interesting example is 
./pants list $file
. yes, that’s how you “find the owning target” today, but arguably, that is a misfeature.
in a world with file Addresses, that should pretty clearly emit 
$file
as the output address.
(and iirc, Daniel referred to 
./pants list $file
giving you the owning target as potentially problematic on the thread where we introduced it… i don’t think that it was the wrong decision, because holy mackerel has it been useful)
h
Any ideas how you would find it in this brave new world? I was thinking 
./pants list --owner
or 
./pants list --original
maybe
w
@hundreds-father-404: see my respond to the ticket yesterday… 
./pants dependees
would do it.
(assuming that base targets depend on file/sub targets)
h
(assuming that base targets depend on file/sub targets)
I remain skeptical to this model change. https://pantsbuild.slack.com/archives/C0D7TNJHL/p1595523264215100?thread_ts=1595463762.203000&cid=C0D7TNJHL It’s already confusing enough for rule authors to have “original targets” vs. “generated subtargets”. Adding “generated base target” to the mix makes my head spin hehe
Without the model change, 
./pants list --original
is reasonable, right? Outside of maybe a bad name?
w
It’s already confusing enough for rule authors to have “original targets” vs. “generated subtargets”. Adding “generated base target” to the mix makes my head spin hehe
this is 100% internal though… see the other thread, but afaict the only way it impacts @rule authors is that they might need to decide to “group” targets when invoking. for end users, i think that our goal is “file deps”, and making that externally consistent
@hundreds-father-404: to be clear, the model change i discussed in the other thread is basically the same as what i discussed yesterday… just that the base target doesn’t end up owning files: instead, it depends on subtargets, each of which own one
all the rest of this falls out, afaict.
(the original post in this thread is still an open question)
h
it depends on subtargets, each of which own one
It depends on subtargets in addition to any other dependencies it has. We lose any distinction between “my own files” vs. “my direct dependencies”
w
@hundreds-father-404: i mean, it would work, for sure. but if list is “show me the addresses that match the spec”, then you might think of your 
--original
as more like 
--and-direct-dependees
?
We lose any distinction between “my own files” vs. “my direct dependencies”
yes, that is true. it would be a bit like both the sources and the dependencies were actually dependencies, but with a different way of declaring them.
h
No, I am saying without your change of model. Right now, 
./pants list f.txt
will give back 
f.txt
, which is generated from 
:example
. I want an option to be able to instead have 
./pants list f.txt
output 
:example
.
w
as i said: “i mean, it would work, for sure”
(sorry, i missed how many other comments were between those lines, but yea, that was in response to your 
list --original
question)
2 Views
Open in Slack
PreviousNext
Powered by