An open question on what we want for CLI specs: when you run

./pants test src/py

, do you expect it to only run tests in

src/py

? 🧵

It's an open question when you run

./pants test src/py

how to handle matching a

python_tests

target defined in

src/py

. Does the user expect this alias mechanism to apply there too? If so, that means

./pants test src/py

might actually run over dozens of directories! Depends if the

python_tests

target is generating into a bunch of subdirectories

(as mentioned elsewhere: i think aliases are useful, and should work consistently… will leave it at that.)

I agree they're useful, I backtracked from my proposal in https://github.com/pantsbuild/pants/issues/13086 to get rid of aliases, mostly due to your feedback. (Thank you for that!) I also fully buy in to expanding aliases for address literals like

./pants test src/py:tests

(when it's a build goal), and with

dependencies

My hesitation is specifically whether it's useful vs. confusing to expand aliases with a directory spec

it’s confusing not to, imo. if

./pants peek src/py

will show the alias,

./pants test src/py

should run it.

Fwit, I offer this design principle in the doc. Doesn't mean we need to agree with it, but my mental model

Pants should do what the user expects, even if that means behavior is slightly different between goals (project introspection vs. build goals).

Does the user expect this alias mechanism to apply there too?

If I run

./pants test src/py

and have a

python_tests(sources=["**/*_test.py"])

in there, I’d expect to run the tests for all those sources.

Okay, thanks Andreas! (Personally, I would expect to run

./pants test src/py:tests

or

./pants test src/py::

to say "all the tests")

Hmm… but that’s perhaps because I’m getting used to targets, rather than files now… 😛

I learned the phrase the Curse of Knowledge only this past month: https://en.wikipedia.org/wiki/Curse_of_knowledge it's a thing I'm trying to be more aware of for me

I didn’t know it was as a thing, but I have that issue a lot at work… 😄

another important example is `*_binary`: if i have only a

*_binary

in a directory and no files, it should match. and then not matching for target types that own files would be inconsistent

a bit of confusion to allow either paths OR spec addresses on the command line…

Part of https://docs.google.com/document/d/1WWQM-X6kHoSCKwItqf61NiKFWNSlpnTC5QNu3ul9RDk/edit is that address specs vs. file specs is no longer as relevant as it used to be I at first proposed

dir::

(addresses) vs

dir/...

(file semantics), then we generally agreed it's confusing for the user to have to know the nuances of

dir:

vs

dir

, and

dir::

vs

dir/...

. 95% of Pants users will likely never read the majority of ours, particularly users who aren't on our Slack and only use Pants because their org uses it. Not worth the subtly

a related example from v1 (which is a bit of a tangent here, but bear with me) was to define a

target

alias with a short name that referred to something deeper in the directory structure, so that you could do

./pants run shortname

,

./pants test shortname

, etc. for that to work, it’s necessary to expand the alias… but because we didn’t have a native facility for that, it got hacked into a bunch of goals

Hmm… perhaps this have been considered and discarded before, but given how hard it is to come to reason with all this, I don’t really think there is one “right way” that will prove intuitive for users (it shouldn’t be that hard to find, if so). So with that, I’m pondering on the feasibility of splitting the two concepts of path specs vs target specs on the command line, so we don’t have to treat them the same (as we can’t always tell them apart, so what the user intended must be consistent regardless of spec type, which is difficult to achieve here). So, that could be that we signal with a flag, or require a fixed prefix or some such for target specs, in order to separate them from path specs. Example sketch

./pants test //src/py/utils  # interpret as target address
./pants test src/py/utils  # interpret as path spec
./pants test src/py/utils:tests  # still path spec, if there is not a dir "utils:tests" it is an error
./pants test --targets src/py/utils:test  # all specs treated as target address specs due to flag

I’m not really up to speed on all the nuances to be able to reason if this will improve the situation considerably or not.. but thought worth exploring (or discarding)… 😉 (I’ll be afk now, time to put some kids to bed..)

@curved-television-6568: yea, right now both

*

wildcard matching and

::

matching exist: the latter is intended to match targets, and the former to match files, regardless of whether there are associated targets. but whether they should be merged (and whether there should be a filtering flag like the one you mentioned) is an open question.

if we stop matching the target in that case, it’s a more significant behavior change

Changing the semantics of

dir

being

dir:dir

in particular can be feature gated to not break the deprecation policy as badly, demonstrated by https://github.com/pantsbuild/pants/pull/13219/commits/1c93cafff6d51e291596a9041a33b51966292c8f Maybe confusing that I used

./pants test src/py

in my example rather than

src/py:

. I was using

src/py

because that's what users will be typing once we finish completely deprecating it beign shorthand for

src/py:py

hm. didn’t realize that that had already landed. but i don’t think that that deprecation is necessary if aliases are matched, since it’s strictly more being included.

Purely the refactor landed. We eagerly convert

DirLiteralSpec

into

AddressLiteralSpec

. I reverted all breaking changes from it

I think

/pants test src/py

to the user means

/pants test src/py/test_*.py

(or whatever their test file naming conventions are)

and if the only thing in that directory is a target that is an alias for files?

Then nothing, if the files themselves are not in that directory

so you’re saying that

./pants run src/py

should run a binary target, even if it has no files, but

./pants test src/py

should ignore a test target?

*a tests target generator. Which solely exists for boilerplate reduction

A binary target can be referenced by its entry point file

@happy-kitchen-89482: and if the file isn’t in that directory?

And users will typically want to do that

Pants run src/py doesn’t make sense, really…

That is, assuming we're agreed that

src/py

means "just the dir, not its subdirs", and I think we all agree on that at this point?

@curved-television-6568: currently it will run the default target in that directory

Right, and I am fine removing that ability

It is a little more nuanced than "only targets that have files". A

go_binary

is file-less. But it does reside in the directory where it is defined, so

./pants run src/go

will run any

go_binary

targets defined there

or

./pants run src/py:tgt

if you want to address the target itself

but… why? why would we not match aliases? this just seems like removing something useful and consistent unnecessarily.

Because it's confusing

to match targets, rather than only files?

yes

why would we not match aliases?

For a user who doesn't really care about targets and BUILD files or even Pants, it's confusing that

./pants test src/py

might run

src/py/foo

,

src/py/foo/bar

, *

Are there use-cases where there really is no file and you want to act on multiple targets? There might be

and

./pants peek src/py

and

./pants list src/py

shouldn’t render those targets either, you’re saying?

Just had a thought: What if addresses with colons always match targets (so

src/py:

and

src/py::

match all targets in the dir/subdirs), but addresses without colons always match files/dirs, and if you want to do so recursively you use

-r

?

Worth considering. I like

-r

more than

dir/...

So

./pants -r peek src/py

and

./pants peek src/py::

are different things

And then I really like that that fixes the weirdness of how we normally want

./pants tailor

to be recursive. We'd "fix" it to not recurse by default, and you use

./pants -r tailor dir

for recursion

So in that example,

/pants peek src/py

doesn't render those targets, but

/pants peek src/py:

does

A limitation of

-r

is that you can't have some dirs be recursive and others be non-recursive, whereas

test dir another_dir::

would handle that. But probably fine

-r

doesn’t let you control recursion on a spec by spec basis

OK

Less of a dealbreaker imo than the clunkiness of

dir/... another-dir/...

i think that the scope of this discussion has gotten too large. i would suggest that we table everything except fixing the fact that

:

and

::

don’t match generated targets, and defer the rest until after 2.8.x. the actual bug that exists right now is that

./pants peek ::

and

./pants list ::

don’t show you all targets, and that’s really the only thing that imo MUST be fixed for 2.8.x

Oh! @happy-kitchen-89482 I really like that this solves the problem from yesterday of what to do with

SourcesSnapshot

(

count-loc

and

validate

). If you use

:

and

::

, that means only operate on sources owned by targets, like today If you use

dir

or

-r dir

, that means all sources regardless of targets

-r

might be a replacement for

*

syntax, but i feel like we’ve gone totally off the rails to be discussing a third globbing/recursion semantics for what started as a bugfix so that Asher can get a graph of the whole repo with

./pants peek ::

Our histories are different there: for me, the motivation was always that Tom and me found specs semantics to be extremely confusing for Go, enough to block it being announcable

the more the scope increases here, the more ill advised it is to be trying to get it in before 2.8.0. there is an immediate bug to be fixed.

Fair, I don't want to squeeze this into 2.8.0 until there is some consensus

I agree with punting on changing

dir

to be shorthand for

dir:

, and punting on it meaning something else. I think it's important we get directory specs right. (And adding dir specs would prompt a bunch of other changes I'd feel compelled we'd need to do) I'm putting up now the "bug fix" that gets

:

and

::

behaving how Stu believes they should. I believe it's a big step forward, even if I'd like to improve it even more (what this thread is about) I don't agree with reverting the Go changes. It's experimental, so we're not committing to things. Thanks to

./pants tailor

, it should be easy for Pants 2.9 to stop doing the target generation and instruct alpha users that they need to now use

./pants tailor

. The docs won't mention generated address syntax, they'll only talk about using

dir:

and

dir::

to run, which this pending bug fix mostly gets right

But I also still believe 

./pants test dir

 should only test 

dir/*

 and not expand target generators. (I also  think 

./pants package dir

 should package all file-less targets like 

go_binary

, which is where this gets tricky. 

dir

 is not strictly shorthand for 

'dir/*'

, it would include file-less targets)

yes, that is where things get tricky. because it is inconsistent for some goals to match targets and others not to.

Not to restart another bike shed...but fyi I think we actually won't need something like

dir/...

, even if we decide for

dir

and

dir:

to have different semantics around target generators To recap, I'm thinking that

dir:

should replace a target generator with all its generated targets, whereas

dir

should not and only run on targets resident to that directory. If we're recursing into subdirs, I don't think that really matters whether we replace target generators or not. We'll already end up globbing over every target generated by it because those targets must be in subdirectories. The end result should be the same That's a good thing! We only need

dir

,

dir:

,

dir::

,

dir/f.py

, and

dir:tgt

. (I suspect we can under-document

dir:

in favor of

dir

also)

To recap, I’m thinking that 

dir:

 should replace a target generator with all its generated targets, whereas 

dir

 should not and only run on targets resident to that directory.

i don’t think it’s a question of “replacing” so much as “expanding”… the caller can decide whether things end up “replaced” (confusingly, by requesting either

Target

or

UnexpandedTarget

). but

Specs

calculation should either expand an alias or not, without replacing it.