Pants peek goal

[wisechengyi]

Borrowing @baroquebobcat 's idea, sometimes it is good to have a quick peek on the target declaration, because a BUILD can get large and contain multiple target declarations, so the peek goal will just show

Example:
In foo/bar/BUILD file:

target(name = 'a',
  dependencies=['other:targets', 'that:might', 'be/useful:to-show']
)

target(name = 'b',
  dependencies=['other:targets', 'that:might', 'be/useful:to-show']
)

target(name = 'c',
  dependencies=['other:targets', 'that:might', 'be/useful:to-show']
)

Result

./pants peek foo/bar:a
# target/to/BUILD:1 // line number
target(name = 'a',
  dependencies=['other:targets', 'that:might', 'be/useful:to-show']
)

[kwlzn]

the feature itself would be great.. but how would you implement this given the current build file parser?

[baroquebobcat]

Using redbaron probably. You could get pretty far doing it that way. It wouldn't be able to deal with things like macros. I think having a way to do those would be really slick though.

[wisechengyi]

@15Dkatz just fyi that this can probably be implemented from buildozer

[15Dkatz]

@denalimarsh:

just fyi that this can be probably be implemented from buildozer

[denalimarsh]

I have opened a PR addressing this:
#5142

[Eric-Arellano]

@jriddy is interested in possibly working on this.

Two approaches:

1) Show the BUILD file exactly as it is.

For each address the user inputs, we will find the corresponding BUILD file, like we do with filedeps:

pants/src/python/pants/backend/project_info/filedeps.py

Lines 96 to 98 in 8da1680

	build_file_addresses = await MultiGet(
	Get(BuildFileAddress, Address, tgt.address) for tgt in targets
	)

We can then use the paths to the BUILD files to say await Get(DigestContents, PathGlobs([build_file_address.rel_path for build_file_address in bfas]). This will get us back the actual contents of each BUILD file, and we can render from there.

2) Show the normalized target data

Use the Target API to show every field for a target after normalization, e.g. after applying default values.

We would iterate over each field in Target.field_values, very similar to the __repr__ for a target:

pants/src/python/pants/engine/target.py

Lines 350 to 357 in 8da1680

	def __repr__(self) -> str:
	fields = ", ".join(str(field) for field in self.field_values.values())
	return (
	f"{self.__class__}("
	f"address={self.address}, "
	f"alias={repr(self.alias)}, "
	f"{fields})"
	)

Possibly, we'd want to filter out any fields=None, which generally means the user left off the value.

We'd maybe want to recreate something that looks like a BUILD file?

./pants peek path/to:tgt path/to/subdir:another_tgt
# path/to/BUILD
python_library(
  name="tgt",
  sources=["foo"],
  dependencies=None,
  compatibility=None,
  description=None,
  tags=None,
)

# path/to/subdir/BUILD
files(
  name="another_tgt",
  sources=["bar"],
  dependencies=None,
  description=None,
  tags=None,
)

This raises an interesting question, if we should evaluate the sources and dependencies fields, or show the raw values.

--

We need to decide which approach (or both) we want to take. To answer this, we need to take a step back: what is the purpose of a peek goal? Yi's original motivation was:

sometimes it is good to have a quick peek on the target declaration, because a BUILD can get large and contain multiple target declarations

I don't think that will be an issue very much anymore thanks to dep inference. Most of our BUILD files are no more than 3 lines in both pantsbuild/pants and the Toolchain repos. Rather, I suspect the more useful thing is seeing "How is Pants reading my BUILD file, after things like defaults and dep inference?" (Approach #2)

[jriddy]

It would be pretty easy to handle both of these situations with a --peek-raw flag to view the raw build file exerpt, whereas the default would be after normalization. I don't see a usecase for partially normalized output, but that's me.

The question that would remain for me is output format, especially in the case of multiple targets (and directories). Do we want a machine readable variant? What would that look like? JSON?

[Eric-Arellano]

All those ideas sound excellent. I think a --json and --raw flag make sense. Alternatively, an --output={raw,normalized,json} option is probably more scalable. See https://www.pantsbuild.org/docs/rules-api-subsystems#type for using enums as options.

[jriddy]

I was thinking --raw was independent of the output format, but i'll adopt something like --output={python,json,table} for output formatting i think.

Still not sure how to handle the look of multiple BUILD targets in "python" output mode...like if i have this source tree:

$ find src/python -not -name '*.py'
./src/python/a
./src/python/a/BUILD
./src/python/b
./src/python/b/BUILD

where both BUILD files are just python_library(), then how should the output of this command look?:

$ ./pants peek --raw src/python::

The naïve way would just be...

python_library()

python_library()

...but I don't like that. Maybe adding comments for the path part of the address makes sense?:

# src/python/a/BUILD:
python_library()


# src/python/b/BUILD:
python_library()

Idk, what do you think?

[Eric-Arellano]

I was thinking --raw was independent of the output format, but i'll adopt something like --output={python,json,table} for output formatting i think.

That could work. Although, the reason I was thinking a single enum is that I don't imagine --raw --json being particularly useful, and that we'd only want JSON to be normalized data.

Table sounds great, but might be non-trivial with formatting (although lots of prior art from tools like Coverage). I recommend first focusing on landing --output={python/raw,json}, and then adding table in a dedicated followup.

--

...but I don't like that. Maybe adding comments for the path part of the address makes sense?:

I think so. Definitely we need some way to ID the target. Maybe something like this is more distinct?

src/python/a/BUILD
-------------------
python_library()

src/python/a/BUILD
-------------------
python_library()

# :tests
python_tests(name="tests")

That is, use the --- dashes to differentiate between distinct BUILD files. Then comments for different targets within that BUILD file. The dashes align with ./pants help, look at ./pants help-advanced pytest.

--

Related question, we want to preserve comments in BUILD files, right? That's actually probably easier to do than stripping, as we're going to simply dump the relevant raw lines without normalization.

The part where I'm not sure: how should we handle comments right above the target?

# IMPORTANT: Use this target in production so that we leverage the GPU.
python_requirement_library(name='req_with_cuda', requirements=['req[cuda]==3.1'])

[Eric-Arellano]

Closed by @jriddy in #11347 😎