Question we need to figure out before 2.8.0 is stable. What should the address for this be?

python_source(name="app", source="app.py")

Option 1:

project:app

Option 2:

project/app.py:app

🧵

see https://github.com/pantsbuild/pants/pull/13627 for backstory

Option 1 is what we do now. I'm partial to it because it's consistent with all other explicitly declared targets, like

pex_binary

etc It makes it really easy to write this section on addresses: https://www.pantsbuild.org/v2.8/docs/targets#target-addresses Otherwise, we need to explain how explicitly declared targets sometimes have the form

path/to:tgt

and sometimes

path/to/f.ext:tgt

@hundreds-father-404: i don’t think that’s quite accurate. you’d have to specify that “now” means “in 2.8.x”. because between 2.0 and 2.8.x, you would always see a file address

Option 2 would remove any difference between generated targets and manually declared targets for

python_source

and friends. They'll have the exact same address format

I’m partial to it because it’s consistent with all other explicitly declared targets, like 

pex_binary

 etc

i think that whether it is is explicitly declared is not what should be driving its address behavior: rather: “what is the atom”? for files, the atom is the file. for non-file targets, the atom is the target itself.

Option 2 also means that we want to stick with file address syntax (https://github.com/pantsbuild/pants/issues/12917), rather than changing

project/f.py:lib

to be

project:lib#f.py

I'm not certain I agree with that. I thinkkk I lean towards file arguments, but the main downside imo is it means we have three types of addresses: •

dir:tgt

•

dir/f.ext:tgt

•

dir:lib#tgt

The table on generated target addresses at https://www.pantsbuild.org/v2.8/docs/targets#target-generation is pretty confusing imo

also means that we want to stick with file address syntax (https://github.com/pantsbuild/pants/issues/12917), rather than changing 

project/f.py:lib

 to be 

project:lib#f.py

i don’t think that this is accurate. making the behavior consistent with other targets now does not rule out all of those targets changing in the future.

cc @happy-kitchen-89482. Maybe let's start with deciding on file address syntax, as that I think blocks what to do with

python_source

?

right… and we would, for consistency

Hmm @witty-crayon-22786 what do you think

project:app

should do in the original example? Alias for

project/app.py:app

? The CLI says

project/app.py:app

always, but you can also write

project:app

?

implementation dependent i think? but no, i don’t think that there is an alias in this case… the generator would have been the alias, and there isn’t one. the only thing that exists is the file. and the file should just have the address

project/app.py:app

what does that last comment mean?

via

Address.maybe_convert_to_target_generator

Yeah lots of weirdness like that. To calculate the Address, it's no longer enough to combine BUILD file path +

name=

. We now need to see if you subclass

SingleSourceField

and use its value to create the

Address

I honestly am not sure how we'd implement this?

imo, this differentiation is precisely what we signed up for with atom vs generator: they should be different in the type system, and so it’s not surprising that they would have different construction

So, still have the internal split, but go back to unregistering the targets and changing the

alias

to be the target generator's such that

./pants filter --target-type

and

peek

work

yea. until the internal split matures some more

Reverting the new target types is weird imo because 2.8 will still make generated targets much more of a thing, with

::

now matching generated targets in project introspection + adding

overrides

We have less of a language to describe what's going on there, whereas right now I'm really happy with the conceptual clarity these changes brought to the docs But worth considering..

For example, I don’t know how we’d rewrite the help message of 

python_test_utils

to suggest using a

python_sources

target?

to suggest using a python_sources target? (

Well, I think we would still want to use

python_test_utils

, not

python_sources

. That was a good change to make and I'm convinced we'll be happy we moved

conftest.py

out of

python_tests

- I don't want to revert that part

sure. i think the

python_source

bit is orthogonal to that.

@curved-television-6568 @happy-kitchen-89482 any thoughts on: 1. What the address for a

python_source

should be? 2. Whether we revert atom targets for Pants 2.8 or lean into that first question?

On balance I think it should be 2)

project/app.py:app

I'm expecting not common. My rewrite of the docs now says >15 times I think "Run `./pants tailor`" throughout different pages. And my rewrite of the targets page https://www.pantsbuild.org/v2.8/docs/targets recommends using target generators generally - Stu suggested a further rewrite of it to lean into that even more, which I plan to do

yea. it seems like they *should exist for conceptual clarity and completeness sake, but overrides will always be shorter

Yes, that is precisely what I intended them to be when adding them. And I'm definitely willing to add to their

help

messages something like "Generally, prefer using

python_sources

for less boilerplate"

so while i think the address syntax would be a weird wart that we would want to try to remove as the atom/generator split matures, it might not be a blocker.

that we would want to try to remove as the atom/generator split matures

I do think it will be tricky to change after a stable release, but yeah it is technically feasible to do. And we have

update-build-files

now to automate

yes

Sorry, tl;dr; but why not have just

project/app.py

for a file target? I think that tacking the target name onto the end is ugly (and superfluous as it can only belong to one target any way) and is preferable over

project:app

for file targets as that doesn’t make sense until you are well versed in the target lingo.

I agree with this sentiment but I think we necessarily allow multiple ownership and it's that corner that negates your parenthetical and torpedos that route.

Oh, I’ve got something backwards then.. thought that you would get an error if a file was owned by more than one target.

Yeah. The only one I can think of is

package

for python distributions.

Ah, right. Yea that’s probably it. It didn’t really stick in my mind the scope of that.. thanks.

It is unfortunate since you can easily have whole repos with no dual ownership that could get away with your proposal with no ambiguity, but users of that repo are still forced to wonder at and learn what the bit with the colon is all about.

when 111 is applied and the target is the default target, the

:target

doesn’t render… which in a freshly tailored well behaved repo is very very common