https://pantsbuild.org/ logo
h

hundreds-father-404

11/16/2021, 12:02 AM
Question we need to figure out before 2.8.0 is stable. What should the address for this be?
Copy code
python_source(name="app", source="app.py")
Option 1:
project:app
Option 2:
project/app.py:app
🧵
w

witty-crayon-22786

11/16/2021, 12:03 AM
h

hundreds-father-404

11/16/2021, 12:04 AM
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
w

witty-crayon-22786

11/16/2021, 12:05 AM
@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
h

hundreds-father-404

11/16/2021, 12:05 AM
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
w

witty-crayon-22786

11/16/2021, 12:07 AM
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.
👍 1
h

hundreds-father-404

11/16/2021, 12:08 AM
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
w

witty-crayon-22786

11/16/2021, 12:09 AM
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.
h

hundreds-father-404

11/16/2021, 12:09 AM
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
?
Oh, you mean because we can change generated
python_source
targets to also use generated address syntax, right? Good point! Okay, file addresses are irrelevant here then. Yay!
w

witty-crayon-22786

11/16/2021, 12:10 AM
right… and we would, for consistency
1
h

hundreds-father-404

11/16/2021, 12:11 AM
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
?
w

witty-crayon-22786

11/16/2021, 12:12 AM
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
hm. but that would prevent conversion to a BUILD target, which is a bit awkward.
h

hundreds-father-404

11/16/2021, 12:14 AM
what does that last comment mean?
w

witty-crayon-22786

11/16/2021, 12:15 AM
via
Address.maybe_convert_to_target_generator
1
h

hundreds-father-404

11/16/2021, 12:17 AM
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?
w

witty-crayon-22786

11/16/2021, 12:18 AM
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
@hundreds-father-404: another way to resolve this in the short term would be to not actually expose the single-file targets yet. they’re a new feature
h

hundreds-father-404

11/16/2021, 12:21 AM
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
w

witty-crayon-22786

11/16/2021, 12:22 AM
yea. until the internal split matures some more
h

hundreds-father-404

11/16/2021, 12:26 AM
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
I think I'd rather lean into the original question of what to do with the address of
python_source
. That seems to be the only fundamental issue, right? You've pointed out file addresses are irrelevant
w

witty-crayon-22786

11/16/2021, 12:30 AM
For example, I don’t know how we’d rewrite the help message of 
python_test_utils
to suggest using a
python_sources
target?
h

hundreds-father-404

11/16/2021, 12:31 AM
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
w

witty-crayon-22786

11/16/2021, 12:31 AM
sure. i think the
python_source
bit is orthogonal to that.
but anyway: as to the actual syntax: it really does seem clear to me that: https://pantsbuild.slack.com/archives/C0D7TNJHL/p1637021276477700?thread_ts=1637020967.476600&cid=C0D7TNJHL
h

hundreds-father-404

11/16/2021, 12:33 AM
@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?
h

happy-kitchen-89482

11/16/2021, 12:41 AM
On balance I think it should be 2)
project/app.py:app
But how common will it be to explicitly declare a
python_source
anyway?
2
h

hundreds-father-404

11/16/2021, 12:43 AM
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
In fact, I only realized last night that
python_source
crashes Pants with dep inference, meaning we haven't used the target in the ~3 weeks it's landed till now. And I realized the much better fix was to use a
python_sources
target with
overrides
w

witty-crayon-22786

11/16/2021, 12:46 AM
yea. it seems like they *should exist for conceptual clarity and completeness sake, but overrides will always be shorter
1
👆 1
💯 1
h

hundreds-father-404

11/16/2021, 12:47 AM
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"
w

witty-crayon-22786

11/16/2021, 12:47 AM
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.
h

hundreds-father-404

11/16/2021, 12:49 AM
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
So...are we okay with keeping the current semantics and then this? 1. Rewriting Targets page again to lean into target generators even more 2. Maybe updating
help
messages for atom targets to more explicitly recommend target generators 3. Land https://github.com/pantsbuild/pants/pull/13627 as a bug fix
w

witty-crayon-22786

11/16/2021, 12:51 AM
yes
🙌 1
c

curved-television-6568

11/16/2021, 6:13 AM
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.
e

enough-analyst-54434

11/16/2021, 12:22 PM
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.
👍 1
c

curved-television-6568

11/16/2021, 12:28 PM
Oh, I’ve got something backwards then.. thought that you would get an error if a file was owned by more than one target.
Ah, perhaps that’s just the case under certain circumstances..?
e

enough-analyst-54434

11/16/2021, 12:31 PM
Yeah. The only one I can think of is
package
for python distributions.
I had an experimental design back in the day similar to overrides that made single ownership a law but we did not go that route when implementing the v2 target API.
c

curved-television-6568

11/16/2021, 12:34 PM
Ah, right. Yea that’s probably it. It didn’t really stick in my mind the scope of that.. thanks.
e

enough-analyst-54434

11/16/2021, 12:37 PM
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.
1
👍 2
w

witty-crayon-22786

11/16/2021, 5:03 PM
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
👍 1
it will render for tests, since they won’t have the default name in the directory (usually
:tests
)