https://pantsbuild.org/ logo
w

witty-crayon-22786

02/04/2022, 6:48 PM
regarding parameterization: Eric’s comment yesterday highlighted that i had not considered
overrides
while finalizing the syntax with
@
first, followed by
#
. after thinking it through, i agree, and have updated the design doc to swap those back, and to add a section on the topic
🙌 1
that does raise some questions though
in particular: if the
#
syntax isn’t going to allow arbitrary syntax (which i had been thinking might be one of its usecases, with the
git
case), then does it really need to be a separate syntax from
:
? or should we preserve the model that we have via CAOFs that generated targets are still targets, without necessarily needing an association with their generator?
this is relevant, because we’re about to change all python requirements from
:req
to
#req
…although it’s already the case that
#
allows more internal syntax than
:
does… because it supports URLs for
go
.
âž• 1
h

hundreds-father-404

02/04/2022, 7:29 PM
And we use the
:generator
snippet to locate where the generated target comes from. It also enforces disambiguation naturally Otherwise we need "address-less target generators" I think: https://docs.google.com/document/d/17XvhHlx8jBn4K9qFRZPe7qaKJQjTfiKMKYRubnzWxjA/edit. Which involves lots of discussion + implementation work
w

witty-crayon-22786

02/04/2022, 7:30 PM
sortof: the alternative would be to eagerly run all generators at AddressFamily expansion time. which was in effect already what was happening for CAOFs
👍 1
h

hundreds-father-404

02/04/2022, 7:34 PM
That eager expansion happens a lot more frequently now because of the change we made for specs to match generated targets
w

witty-crayon-22786

02/04/2022, 7:34 PM
and because of inference. it will basically always happen.
👍 1
the “other syntaxes in generator names” bit remains challenging though. we probably don’t want to support arbitrary syntax in target names in general.
h

hundreds-father-404

02/04/2022, 7:37 PM
This is interesting, we should 100% block 2.10 on resolving this imo. What's interesting to me about this is that the target generator will still be addressable, unlike the "address-less target generators" thing. That was my least favorite part of the proposal, why you can sometimes do a thing and sometimes can't
w

witty-crayon-22786

02/04/2022, 7:37 PM
i suppose that it also might eventually be useful for
#
to mark “completely synthetic targets”, if we start creating targets from scratch.
👍 1
h

hundreds-father-404

02/04/2022, 7:38 PM
i suppose that it also might eventually be useful for # to mark “completely synthetic targets”, if we start creating targets from scratch.
Meaning you want to reserve
#
for a different future use?
I'm also wondering what happens to file address syntax. If this means we drop the
:tgt
suffix vs. leave it unchanged
w

witty-crayon-22786

02/04/2022, 7:39 PM
the same use, really:
#
marking a target that doesn’t actually exist in a BUILD file. so if you generated a “default” scala toolchain Target, for example, you’d generate it with a
#
address like
//#default-scala-toolchain
or something.
👍 1
file address stays, imo. it’s too natural to remove.
h

hundreds-father-404

02/04/2022, 7:42 PM
While I don't love the verbosity of
//:reqs#Django
rather than
//:Django
, I do think there's some value to being able to easily trace where something is coming from. For example,
debug_hint
with stacktraces. And then, again, avoids the problem of
//:reqs#Django
vs
//:poetry#Django
in the same repo. Which your design doc points out is an actual real issue, that macro authors etc need to reason about address disambiguation
w

witty-crayon-22786

02/04/2022, 7:42 PM
also, i think that given its position in the address, the limitations on syntax inside of
#
aren’t really that bad… the only character you’d be forced to escape would be
@
âž• 1
ok. maybe we should keep it then. sorry, my knee jerk reaction to complexity here is to try and delete things, heh
👏 2
h

hundreds-father-404

02/04/2022, 7:44 PM
That's okay, that's my reaction too! I don't love how complex addresses have become, but I also do really think it's justified / a necessary evil. Less offensive than the alternatives
c

curved-television-6568

02/04/2022, 7:46 PM
Agree it’s a healthy reaction, and always good to take a moment to reevaluate also “known truths”.
âž• 1
w

witty-crayon-22786

02/04/2022, 8:26 PM
…argh. i’m spinning on this. looking more into the implementation of supporting
overrides={'t.py': {"resolve": parameterize(..)}
, there are challenges: https://docs.google.com/document/d/1mzZWnXiE6OkgMH_Dm7WeA3ck9veusQmr_c6GWPb_tsQ/edit?disco=AAAAVIAE4Ds
while it would be great to be able to do it, i’m not sure that we can without either 1. making target generators “not real targets”… i.e., not giving them fields at all 2. having two levels of parameterization (
src/python:example@resolve=one#generated@resolve=two
)
h

hundreds-father-404

02/04/2022, 8:30 PM
making target generators “not real targets”… i.e., not giving them fields at all
I've roughly proposed that in the past https://github.com/pantsbuild/pants/issues/13086, but we can't to keep the alias mechanism +
./pants peek
w

witty-crayon-22786

02/04/2022, 8:32 PM
yea… and your “generators are all macros” proposal, which i can’t find anymore.
👍 1
h

hundreds-father-404

02/04/2022, 8:34 PM
But yeah what makes sense in my head is that you're not actually parametrizing the generator target...you're really parametrizing the generated targets. And overrides would override that.
Copy code
python_sources(
   resolve=parametrize("a", "b")
   overrides={
      "utils.py": {"resolve": "c"},
      "another.py": {"resolve": parametrize("c", "d")},
},
should result in:
Copy code
dir/f.py@resolve=a
dir/f.py@resolve=b
dir/utils.py
dir/another.py@resolve=c
dir/another.py@resolve=d
and your “generators are all macros” proposal,
the first drafts of https://docs.google.com/document/d/1HpJn2jTWf5sKob6zhe4SqHZ7KWBlX4a6OJOwnuw-IHo/edit#heading=h.holikof0ozs1
w

witty-crayon-22786

02/04/2022, 8:35 PM
that only works if the target generator has no fields itself: otherwise there is no clear choice of field for it.
h

hundreds-father-404

02/04/2022, 8:36 PM
Gah I do wish target generators weren't targets. My main insight is what Pants actually cares about are atom targets. We should do everything to make those atom targets as precise as possible. Everything else is only for boilerplate reduction
w

witty-crayon-22786

02/04/2022, 8:37 PM
removing their fields would be a step in that direction, i suppose.
h

hundreds-father-404

02/04/2022, 8:38 PM
What would that look like?
w

witty-crayon-22786

02/04/2022, 8:39 PM
you would still declare the Fields on the generator type, but they would never be passed to the constructor.
👍 1
…and so wouldn’t show up in
peek
, etc.
oh. hm. maybe what this is instead is that the target generator chooses everything about construction, including which Fields to pass.
❤️ 1
đź’Ż 1
because thirdparty targets still would want explicit requirements lists in some cases, tags, etc.
h

hundreds-father-404

02/04/2022, 8:41 PM
Yes I think so! Can't remember if I put the comment that I suspect parametrization must leak into target generation `@rule`s, and that is okay
w

witty-crayon-22786

02/04/2022, 8:41 PM
frankly… they are super redundant right now, so diving in on that is both exciting and scary.
but yea
h

hundreds-father-404

02/04/2022, 8:43 PM
I think that when you parametrize a target generator, you aren't actually creating multiple copies of the target generator. You're signaling instead that its generation rule should create multiple copies of each generated target (unless overridden)
w

witty-crayon-22786

02/04/2022, 8:44 PM
yea. given the redundancy, that’s going to take some refactoring. but yea.
👍 1
h

hundreds-father-404

02/04/2022, 8:45 PM
Target generators are already really weird, like how
tags
exists both on the generator itself and all generated targets. So I don't think the above proposed semantics will be that confusing. The docs try to push really hard that core message about atom targets being what matters
w

witty-crayon-22786

02/04/2022, 8:45 PM
ok. regarding generator targets becoming less significant: (mostly) removing their fields seems like a step in that direction, which would align with optionally (in the future) not creating them at all if they don’t have a name
h

hundreds-father-404

02/04/2022, 8:47 PM
I'm open to considering that, although not sure it's necessary? I guess it would be if you parametrize one of its fields..I wonder if we could work around that another way, like literally showing
parametrize("a", "b")
in the
./pants peek
for it. (although JSON serialization is a thing)
Also, I'm definitely open to making you do things like adding
TargetGeneratorMixin
to the
Target
class
w

witty-crayon-22786

02/04/2022, 8:52 PM
I’m open to considering that, although not sure it’s necessary? I guess it would be if you parametrize one of its fields..I wonder if we could work around that another way, like literally showing 
parametrize("a", "b")
 in the 
./pants peek
 for it. (although JSON serialization is a thing)
that, and the Field itself must actually be constructed (with some value)
h

hundreds-father-404

02/04/2022, 8:55 PM
I do think it's helpful to use
./pants peek
on target generators because it lets you programmatically understand your BUILD files. But I don't think that is as crucial as
overrides
working as described above