https://pantsbuild.org/ logo
Join Slack
Powered by
An invitation to a bikeshed:
# development
h
An invitation to a bikeshed:
The 
peek
goal currently emits a JSON structure with per-target information culled from BUILD files.
This includes whatever is in the 
dependencies=
field, namely explicit dependencies.
That change enhances this to add all dependencies (explicit and inferred), so you can treat the output as a build graph
So now we need two JSON keys, one for the explicit dependencies (that currently go in the 
dependencies
JSON key, as these are exactly the BUILD file fields), and one for "all dependencies" including the inferred ones.
The question is what to name each of these keys
Options include
A) Keep 
dependencies
as is, to match the BUILD file fields, and find a new name for all dependencies. B) Put all dependencies in 
dependencies
and find a new name for the explicit dependencies. C) Find new names for both. D) Nest the BUILD file information in a subobject, so that it's clear that all these keys are just verbatim from the BUILD file, and have the "all dependencies" information outside that subobject. E) Something else
Please tell me what color this bike shed should be!
h
Oh I like the idea of renaming 
depenencies
to 
extra_deps
! We keep talking about how important dep inference is, so I think it improves clarity a lot that you're augmenting what's inferred
h
What is 
extra
about them?
dependencies
today is just the BUILD file dependencies
Oh I see
extra to what is inferred
If we're going to rename them then I would say 
explicit_dependencies
is good
h
I'm suggesting we rename the 
dependencies
field to 
extra_deps
. You are explicitly adding deps in a BUILD file, which are extra to the inferred ones
h
OK, that is for another day, and a huge and unnecessary change
Let's not turn the bikeshed into a yak shave... 🙂
This is just about what 
peek
should do, renaming the target field is a massive undertaking
In terms of the amount of deprecation warnings it would generate everywhere
h
Isn't my suggestion to rename the 
dependencies
field 
extra_deps
what your option c was? Or instead you meant we're only changing names in 
./pants peek
and not changing any build file symbols?
coke
w
i would say use the same field name, but make expanding them optional. and would expand them by default
➕ 1
applies to other AsyncField types like 
sources
as well… and shouldn’t need a new name for the expanded version of each AsyncField
👍 1
h
An option like 
./pants peek --evaluate-fields
? 
--raw-fields
?
h
I think it's potentially really useful to show both
seeing explicit vs implicit deps
for example
w
An option like 
./pants peek --evaluate-fields
? 
--raw-fields
?
or the opposite, yea… 
--raw-fields
h
And I am really trying to not have this be more of a science project than it has already become
w
maybe. but needing to name it is awkward
h
Can have two subobjects with the same keys, one expanded one not
build_file: {}, expanded: {}
Or something
w
while i’m sure there are usecases for seeing both in one invoke, it seems a bit speculative
(admittedly, so is disabling expansion of the async field. so maybe just punt on that and expand)
c
From my point of view, what 
peek
would be most useful for is to inspect what information is available to a certain goal/rule is, to answer questions like “why does this not work/do what I want” or “where does this come from?” etc. Given that, I’d like to be able to query peek for this information, from a goals perspective, that is something like:
Copy code
./pants peek --goal=package src/python/pants/bin:pants
And that presents the JSON data for all targets and the dependencies and sources fully expanded etc as consumed by package. Regarding explicit vs infered/injected dependencies, if the structure is expanded, there could be a meta field on the nested data, indicating which it was.. as I think that dependencies are dependencies, explicit or inferred is just a means how they got there. If you for a particular query want to see only explicit for instance, that could be a filtering option to peek to exclude all other deps.
Copy code
# current
[
  {
    "address": "src/python/pants/bin:pants",
    "target_type": "pex_binary",
    "always_write_cache": false,
    "dependencies": [
      ":pants_loader"  # => expand this
    ],
    "description": null,
    "emit_warnings": null,
...
# suggested =>
[
  {
    "address": "src/python/pants/bin:pants",
    "target_type": "pex_binary",
    "always_write_cache": false,
    "dependencies": [

  {
    "address": "src/python/pants/bin:pants_loader",
    "target_type": "python_library",
    "dependencies": null,
"dependency_kind": "explicit",   # peek meta field indicating kind of dependency
    "description": null,
 ...
My splash of paint on the wall.. 😛
h
uuuuuugh, just realized that peek operates on unexpanded targets
So it may not after all be the right place for generating a build graph
./pants peek ::
will not give you the generated targets, only the generators
h
Which we should change @happy-kitchen-89482. I think it should give both the target generator and generated target
Same with ./pants list - it should list every target in your project
h
I tend to agree
It was probably that way because of the RAW mode
but maybe only the generators that have addresses, yes?
In a world where some may not
h
Yes, precisely. This means we need to land https://github.com/pantsbuild/pants/pull/12952, altho it will require some more prework
Open in Slack
PreviousNext
Powered by