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

witty-crayon-22786

07/31/2020, 6:44 PM
@happy-kitchen-89482, @hundreds-father-404: ok, no coding this morning, just thinking. but i think that it has been fruitful
❤️ 1
commented on https://github.com/pantsbuild/pants/issues/10455 with two independent-ish realizations
i discussed the second half briefly with @hundreds-father-404 yesterday, and he suggested breaking it out, which makes sense to me, especially if he has time to look at that today
meanwhile, i can adapt the address syntax for the first part, and fix tests that don’t have to do with “the root targets doen’t have the type i was expecting”… sort of failures
h

happy-kitchen-89482

07/31/2020, 7:05 PM
Commented there, but me like!
w

witty-crayon-22786

07/31/2020, 7:17 PM
holy wow. i just realized that the owning BUILD file can be a Field of the Target. no need for BFA.
1
h

hundreds-father-404

07/31/2020, 7:19 PM
a proper
Field
, or a special cased property? A tricky thing with
Field
is that the user must include it in the target’s definition, i.e. the class property
core_fields = ()
.
Generally, storing it on the
Target
sounds like a nice change, though. Notably, it would allow us to consistently have better error messages by pointing to the actual BUILD file. In the future, we could even include the line number of the BUILD file
w

witty-crayon-22786

07/31/2020, 7:22 PM
i was thinking proper
Field
. the implication is that a
Target
always “has”
BuildFileSources
, which aligns with the
Address
change
h

hundreds-father-404

07/31/2020, 7:28 PM
Disclaimer: Sorry I’m probably being too low-level right now, which I know isn’t great during the brainstorming stage. But, if it was a conventional proper
Field
, you wouldn’t be able to confirm it always has
BuildFileSources
. It was intentional that a target can have zero fields registered on it - we don’t want to force you to use any specific field. You’re encouraged, but not required, to use
COMMON_TARGET_FIELDS
. If you don’t want to special case that the field is always no matter what registered, you’d need to add an
assert
to the constructor to validate that
BuildFileSources
is included in
core_fields
. At that point, I’d recommend calling out the special casing by instead having
BuildFileSources
be an actual distinct property that you access via
tgt.build_file_sources
, rather than
tgt[BuildFileSources]
. -- The other tricky thing is that you need a consistent way to calculate that
BuildFileSources
. One place that complicates things is testing, as right now you can create a
Target
like a normal Python object without any BUILD file actually existing on disk. This problem is what’s stopped me in the past with a side project to merge
Address
with
BFA
. I think 1.5 months ago, I realized how much nicer it would be if we always had the information stored in
BFA
, just without the clunkiness of that actual type. I found it harder to do than I expected.
w

witty-crayon-22786

07/31/2020, 7:31 PM
The other tricky thing is that you need a consistent way to calculate that 
BuildFileSources
. One place that complicates things is testing, as right now you can create a 
Target
 like a normal Python object without any BUILD file actually existing on disk.
you can do this with any Sources field without the files existing on disk, as long as you don’t try to hydrate them
afaict, BFA right now is only used to determine which targets “own” a BUILD file… which is the perfect usecase for
Sources
.
i’m not sure whether it should be a “magical” field, or a real field, but… it feels like making it a field is less of a special case.
h

hundreds-father-404

07/31/2020, 7:33 PM
but… it feels like making it a field is less of a special case.
The difference is that it sounds like you expect this field to always exist, no matter what. That is in itself a special case. Even with
Sources
, that is not always used. For example,
python_requirement_library
has no
Sources
field.
w

witty-crayon-22786

07/31/2020, 7:33 PM
relates to the fact that
name
is a “required” field (that “turns into” your `Address`…? very related, i think…)
☝️ 1
anyway, we don’t need to do the BFA thing today… not a blocker.
👍 1
h

hundreds-father-404

07/31/2020, 7:34 PM
Yes, but
name
is not a
Field
. We actually never expose that via the Target API. The rule to go from
TargetAdaptor -> Target
will pop the
name
field from the
kwargs
and convert it into an
Address
, which then gets passed to the constructor of
Target
as a special value
w

witty-crayon-22786

07/31/2020, 7:35 PM
just food for thought. the “naming”/addressing of the
Target
feels integrally related to which BUILD file it comes from. i expect it’s possible, but whether it’s an extension of the special case or not is an open question