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

What target modeling changes to land in Pants 2.8? #13160

Closed
Eric-Arellano opened this issue Oct 8, 2021 · 5 comments
Closed

What target modeling changes to land in Pants 2.8? #13160

Eric-Arellano opened this issue Oct 8, 2021 · 5 comments

Comments

@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented Oct 8, 2021

The target generator proposal kicked off a slew of changes and new questions. This ticket tries to catalog Where We're At and Where We Should Go.

What has already landed

Much of the target generator proposal has been implemented:

What I suggest for Pants 2.8

Finish most of the original target generator proposal, but punt on any changes to project introspection.

  • Finish internally splitting target generator vs. atom targets for files, resources, python_tests, and python_library. Plan to internally split python_library into two targets without user API changes #12954
  • Rename targets we already have exposed:
    • python_requirement_library -> python_requirement
    • python_library -> python_sources
    • protobuf_library -> protobuf_sources
    • shell_library -> shell_sources
  • Add a source: str field to complement sources: list[str].
  • Rename the generated file targets to stop being the name of target generator
    • shell_source
    • shunit2_test
    • python_source
    • python_test
    • file
    • resource
    • protobuf_source
  • Add an overrides field for file-based target generators like python_tests. This is a splashy new feature with tangible benefits in boilerplate reduction.
  • Add ./pants mend to automate fixing this all.

Why do these now?

These changes can wait if I can't finish them in time—they're not blocking.

However, I'd like to try to land them because they clean up the conceptual space of "file targets" without changing any current semantics. The only deprecation is needing to run our migration script rename_target_types_pants28.py. We already have file targets, which are confusing and under-documented, both for users and for plugin authors. These changes give us the benefits of the target generator proposal right away.

I'd like to land the overrides field because it is an easy win to make a big dent in boilerplate and to give an alternative to the confusing semantics of how splitting out a new target currently requires excluding the sources from its previous owner.

We still have a lot of upcoming changes—landing these changes in Pants 2.8 will make those subsequent changes easier to adapt to. We're amortizing the deprecations.

What remains

@stuhood
Copy link
Sponsor Member

stuhood commented Oct 8, 2021

Works for me. Thanks for writing this up.

The fact that Specs don't match generated/file targets living in the matched directories is I think closer to being a bug than something we should improve. I expect that breaking that portion out of #13086 (which is overall more to do with what the dependencies of targets should be, rather than whether Specs match them) might be good.

@Eric-Arellano
Copy link
Contributor Author

The fact that Specs don't match generated/file targets living in the matched directories is I think closer to being a bug than something we should improve.

Agreed that that is a super awkward edge, possibly enough to be a bug. Thanks for your comments on #13086 - I'm going to focus for now on what I proposed in Pants 2.8 because it is unblocked by everything else, and then will spend more time thinking about What Remains...

@Eric-Arellano
Copy link
Contributor Author

I revised the plan for 2.8 to distinguish between renaming target types we already expose (python_requirement_library, protobuf_library) vs. giving a new target type name for generated targets (protobuf_source etc).

I think it makes sense to do the renames of existing symbols no matter what, as they are more descriptive (people get confused what "library" means) and it prepares us for introducing the new targets.

But I want to take more caution with the new target types, as it means users will be able to explicitly declare them in BUILD files. I want to make sure we have the modeling of those file-based targets right before we do this. For example, I feel fairly convinced that it should be file(source="f.json") rather than file(sources=["f.json"]) w/ eager validation that # files == 1. We can't make that type of change as easily once we expose the file target name.

@Eric-Arellano
Copy link
Contributor Author

I had a productive weekend :) Status update:

Newly finished

Remaining

@Eric-Arellano
Copy link
Contributor Author

Almost everything I wanted to land in 2.8 is now landed!

Only last blocker is conftest.py modeling, only blocked by further review of #13274.

Newly finished

Remaining for Pants 2.9+

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

No branches or pull requests

2 participants