Add support for buildifier style in BUILD files, or support running an auto-format binary after pants tailor

[ahyatt-continua]

Is your feature request related to a problem? Please describe.

Our organization wants to have a consistent BUILD style, so we are using bazel's buildifier binary, which works well. However, by default, pants tailor creates BUILD files that do not conform to this binary, so now users have to run pants tailor and then manually go and run buildifier.

Describe the solution you'd like
Two good solutions I can think of:

1. A tailor option to support buildifier style, or any other popular style.
2. A tailor option to run an arbitrary program on the generated files.

Describe alternatives you've considered
We're living the alternative, which is to just do things manually. It could be better.

Additional context
None

[sureshjoshi]

I don't know if this works as any of the tailor stuff I run is pretty basic, but would something like this work for you?

I'm not entirely sure if fmt necessarily runs after tailor, but even if not, creating the new build files should tell fmt that the filesystem has changed and kick it off anyways.

backend_packages = [
    "pants.backend.build_files.fmt.buildifier",
    ...

pants tailor fmt ::

As a fallback, if that didn't work, then maybe something like pants tailor :: && pants fmt ::

https://www.pantsbuild.org/2.20/reference/subsystems/buildifier

[ahyatt-continua]

Yes, those work, and are a way to avoid manually running buildifier, but in exchange you manually run pants tailor. It seems reasonable that anything generated by pants should then be formatted by pants. In other words, it should be that this is something we shouldn't have to think about.

[sureshjoshi]

I'll concede, I'm a bit confused as to your workflow now.

support running an auto-format binary after pants tailor

by default, pants tailor creates BUILD files that do not conform to this binary, so now users have to run pants tailor and then manually go and run buildifier.

How are you running pants tailor if not manually? And what's the incremental effort to running pants tailor fmt?

[ahyatt-continua]

Yes, we're running it manually. I'd prefer to just run one command, otherwise we need to mention to everyone the "proper" way to create new build files, which is not just to run pants tailor but to do other things as well. This feature request would simplify that.

[sureshjoshi]

So, in pants it's pretty typical to chain goals as a single command:
pants fix fmt lint check ::

That's a very common paradigm, so pants tailor fmt :: would be a single command running 2 goals against all targets.

If you wanted to "simplify" it, you could create an alias to do that for you (https://www.pantsbuild.org/2.20/reference/subsystems/cli#alias).

[cli.alias]
ftailor = "tailor fmt"

[ahyatt-continua]

Thanks, I agree that's better than I had originally come up with.

I still think, though, that automatically formatting (if Pants knows how) anything Pants generates would be a completely reasonable thing to implement, and would help simplify this. Ideally, Pants should get things right without workarounds, even simple ones.

[huonw]

I agree with your point that this could be better: pants is writing new data to file that it (a) knows how to reformat, and (b) knows should be reformatted. Would be a nice quality-of-life improvement to do the reformatting automatically!

It looks like this is the rule that does the editing, including reading the existing contents:

pants/src/python/pants/core/goals/tailor.py

Lines 532 to 573 in 0d35991

	@rule(desc="Edit BUILD files with new targets", level=LogLevel.DEBUG)
	async def edit_build_files(
	req: EditBuildFilesRequest, tailor_subsystem: TailorSubsystem
	) -> EditedBuildFiles:
	ptgts_by_build_file = group_by_build_file(
	tailor_subsystem.build_file_name, req.putative_targets
	)
	# There may be an existing *directory* whose name collides with that of a BUILD file
	# we want to create. This is more likely on a system with case-insensitive paths,
	# such as MacOS. We detect such cases and use an alt BUILD file name to fix.
	existing_paths = await Get(Paths, PathGlobs(ptgts_by_build_file.keys()))
	existing_dirs = set(existing_paths.dirs)
	# Technically there could be a dir named "BUILD.pants" as well, but that's pretty unlikely.
	ptgts_by_build_file = {
	(f"{bf}.pants" if bf in existing_dirs else bf): pts
	for bf, pts in ptgts_by_build_file.items()
	}
	existing_build_files_contents = await Get(DigestContents, PathGlobs(ptgts_by_build_file.keys()))
	existing_build_files_contents_by_path = {
	ebfc.path: ebfc.content for ebfc in existing_build_files_contents
	}
	def make_content(bf_path: str, pts: Iterable[PutativeTarget]) -> FileContent:
	existing_content_bytes = existing_build_files_contents_by_path.get(bf_path)
	existing_content = (
	tailor_subsystem.build_file_header
	if existing_content_bytes is None
	else existing_content_bytes.decode()
	)
	new_content_bytes = make_content_str(
	existing_content, tailor_subsystem.build_file_indent, pts
	).encode()
	return FileContent(bf_path, new_content_bytes)
	new_digest = await Get(
	Digest,
	CreateDigest([make_content(path, ptgts) for path, ptgts in ptgts_by_build_file.items()]),
	)
	updated = set(existing_build_files_contents_by_path.keys())
	created = set(ptgts_by_build_file.keys()) - updated
	return EditedBuildFiles(new_digest, tuple(sorted(created)), tuple(sorted(updated)))

I'm not exactly sure how we'd apply the appropriate build file formatting rules likely something involving rules from https://github.com/pantsbuild/pants/blob/0d35991d0814fa4f45ce504933d143525898e653/src/python/pants/core/goals/update_build_files.py