https://pantsbuild.org/ logo
Join Slack
Powered by
and yea, the hairy part is what danny is referring...
# development
w
and yea, the hairy part is what danny is referring to i think. how can i "test everything that is testable" without a bunch of external work and filtering? unless we also had an override to silently ignore things.
☝️ 1
a
thanks for making this clear, hadn't considered this use case yet
the answer to this could be "unified target filtering syntax"
which also becomes more feasible in a pure v2 world
that being said it is nice to be able to 
./pants test ::
so if we want to allow that then hm
w
maybe... if it were type/rulegraph driven somehow? but that is unexplored ground i think. and saying "please run 
test
on all things for which you can run test" is sortof a tautology.
a
well. that may not be an issue because strictness then forces us to define a specific operation for the 
test
goal for all target types (with an easy way to default to no-op? not sure if that misses the point though)
i think "type/rulegraph driven" might be what i'm trying to think of too
this might go back to the ideas from "v2 products in v1 tasks" issue
a
Yeah, maybe it should just be a flag... But I kind of want the flag to be inconvenient by default, which sort of negates the point of the flag...
w
a (somewhat wildly) tiny alternative is to warn if you don't have matches
a
Do you mean "if no targets match" or "if any targets didn't match"?
w
no, if no targets match
the case that i feel like we're trying to address is: "i asked for an action on some specific target, but nothing happened"
a
i think that sounds great as a warning (maybe with an option to turn off the warning?), it allows people to generate command lines from something which just end up no-oping if they know they want that
w
if i ask for bundles for 
./pants bundle dir/::
... that's probably what i wanted (again, unless nothing was bundlable there)
a
I think "no targets matched your goal" should definitely hard error.
The "some targets didn't match", I'm very torn on
a
some more investigation into "target filtering syntax" might subvert some of these ambiguities, orthogonal to discussion about strictness
w
the concrete reason we don't hard error now is for things like 
./pants --changed... list
a
that's the kind of thing i'm thinking of
w
(but we could differentiate "there were no roots" from "there were roots, but a goal couldn't do anything with any of them")
a
I buy that a query-like goal should be allowed to match zero targets.
a
do we allow `@console_rule`s to declare that they can match nothing?
w
what about 
./pants --changed.. bundle
?
a
I don't buy that 
--changed
should be a global flag :)
a
or 
./pants --changed.. publish
too is another good example maybe
w
@average-vr-56795: ok. but if it were any kind of query language
👍 1
a
i think 
--changed
as well as 
filter
options could be merged into the same interface
a
./pants query 'changed(::)
is my answer to this problem
It doesn't take specs, so doesn't error when it didn't act on specs
w
basically, you'd be requiring that all useful queries in the query language include a filter saying something like: "only do this thing to targets for the thing can be done"... which is the tautology again
query as separate goal still requires this
a
i would strongly prefer it if we could avoid doing queries as a separate goal unless the idea is that it applies to subsequent goals, but at that point i would prefer an option still
w
cc @enough-analyst-54434, @happy-kitchen-89482: i know you folks have thoughts on this one
a
that or if we can remove the global file lock so we can do 
./pants query ... | ./pants --target-spec-file=- whatever
but i also am extremely skeptical of any query language or languages in general because people have to learn it and it becomes its own whole thing you have to maintain
a
I think having an explicit language is better than a combination of flags with unclear implications and interactions...
w
@average-vr-56795: i think that that is fine, but orthogonal
a
Fair :)
a
i think a query language has the exact same issues, except it's more unclear because it doesn't fit into the framework that already exists, and if it does have interactions with the rest of pants, they're also ad hoc
i will stop pontificating
w
i mean, it's related... apologies. but i pointed out above that whether this is a query language or not, you have the same problem of forcing people to apply filters to confirm that they really wanted to do the thing they said they wanted
a
./pants query ... | ./pants --target-spec-file=- whatever
is always going to be less efficient and less hygienic (because people will start relying on the ability to dump the target specs into a separate file, maybe pipe it through sed, etc) than doing it in the same command line
whether it's a language or a set of extra options is orthogonal though, i can make a new thread
a
I guess the question is: how confident are we we're really doing what you mean when we do what you say? And sadly, I worry we need the unfriendly behaviour by default to teach you when what you say and what you mean are different.
w
@aloof-angle-91616: there is a ticket for that discussion: https://github.com/pantsbuild/pants/issues/6501
@average-vr-56795: but is the warning/hard-error for "no matches" sufficient to catch 
./pants bundle $target
?
a
For one goal and one target-specific spec, yes. For a multi-target spec, I'm not sure.
w
or, once per spec, perhaps?
which is what we do for glob matches... you check that each literal glob matches something (i think Danny mentioned this in the other thread or somthing)
a
Yeah, that would probably do the job...
Yeah, I think I could live with 
--per-spec-goal-mismatch-behavior={ignore,warn,error}
w
i'm definitely not sure how to implement this with v2. but it seems worth writing down and linking to some things, heh
a
We can implement it ad hoc for now in each console_rule we make, and look for an abstraction after a few
w
yea.
a
I guess having to track the specs -> addresses map will be a little annoying... But hey, abstractions appear :)
a
i left a comment on #6501 about how queries can be made orthogonal to what each task does with them https://github.com/pantsbuild/pants/issues/6501#issuecomment-452069677
👍 1
a
@aloof-angle-91616 I like it :)
a
i have no concern about a query language now, 
jq
just happens to be a relevant example i didn't think of before (in addition to json also being a well-known graph-ish output format) -- i think not being too tied to any existing examples at the start is a good idea
h
Catching up on the thread now
So, “some targets didn’t match your goal” should definitely not error, or even warn. Things like 
./pants test everything/testable/under/here::
really should work, and in the way users expect.
I tend to think that even if no targets at all match, that should not be an error, although maybe a warning could be appropriate.
If there are no tests then 
./pants test everything/testable/under/here::
should probably be allowed to be vacuously true.
a
@happy-kitchen-89482 What if the spec is a single target address? 
./pants test tests/some:binary
?
e
I would propose that fails. I think this is a summary of discussion so far / what seems sane to me: Given glossary: 1. interactive: tty attached / not in a pipeline 2. explicit spec: a single explicit target address 3. goal: explicit command line rule 4. strict mode: a mode of running pants where failure to be able to produce the product of at least one goal for at least one explicit spec is an error condition Proposal: 0. Default mode in non-strict. 1. Interactive sessions with at least one explicit spec turn on strict mode by default. 2. Non-interactive sessions turn off strict mode by default. 3. You can option toggle strict mode.
a
That generally sounds good to me except for a few caveats: 1. This is a pretty subtle behaviour to be changing defaults of implicitly… (But also, I can see why all of those default changes aim to do what you mean) 2. Not sure whether strict mode should be “for at least one explicit spec” or “for at least every explicit spec”
e
Agreed on 1. I'd actually prefer always non-strict and only turned on by option, but the thread seemed to be converging on this general compromise.
h
I like John’s summary, I think it captures the user’s likely intent pretty well. A possibly simpler, way of articulating it is to slightly alter the meaning of 
foo/bar::
to be “all targets under foo/bar, if any, that are relevant to the requested goals”. And that suggests a simpler scheme: Instead of differentiating modes and interactivity and options, we can always error if 
path/to/singletarget
produces nothing, and never error if 
path/to::
produces nothing. And the user can invoke on 
path/to/singletarget::
if they want to not error in the vacuous case of a single target.
I think this helps avoid the subtleties @average-vr-56795 mentioned about changing behaviors implicitly.
a
Yeah, that sounds pretty nice actually
h
The user is always in control: If you specify single target specs, they are each expected to produce something. If you use 
::
then you have been trained to understand that this means “only relevant targets, and there may be none”.
And if there is ever a need to glob over a subtree of targets “strictly” (i.e., each one must still produce something) we can introduce new globbing syntax, e.g., 
path/to**
or something.
w
hm. i feel like even in the 
./pants $goal ::
case, you'd want a warning if your command did nothing.
(and the nice thing about warnings for this usecase is that they work the same way with/without a tty)
a
Except without a tty I’ll probably never see the warning
When your CI stops running your tests, and all you know is that your builds are really green, there’s not particularly a trigger to investigate…
w
right. but john was suggesting disabling strict mode for non-tty.
but... yea, i think overall i'm roughly on board with John's proposal.
i'm less excited about globs not being checked... it feels like it is unnecessarily lax given that the only case where it might be ok to match nothing would be in automation (ie, the non-tty case John referred to)
h
For the tty case we can print out a summary of how many goals were produced, to give a visual clue?
Open in Slack
PreviousNext
Powered by