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

witty-crayon-22786

01/07/2019, 7:50 PM
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

aloof-angle-91616

01/07/2019, 7:50 PM
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

witty-crayon-22786

01/07/2019, 7:52 PM
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

aloof-angle-91616

01/07/2019, 7:53 PM
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

average-vr-56795

01/07/2019, 7:56 PM
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

witty-crayon-22786

01/07/2019, 7:56 PM
a (somewhat wildly) tiny alternative is to warn if you don't have matches
a

average-vr-56795

01/07/2019, 7:58 PM
Do you mean "if no targets match" or "if any targets didn't match"?
w

witty-crayon-22786

01/07/2019, 7:58 PM
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

aloof-angle-91616

01/07/2019, 7:59 PM
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

witty-crayon-22786

01/07/2019, 7:59 PM
if i ask for bundles for
./pants bundle dir/::
... that's probably what i wanted (again, unless nothing was bundlable there)
a

average-vr-56795

01/07/2019, 8:00 PM
I think "no targets matched your goal" should definitely hard error.
The "some targets didn't match", I'm very torn on
a

aloof-angle-91616

01/07/2019, 8:02 PM
some more investigation into "target filtering syntax" might subvert some of these ambiguities, orthogonal to discussion about strictness
w

witty-crayon-22786

01/07/2019, 8:02 PM
the concrete reason we don't hard error now is for things like
./pants --changed... list
a

aloof-angle-91616

01/07/2019, 8:02 PM
that's the kind of thing i'm thinking of
w

witty-crayon-22786

01/07/2019, 8:02 PM
(but we could differentiate "there were no roots" from "there were roots, but a goal couldn't do anything with any of them")
a

average-vr-56795

01/07/2019, 8:02 PM
I buy that a query-like goal should be allowed to match zero targets.
a

aloof-angle-91616

01/07/2019, 8:03 PM
do we allow `@console_rule`s to declare that they can match nothing?
w

witty-crayon-22786

01/07/2019, 8:03 PM
what about
./pants --changed.. bundle
?
a

average-vr-56795

01/07/2019, 8:04 PM
I don't buy that
--changed
should be a global flag :)
a

aloof-angle-91616

01/07/2019, 8:04 PM
or
./pants --changed.. publish
too is another good example maybe
w

witty-crayon-22786

01/07/2019, 8:04 PM
@average-vr-56795: ok. but if it were any kind of query language
👍 1
a

aloof-angle-91616

01/07/2019, 8:04 PM
i think
--changed
as well as
filter
options could be merged into the same interface
a

average-vr-56795

01/07/2019, 8:04 PM
./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

witty-crayon-22786

01/07/2019, 8:05 PM
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

aloof-angle-91616

01/07/2019, 8:06 PM
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

witty-crayon-22786

01/07/2019, 8:07 PM
cc @enough-analyst-54434, @happy-kitchen-89482: i know you folks have thoughts on this one
a

aloof-angle-91616

01/07/2019, 8:07 PM
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

average-vr-56795

01/07/2019, 8:09 PM
I think having an explicit language is better than a combination of flags with unclear implications and interactions...
w

witty-crayon-22786

01/07/2019, 8:10 PM
@average-vr-56795: i think that that is fine, but orthogonal
a

average-vr-56795

01/07/2019, 8:10 PM
Fair :)
a

aloof-angle-91616

01/07/2019, 8:10 PM
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

witty-crayon-22786

01/07/2019, 8:11 PM
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

aloof-angle-91616

01/07/2019, 8:11 PM
./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

average-vr-56795

01/07/2019, 8:14 PM
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

witty-crayon-22786

01/07/2019, 8:15 PM
@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

average-vr-56795

01/07/2019, 8:17 PM
For one goal and one target-specific spec, yes. For a multi-target spec, I'm not sure.
w

witty-crayon-22786

01/07/2019, 8:17 PM
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

average-vr-56795

01/07/2019, 8:18 PM
Yeah, that would probably do the job...
Yeah, I think I could live with
--per-spec-goal-mismatch-behavior={ignore,warn,error}
w

witty-crayon-22786

01/07/2019, 8:21 PM
i'm definitely not sure how to implement this with v2. but it seems worth writing down and linking to some things, heh
a

average-vr-56795

01/07/2019, 8:22 PM
We can implement it ad hoc for now in each console_rule we make, and look for an abstraction after a few
w

witty-crayon-22786

01/07/2019, 8:23 PM
yea.
a

average-vr-56795

01/07/2019, 8:24 PM
I guess having to track the specs -> addresses map will be a little annoying... But hey, abstractions appear :)
a

aloof-angle-91616

01/07/2019, 8:26 PM
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

average-vr-56795

01/07/2019, 8:32 PM
@aloof-angle-91616 I like it :)
a

aloof-angle-91616

01/07/2019, 8:34 PM
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

happy-kitchen-89482

01/07/2019, 10:59 PM
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

average-vr-56795

01/08/2019, 9:46 AM
@happy-kitchen-89482 What if the spec is a single target address?
./pants test tests/some:binary
?
e

enough-analyst-54434

01/08/2019, 3:48 PM
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

average-vr-56795

01/08/2019, 3:56 PM
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

enough-analyst-54434

01/08/2019, 4:21 PM
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

happy-kitchen-89482

01/08/2019, 6:04 PM
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

average-vr-56795

01/08/2019, 6:05 PM
Yeah, that sounds pretty nice actually
h

happy-kitchen-89482

01/08/2019, 6:05 PM
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

witty-crayon-22786

01/08/2019, 6:09 PM
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

average-vr-56795

01/08/2019, 6:11 PM
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

witty-crayon-22786

01/08/2019, 6:12 PM
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

happy-kitchen-89482

01/08/2019, 6:28 PM
For the tty case we can print out a summary of how many goals were produced, to give a visual clue?