Shifting asset metadata to consumer-node: Our options #15847
Replies: 8 comments 28 replies
-
I'm generally +1 to this proposal. If |
Beta Was this translation helpful? Give feedback.
-
I think there's a fourth option (which basically is an alternative of option 3, I think), and that is to keep the dependencies field as a sequence of strings, but with optional extra metadata for the edges. Perhaps something like this, to give an idea of what it could look like: dependencies = [
":config @resource",
"//:lib @test",
"//:util @runtime",
...
] Related discussions on previous issues: I'd like to keep the dependencies syntax as strings, if possible, to make the change less impactful (and backwards compatible), so you only add edge data if you need it. Edit: so this idea is inspired by a comment made by @jsirois, only I can't find that particular comment now.. |
Beta Was this translation helpful? Give feedback.
-
I harp on this point alot, but "Shifting asset metadata to consumer-node" is another way of saying Pants metadata configures the verbs; i.e.: it should always be thought of fundamentally as use-dependendent. I.E.: targets never made much sense, but configuring goals does make sense, and even more so to an end user. A user fundamentally wants to do things, not declare things. It goes: "I want to vroom - oh, that needs gas.". Not "I want to state I have gas here. Ah, maybe I should vroom?". So, a doc goal may have a different idea of dependencies from a javac goal, for example; so 1 above makes sense. A doc goal or its rules might need a field set that includes css dependencies whereas a javac goal or its rules need a field set with just dependencies that can be turned into classfiles (metaprogramming and annotation processors aside - since those could use resources to drive compilation). These different ideas of dependencies, or even just the more general different field sets needed by different goals, points to 1 as far as I can tell. Some verbs will simply not fit "runtime", "provided", etc scopes at all. |
Beta Was this translation helpful? Give feedback.
-
In general, huge +1 to this. I like consolidating files/resources into a single type ( If I were to rank these, as is, it would be 1, 2, 4 (the kaos proposal) as all being very close, with a slightly distant 3. I really hate In the current Here's a question: Are the my_sources(
name="lib",
...
)
assets(
name="build-stuff",
sources=["config.json", "another-config.json"]
)
assets(
name="runtime-stuff",
sources=["fancy-pants.exe"]
)
some_target(
name="t",
dependencies=[":lib", ":build-stuff"],
runtime=[":runtime-stuff"]
) Having typed all this, I started thinking about |
Beta Was this translation helpful? Give feedback.
-
Leaning in to @jsirois's insight, to truly represent this we would need one set of metadata per goal. Or at least one dependencies field per goal. That is of course very verbose, so we would also need some way to shorthand that for the 95% of cases where those are all the same in practice. We'd also need to figure out how this all interacts with dep inference. |
Beta Was this translation helpful? Give feedback.
-
I think that from a grokkability perspective, option 2 makes the most sense to me (although possibly with a shorter name, like But really: given that we are already mostly in the world of option 1, sticking with option 1, but:
... will go a long way. For example: if |
Beta Was this translation helpful? Give feedback.
-
Since we have to decide on what to do with assents at the consumer, should we leave a door open to describe, what an asset is (not how it's supposed to be used), like an optional field?. If we focus on handling it in a 'this is an asset, it represents something' way, we would have to rely on the toolchains to figure out how to use it. There might be cases where that's not immediately possible. To continue, I could see use for attaching meaning to a file, for intent of deprecating assets (so that a warning appears). I think, adding metadata to an asset is not wrong per se. It's just the wrong approach to let it define in advance, how it's going to be used. About the syntax, I am mighty undecided. I probably still need to take a few showers to think it through. All in all, it sounds reasonable to have a more general asset definition. Is it possible to describe groups of assets with this proposal? |
Beta Was this translation helpful? Give feedback.
-
Another aspect I think is worth considering in this context is how to treat the various syntaxes used for assets. What I mean by that is, say we have linters for yaml, json, toml, html, css etc... you get the picture. Now if we include such files with an The brute force method would be that every "asset file" linter iterates over all the files, and picks out the ones it recognizes. Maybe this is good enough (or even best). Just wanted to raise the question in case there are something clever to be done here, while things are still in the mold. |
Beta Was this translation helpful? Give feedback.
-
There's a proposal to unify
file
andresource
intoasset
, which is missing a key piece of Pants functionality we need to decide on.The TL;DR of all this is "We (both the user and us devs) can't always know how a source file will be consumed (resource v file), so forcing that metadata at the node-site is folly". Instead we need to have one asset type, and shift that metadata to consumption-site. This eventually turns into "typed dependencies", which is a much larger discussion.
This isn't unique, however this will be widespread. Examples of current modeling:
archive
having bothfiles
andpackages
(and nodependencies
)python_test
having bothdependencies
andruntime_package_dependencies
jvm_war
having bothdependencies
andcontent
fieldsThere's also some cases we don't yet implement which could benefit from the result of this discussion:
pex_binary
targets #15454 could be implemented with atypecheck_dependencies
field (or equivalent). This also allows us to allow Treat imports under "if TYPE_CHECKING" as weak. #15384 , because we can infer the dep into a differentdependencies
field.go_package
in apython_source
, with the expectation that they could run the package as a subprocess because it will exist in the test sandbox.Some options we can consider (plus more if you have your thinking cap on):
files
and/orresources
field added to relevant targets. (Even though we can get away with one field, and leave the other "type" to be implicit independencies
, I'd argue we'd want both to be explicit and to call a spade a spade)BuiltPackage
could be converted to a file, so we'd actually know how to consume it from, say, apython_test
(especially if the built package address was in a field calledfiles
. There's no ambiguity).source_deps
,files
,resources
,py_distribution_deps
,system_deps
).runtime_dependencies
would contain all dependencies needed to be present at runtime. This would bleed into test sandboxes and possiblyarchive
s anddocker_image
s.dependencies
would contain dependencies in every environment (either on the FS or in a packaged thing). We could also support one-offs liketypecheck_dependencies
case above.python_test
getruntime_package_dependencies
"promoted" to justruntime_dependencies
.dependencies
->resource
,runtime_dependencies
->file
.dependencies
field.dependencies = {"build": [...], "runtime": [...]}
or something to this vein of keeping one field, but having it become more "rich".The engine is flexible enough we can get any of these working, so it's a matter of what direction we want to head with the lens of our users in mind.
Beta Was this translation helpful? Give feedback.
All reactions