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

Consider adding explicit support for applying build metadata to multiple targets at once #13767

Closed
stuhood opened this issue Dec 1, 2021 · 4 comments · Fixed by #15836
Closed

Comments

@stuhood
Copy link
Member

stuhood commented Dec 1, 2021

There has been a longstanding desire to make it easier to remove boilerplate across multiple targets.

A few approaches have been proposed, but the closest thing we have to direct support is defining a macro that applies the metadata you'd like, and then using it in multiple places. Relatedly, metadata can now be applied to a subset of files in a target via overrides, but this is most useful within a single directory.

A common workaround that requires a patch to Pants is to have a Subsystem that provides the defaults (e.g. the python subsystem allowing for setting the default values of various python_sources arguments.)

Filesystem hierarchy

One of the most attractive (but potentially controversial) approaches is using filesystem hierarchy to remove boilerplate across multiple targets. This would involve allowing metadata to be defined in parent directories (likely in BUILD files, but potentially in other locations to account for the different semantics) which would affect targets defined in BUILD files in child directories.

But this is lightly in conflict with 1:1:1 (discussed most recently in #13488): metadata in parent directories affecting child directories without any references in those BUILD stretches the idea of "metadata living near the things it affects". But it's also possible to argue that if "metadata in parent directories (can) affect children" became a common enough pattern, then users would learn that they might have to look upward (and only upward) to find it.

Another challenge is how "matching" would work in a hierarchical system: having include/exclude lists in parent directories is fragile, and might require specifying filenames as long as the hierarchy is deep. A previous proposal explored explicit matches, and encounters this problem for deep hiearchies... it evolved into the existing overrides feature (which is best within a single directory).

Macros

As mentioned above: a repository can define v2 macros which remove boilerplate, and partially specify defaults, etc. This lies in a middle ground in terms of how explicit it is: on the one hand, each target signature has its own name, which is reasonably easy to understand. On the other hand, a target using a macro does not directly explain where the macro is defined the way inheritance would: users need to learn how and where macros are defined.

Inheritance/prototypes

The v2 engine experiments included a concept of inheritance for BUILD definitions which lived on until somewhere around the 2.0 release (a large batch of this code was removed in #10298). A target could indicate that it "inherited" from some other target, which could be marked abstract. When compared to a macro, this meant that a prototype/abstract-base could be defined in a BUILD file anywhere in the repo.

This is more explicit than either hierarchy or macros, since the prototype would always be explicitly referenced (if referenced at an absolute path though, it could be very short: á la //:django2 or //:hadoop3).

@stuhood stuhood changed the title Add support for hierarchical build metadata Consider adding support for hierarchical build metadata Dec 1, 2021
@stuhood stuhood changed the title Consider adding support for hierarchical build metadata Consider adding explicit support for applying build metadata to multiple targets at once Dec 1, 2021
@stuhood
Copy link
Member Author

stuhood commented Dec 1, 2021

The description tries not to have an opinion on an actual implementation, but my strongest impression after writing it is that combining hierarchy and macros could thread the needle between "being explicit" and "reducing boilerplate". Basically: put macros defined in any parent directory in scope for child directories... that way, when looking at an affected BUILD file, you see that it is affected (django2_python_sources(..), etc), but the need for macros to be defined globally goes away.

I haven't considered how that would affect tailor though... and I could imagine that being a bit awkward.

EDIT: After more thought and some feedback on this, I'm less sure that macros are a good idea. They're not declarative enough for tailor, and probably also aren't declarative enough for humans.

@benjyw
Copy link
Contributor

benjyw commented Dec 6, 2021

I think inheriting metadata from up the filesystem is fairly intuitive. Other config works that way, including .gitignore.

@thejcannon
Copy link
Member

thejcannon commented Dec 20, 2021

I'm partial to option 1, as it keeps the boilerplate to a bare minimum and shouldn't be a foreign concept to users. Perhaps for sanity you could explore scoping what is allowed to be in a parent-dir to be targets generators or just "owning" generators.

I see this akin to (albeit much more complex) to .gitignore files. Most repos have a top-level one relevant to every file in the repo. But every now and then you need a more nested .gitignore specific to this directory.

The most natural to me would be my python_* generators at my code root, and then when necessary for additional info or overrides I can define a BUILD file in a subdir (declare pexs or skip_pylint, etc...). Even if I have to repeat some metadata in that subdir (I.e. no "inheritance" between generators) the end result is still less boilerplate than what exists today, so that's a net win.

All of this says nothing for other language support 🤓

@stuhood
Copy link
Member Author

stuhood commented Mar 29, 2022

Some design happened around "Configuration" in https://docs.google.com/document/d/1mzZWnXiE6OkgMH_Dm7WeA3ck9veusQmr_c6GWPb_tsQ/edit#, but it was separable from #13882, and so was deferred. Seeing how parametrize is used in practice will give us better insight into whether Configuration is the right approach.

kaos added a commit that referenced this issue Jun 23, 2022
Provide default field values for a set of targets in a particular subtree of a project using the `__defaults__` symbol, using the same syntax as for the `overrides` field.

Example:

```python
# BUILD file
__defaults__(
  {
    pyhon_sources: dict(sources=["*.py"]),
    (python_tests, python_testutils): dict(tags=["some-tag"]),
  },
  all=dict(description="Copyright (c) 2022, Company Ltd."),
)

...
```

The `__defaults__` declaration should go at the top of the BUILD file, in order to apply to the targets in the same file. All BUILD files in subdirectories inherit the same defaults by default, but can be overridden/extended with their own `__defaults__`.

Closes #13767.
Discussion: #15809.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants