https://pantsbuild.org/ logo
#development
Title
# development
w

witty-crayon-22786

01/23/2020, 11:11 PM
@happy-kitchen-89482: i feel pretty strongly about https://github.com/pantsbuild/pants/pull/8985#issuecomment-577865695 ... i think you'll remember that we discussed this when the 5 toolchain folks visited
--owner-of
is popular, for good reason... it allows for the kind of syntax you're proposing
people want to be able to run the tests for a file, we know that. but how that is implemented for each test runner is very, very runner specific, and it's easy to imagine language test runners that wouldn't support it
and finally, i don't think that there are any implementation blockers for doing the thing i'm proposing. i think that it would probably make the patch smaller, if anything.
(...not sure about that. but if it doesn't, it's because there are potential pitfalls integrating owner-of with FilesystemSpecs... which is exactly the kind of thing we'd want to figure out sooner rather than later.)
cc @hundreds-father-404 ^
the "meshing of implementations" is less about v1 vs v2, and more about filesystemspecs vs owner-of
h

happy-kitchen-89482

01/23/2020, 11:31 PM
Hmm, there is still a pretty big distinction, in that
--owner-of
implies that you're thinking in the world of targets, and filesystem specs imply that you're thinking in the world of files.
👍 1
The underlying source of disagreement here, I think, is about optimizing for the smooth transition of existing users, vs. the initial adoption curve, and overall experience, for new users. You're understandably focused on the former, we are very focused on the latter.
h

hundreds-father-404

01/23/2020, 11:33 PM
Shouldn’t make the diff smaller - the PR’s diff is almost entirely from introducing
FilesystemSpec
,
FilesystemSpecs
, and
Specs
, then updating all the call sites to use the new names + updating
cmd_line_spec_parser.py
to resolve between the two types of specs. We need those changes no matter what Also, we can always decide to deprecate
--owner-of
in a followup that backports fs specs. While this PR adds messages saying fs specs don’t work with V1, those messages may be easily removed just as easily as they were added. I’m hoping to land the PR independent of the
--owner-of
discussion so that it’s easier to work on followup PRs, like getting FS specs to emulate
--owner-of
. Right now, FS specs only work with the toy examples of
validate
and
cloc
. But, this PR lays the foundation to do more powerful things
h

happy-kitchen-89482

01/23/2020, 11:34 PM
I may be misunderstanding something, but this change doesn't affect v1 at all I think? I.e., filesystem specs don't work in v1 and
--owner-of
continues to work as usual, so this change only affects v2 users, and so seems like something we can continue to debate?
👍 1
w

witty-crayon-22786

01/23/2020, 11:34 PM
the "world of files" does not exist except for in
cloc
and
validate
. i do not think it is worth biasing toward.
h

happy-kitchen-89482

01/23/2020, 11:34 PM
Or am I missing the intricacy of mixed v1/v2 mode? Is that the concern?
So I think in the eye of a user not already inclined towards thinking of targets, the "world of files" exists for everything. For them targets are the weird thing, not files...
w

witty-crayon-22786

01/23/2020, 11:36 PM
i recognize that
buildgen
is important as well, but you have yourself pointed out that targets should be secondary: so contorting to support a goal that shouldn't be necessary to run manually (because dep inference) isn't great
h

happy-kitchen-89482

01/23/2020, 11:36 PM
cloc
and
validate
happen to not require targets at all in order to function, but there is a significant difference between "targets as background config" and "targets as the unit of CLI interaction"
👍 1
So for a user
./pants test path/to/file_test.py
is very natural
w

witty-crayon-22786

01/23/2020, 11:37 PM
no one is arguing for "targets as the unit of CLI interaction"
we are in violent agreement about that.
💯 1
h

happy-kitchen-89482

01/23/2020, 11:37 PM
Then I am confused
w

witty-crayon-22786

01/23/2020, 11:37 PM
you can implement very little without targets
h

happy-kitchen-89482

01/23/2020, 11:38 PM
Not necessarily true with dep inference
w

witty-crayon-22786

01/23/2020, 11:38 PM
meaning:
--owner-of
and FilesystemSpecs are intimately related
h

happy-kitchen-89482

01/23/2020, 11:38 PM
But think of targets as config
w

witty-crayon-22786

01/23/2020, 11:38 PM
@happy-kitchen-89482: dependency inference would still infer the target given a file
h

happy-kitchen-89482

01/23/2020, 11:39 PM
OK so that's what I'm not sure about.
--owner-of
says "there is this thing called a target, that is the unit of CLI interaction, we happen to have a way of mapping files back to targets, but your operations are happening at the target level", whereas filesystem specs say "operations happen at the file level, they may get some config from these things called targets"
That feels very different to me
w

witty-crayon-22786

01/23/2020, 11:40 PM
https://pantsbuild.slack.com/archives/C0D7TNJHL/p1579822484021600?thread_ts=1579821109.019400&cid=C0D7TNJHL my point is that FilesystemSpecs should deprecate
--owner-of
. if they don't, then we are asking for confusion and pain.
h

happy-kitchen-89482

01/23/2020, 11:40 PM
Right, but in v1 I don't see a way to do that with the same semantics as v2 (i.e., that the operations are at file level)
👍 1
--owner-of
is not that
w

witty-crayon-22786

01/23/2020, 11:40 PM
i described it on the ticket.
h

happy-kitchen-89482

01/23/2020, 11:41 PM
operations are still at the target level
w

witty-crayon-22786

01/23/2020, 11:41 PM
they always will be.
as you said: the target is the config
h

happy-kitchen-89482

01/23/2020, 11:41 PM
OK that's what we're disagreeing on I think
w

witty-crayon-22786

01/23/2020, 11:41 PM
you cannot run the test without config
if you did, then you are still settling on a "default" config
h

happy-kitchen-89482

01/23/2020, 11:42 PM
Right, but you can run it just on the file you were asked to run it on, not the other files that happen to share the same config
👍 1
And I think that's generally true
w

witty-crayon-22786

01/23/2020, 11:42 PM
yep. but that is just an implementation detail.
i described exactly how to have the runner filter.
think of it this way:
the difference we're talking about is between goals exploding if they do not* have explicit support for operating on files baked in
...and having filesystem specs work for all goals that only support targets, while being able to also step in and layer in the more specific behaviour that a test runner would to filter down to one file
because most test runners can (not even all).
and certainly not all goals will be able to operate on files.
in those cases, you would blow up on the command line: "sorry, you gave me a file! i only understand targets"
h

happy-kitchen-89482

01/23/2020, 11:46 PM
That's the part I'm not sure I agree with.
What's an example?
w

witty-crayon-22786

01/23/2020, 11:46 PM
any target or goal for which there is required config.
binary, bundle, etc.
publish
h

hundreds-father-404

01/23/2020, 11:47 PM
in those cases, you would blow up on the command line: “sorry, you gave me a file! i only understand targets”
In this case, the fs spec would act like
--owner-of
and map back to the target
h

happy-kitchen-89482

01/23/2020, 11:48 PM
Wait wait
w

witty-crayon-22786

01/23/2020, 11:48 PM
./pants publish path/to/some/file.py
# "Exception: this is a goal that can only understand targets"
h

happy-kitchen-89482

01/23/2020, 11:48 PM
It's not "sorry you gave me a file I only understand targets", it's "I need some config in order to operate on this file"
w

witty-crayon-22786

01/23/2020, 11:48 PM
h

happy-kitchen-89482

01/23/2020, 11:48 PM
I know that seems like a small distinction, but it's conceptually large
w

witty-crayon-22786

01/23/2020, 11:49 PM
@happy-kitchen-89482: in terms of the wording of the error message...?
i'm suggesting that there should not be an error message.
because it's not necessary.
h

happy-kitchen-89482

01/23/2020, 11:49 PM
In terms of the expected behavior. For example
./pants binary path/to/file_with_an_entrypoint.py
is pretty unambiguous
So we should build a binary for that thing, there may not need to be a
python_binary
target for it at all.
w

witty-crayon-22786

01/23/2020, 11:50 PM
i don't think so.
h

happy-kitchen-89482

01/23/2020, 11:50 PM
It might be embedded in a
python_library
We can get its deps from there
w

witty-crayon-22786

01/23/2020, 11:52 PM
and how would a goal actually do that?
h

happy-kitchen-89482

01/23/2020, 11:52 PM
I mean I'm fine deprecating
--owner-of
in favor of filesystem specs if there's a way to do that neatly in v1 (and it sounds like you're saying there is), but there will still be a discontinuity between how that behaves in v1 and v2, or at least how I'm envisioning v2 working... 🙂
w

witty-crayon-22786

01/23/2020, 11:52 PM
by having two implementations, depending on whether it got a file or a target?
@happy-kitchen-89482: i don't think we're disagreeing about how it works from a user perspective.
h

happy-kitchen-89482

01/23/2020, 11:53 PM
We do seem to be going around in circles, which can be a sign of heated agreement...
So what, if anything, are we disagreeing on?
w

witty-crayon-22786

01/23/2020, 11:54 PM
i'm saying that if someone did ``./pants binary path/to/file_with_an_entrypoint.py`` and you wanted that to work, you would either 1) have two implementations of the goal or special handling for files vs targets, 2) infer (possibly by generating it in memory) the relevant target to operate on
the latter (2) is
--owner-of
.
h

happy-kitchen-89482

01/23/2020, 11:55 PM
I mean, wouldn't we just need to map a file to its owning target if such a target exists
The target can be an optional place for config related to the file (say, python interpreter constraints, dependencies, whatever)
w

witty-crayon-22786

01/23/2020, 11:55 PM
if you wanted to work without such a target existing, you would still need to infer a target.
h

happy-kitchen-89482

01/23/2020, 11:56 PM
But the user interaction would always be files
usually, anyway
w

witty-crayon-22786

01/23/2020, 11:56 PM
so perhaps a useful question is: what would you like to happen in the
binary $file
case if there was a target owning the file?
you can't ignore the target, i think. because that is how someone specified the config
h

happy-kitchen-89482

01/23/2020, 11:57 PM
"take relevant config from that target, if it exists, use sensible defaults otherwise"
right, I'm just saying that it need not exist
w

witty-crayon-22786

01/23/2020, 11:57 PM
ok. that's
--owner-of
then.
--owner-of
plus dependency inference.
h

happy-kitchen-89482

01/23/2020, 11:58 PM
--owner-of
requires a target to exist, no?
w

witty-crayon-22786

01/23/2020, 11:58 PM
in v1 it does, yea.
(currently)
@happy-kitchen-89482: again, the argument is that "a target always exists"
if it doesn't, you infer one... and voila, it exists
h

happy-kitchen-89482

01/24/2020, 12:00 AM
OK, this is in v2 target land where targets really are just strongly-typed config?
That I can wrap my brain around
But the difference remains when a target has multiple files in it
so maybe
binary
is not the best example to use
w

witty-crayon-22786

01/24/2020, 12:01 AM
my feeling is that if
binary $file
, you really have to attempt to find the owning target on disk first
otherwise you'd ignore any config that was specified nearby in favor of ... doing the most literal thing? even if its the wrong thing
finally: about
validate
,
buildgen
,
cloc
: they should be the special cases (in my mind) that don't even ask for addresses, and just directly consume filespecs
👍 1
h

happy-kitchen-89482

01/24/2020, 12:04 AM
sure, but the user did not mean "act on this target", they meant "act on this file". For example, what if the entrypoint file is just one of many files in a
python_library
? All the useful config is on that library target, and the user is directly providing the entrypoint, which is usually the only salient thing a
python_binary
provides that a
python_library
does not. So a
python_binary
need not exist at all, not even by inference.
w

witty-crayon-22786

01/24/2020, 12:04 AM
@happy-kitchen-89482: that is "the most literal thing", but it is probably the wrong thing.
h

happy-kitchen-89482

01/24/2020, 12:05 AM
How so?
h

hundreds-father-404

01/24/2020, 12:05 AM
finally: about validate , buildgen , cloc : they should be the special cases (in my mind) that don’t even ask for addresses, and just directly consume filespecs
Ftr, that’s what I’m proposing in the design doc. I could see us eventually going to this future where targets don’t exist and/or are much less relevant, but for now every goal but
validate
,
buildgen
, and
cloc
needs to map back to an owning target. The edge case is goals that should map back to owning targets + preserve the files specified to be more precise in how they operate, like
test
and
lint
. This is handled by the
TargetWithSources
type in the proposal
👍 1
w

witty-crayon-22786

01/24/2020, 12:05 AM
@happy-kitchen-89482: i don't want you to ignore my
python_tests
config if i type
./pants test $file
it's no less necessary because i used syntax sugar
h

happy-kitchen-89482

01/24/2020, 12:06 AM
no, but we can do the right thing if it doesn't exist.
w

witty-crayon-22786

01/24/2020, 12:06 AM
@hundreds-father-404: ok. if that's the case, then deprecating owner-of should be straightforward.
@happy-kitchen-89482: yep, violent agreement there.
h

hundreds-father-404

01/24/2020, 12:09 AM
if that’s the case, then deprecating owner-of should be straightforward.
if we were okay with
./pants test foo.py
meaning in V2 to try to only run over
foo.py
(but still grabbing metadata from the owning target), whereas in V1 it always no matter what means the owning target. I find the difference in those semantics very confusing, personally, but am willing to trust others’ judgment on this (Originally, the other edge case I was concerned about is that there’s no way to port
SourcesSnapshot
, i.e. get V1 goals to work on files that have no owners. I realized that’s irrelevant. Only
cloc
,
validate
, and
buildgen
behave this way, and they already are implemented in V2)
w

witty-crayon-22786

01/24/2020, 12:10 AM
but still grabbing metadata from the owning target
i think that you have to. see two or three above.
h

happy-kitchen-89482

01/24/2020, 12:10 AM
My point is that deprecating v1 --owner-of in favor of filesystem specs in v1 means that filesystem specs will have, in general, different semantics in v1 vs. v2. Some things will fail in v1 but succeed in v2 (because we can be sloppier about missing targets), and e.g.,
./pants test path/to/foo_test.py
will run just the tests in that file in v2, but all the tests in that file's owning target in v1. So it sounds like you're comfortable with those differences?
w

witty-crayon-22786

01/24/2020, 12:10 AM
--owner-of
is not v1 only, it works with both v1 and v2
this has very little to do with v1 and v2
So it sounds like you're comfortable with those differences?
yes.
h

hundreds-father-404

01/24/2020, 12:11 AM
> but still grabbing metadata from the owning target
i think that you have to. see two or three above.
Right. I’m not debating resolving the owning target at all. The conversation took a turn towards that for a bit, but I view that part of the conservation as “this might be a possible future where we aren’t as wedded to targets.” For now, I think that’s the wrong discussion
w

witty-crayon-22786

01/24/2020, 12:12 AM
k
(even in a world where we're "not wedded to targets", IMO it's because we generating a default in memory, not because we don't go looking for your config/BUILD if it exists)
👍 2
h

happy-kitchen-89482

01/24/2020, 12:13 AM
We always use your target if it exists
We are in heated agreement on that
w

witty-crayon-22786

01/24/2020, 12:13 AM
ok, then we can deprecate
--owner-of
with this mechanism.
h

hundreds-father-404

01/24/2020, 12:15 AM
> So it sounds like you’re comfortable with those differences?
yes.
This is the heart of the disagreement in my eyes, then. If we were okay with the different semantics between V1 vs. V2 goals—where V2 implementations were more powerful to attempt to only operate on the specified files whereas V1 is more imprecise and operates over teh whole owning target—then, yes, I agree with deprecating
--owner-of
h

happy-kitchen-89482

01/24/2020, 12:15 AM
With the caveat of some differences in behavior between v1 and v2 when using filespecs (which would not be different with
--owner-of
)
👍 1
w

witty-crayon-22786

01/24/2020, 12:15 AM
right
h

happy-kitchen-89482

01/24/2020, 12:15 AM
OK, so it sounds like we basically agree after all
As long as there is a sensible way to implement filesystem specs for v1, which it sounds like there is. That part I have not looked into at all.
Sensible and not many weeks of work... 🙂
w

witty-crayon-22786

01/24/2020, 12:16 AM
i do think that the "
./pants $goal $file
always attempting to actually find the owning target before it decides to generate one" bit is an important takeaway from this discussion
h

hundreds-father-404

01/24/2020, 12:17 AM
Hm I do now see some concerns why
./pants fmt2 foo.py
works but
./pants fmt foo.py
fails. I think you’re right, Stu, that that confusion is even worse than the confusion of why
./pants fmt2 foo.py
only runs on
foo.py
whereas
./pants fmt foo.py
runs over the whole target
always attempting to actually find the owning target before it decides to generate one” bit is an important takeaway from this discussion
*For any goal which does not request
SourcesSnapshot
/ needs target info
w

witty-crayon-22786

01/24/2020, 12:18 AM
*For any goal which does not request
SourcesSnapshot
which is
cloc
/
buildgen
/
validate
, right?
👍 1
er. those are the ones that do request
SourcesSnapshot
👍 1
h

happy-kitchen-89482

01/24/2020, 12:18 AM
Yes, I think we all agree that if there is a target that owns a file, that target provides config for the goal. In v1 that if is always true. In v2 we can be more forgiving, but naturally if you provide explicit target info we use that, same as today.
w

witty-crayon-22786

01/24/2020, 12:18 AM
k
h

hundreds-father-404

01/24/2020, 12:18 AM
As long as there is a sensible way to implement filesystem specs for v1, which it sounds like there is. That part I have not looked into at all.
Yep. My next task was getting fs specs to work like
--owner-of
h

happy-kitchen-89482

01/24/2020, 12:19 AM
We can be more forgiving in the future, anyway
👍 1
w

witty-crayon-22786

01/24/2020, 12:19 AM
yep.
thanks folks 😃
😁 1
h

hundreds-father-404

01/24/2020, 12:20 AM
@witty-crayon-22786 I’m hoping to land https://github.com/pantsbuild/pants/pull/8985 even though it currently does error if you try to use FS specs with V1, because it lays the foundation for that next feature. Notably, the PR will also error if you try to use FS specs with any goal other than
cloc2
and
validate
We’d remove those error messages as soon as that lands. But, for now, the error message is technically correct
h

happy-kitchen-89482

01/24/2020, 12:21 AM
To be super-duper clear, when I say "no targets" to me that's just shorthand for "targets are not (or at least need not) be the unit of CLI interaction", not "targets shouldn't exist"... And it sounds like we mostly agree on this.
👍 1
w

witty-crayon-22786

01/24/2020, 12:21 AM
Yep
@hundreds-father-404: fine with that. Will review shortly.
h

hundreds-father-404

01/24/2020, 12:22 AM
I’ll update the PR description to explain the plan of deprecating
--owner-of
. Will put up the RunTracker change in a second too
w

witty-crayon-22786

01/24/2020, 12:24 AM
thanks much
shipit + comment
❤️ 1
thanks
❤️ 1