Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of boilerplate/trivial BUILD files #14074

Open
asherf opened this issue Jan 5, 2022 · 5 comments
Open

Get rid of boilerplate/trivial BUILD files #14074

asherf opened this issue Jan 5, 2022 · 5 comments

Comments

@asherf
Copy link
Member

asherf commented Jan 5, 2022

I find that there are a lot of BUILD files contain just boilerplate:

python_sources()

or

python_sources()
python_tests(name="tests")
python_test_utils(name="test_utils")

or

shell_sources()

Pants should be able to infer those so we don't actually need to have those on the file system and in source control.

@Eric-Arellano
Copy link
Contributor

Currently, one role of targets is to explicitly opt into what Pants should operate on. But this isn't a compelling motivation - we can switch into an opt-out model. That's how ./pants tailor works with its [tailor].ignore_* options, and I think it works well.

Eric-Arellano added a commit that referenced this issue Jan 29, 2022
This adds generic support for `lint` implementations that do not deal with targets. That allows us to merge `validate` into `lint`, which is much cleaner.

## CLI specs

As before with the `validate` goal, it's not very intuitive how to get Pants to run on files not owned by targets, which you want for `validate`. `::` only matches files owned by targets, whereas `**` matches _all_ files regardless of targets.

So, users of `regex-lint` should typically use `./pants lint '**'` rather than `./pants lint ::`, which is not intuitive

https://docs.google.com/document/d/1WWQM-X6kHoSCKwItqf61NiKFWNSlpnTC5QNu3ul9RDk/edit#heading=h.1h4j0d5mazhu proposes changing `::` to match all files, so you can simply use `./pants lint ::`. I don't think we need to block on this proposal? This is still forward progress, and also `validate`/`regex-lint` is not used very much fwict.

## Batching

We don't yet batch per #14186, although it would be trivial for us to hook up. I'm only waiting to do it till we can better reason about if it makes sense to apply here too.

## The `fmt` goal

Note that we need more design for `fmt` before we can apply this same change there. fmt is tricky because we run each formatter for a certain language sequentially so that they don't overwrite each other; but we run distinct languages in parallel. We would need some way to know which "language" target-less files are for.

## "Inferred targets"

A related technology would be inferred targets, where you don't need a BUILD flie but we still have a target: #14074.

This is a complementary technology. The main difference here is that we can operate on files that will _never_ have an owning target, such as a BUILD file itself.

[ci skip-rust]
[ci skip-build-wheels]
@thejcannon
Copy link
Member

I think the way we'll eventually "accomplish" this is through the "recursive" target generators @Eric-Arellano has been getting is nearer to.

So you'd just have a root BUILD with python_sources(recurse=True) (or whatever). We haven't decided overrides, but that's the gist.

@Eric-Arellano
Copy link
Contributor

Possibly -- for that to make sense, we might need support for a subdirectory BUILD file overriding a parent. Which imo is somewhat controversial and needs design. I'm not committed to "recursive target generators" or anything, but do indeed want to give us options.

Another approach discussed w/ several users in Slack is first-class support for BUILD file management, e.g. Buildozer. It would help w/ how annoying it is to update metadata for several places. It would not help, though, with the ugly aesthetics of mostly empty files.

@stuhood
Copy link
Member

stuhood commented May 24, 2022

IMO, we should close this one in favor of #13767, and add any keywords needed over there.

@Eric-Arellano
Copy link
Contributor

I think it might depend on which problem(s) we are prioritizing solving:

  • Admin frustration w/ applying metadata to many files
  • General frustration w/ mostly empty BUILD files

While related, #13767 seems more focused on the first problem of changing metadata for many files. In such that—a totally valid solution for #13767 is to lean into something like Buildozer—but that is not a valid solution for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants