New mechanism for marking process outputs as platform agnostic #16873
Comments
Yea, this should probably be inverted by having the default platform value come from Marking things platform agnostic would be good, but is probably not critical until after #12203 is implemented, since cross-platform hits are very unlikely right now for other reasons. |
For Remote Execution, we currently hardcode the platform for the runner: https://github.com/pantsbuild/pants/blob/ba7566515016ab91668c3610666594844dd5e7db/src/rust/engine/src/context.rs#L204-L206 This won't work as we move to remote execution being a per-environment/process mechanism, such as via #16890. We will add remote execution environment targets, which will allow users to set different platform properties per environment, such as using different Docker images. Thus, each environment target will have a `platform` field because it may be different among them. So, the platform must come from the environment target, not a global hardcoded value. Our remote command runner doesn't like the platform being optional. In practice, it no longer was optional thanks to #16874, but our types did not reflect that. When we add back platform-agnostic processes, we will use a new field to indicate this, while still setting the platform. See #16873. [ci skip-build-wheels]
|
In the context of #17354, this becomes very relevant again. The key takeaway from that issue is that processes themselves can't really be truly platform independent (they need to run "somewhere"): but their outputs can be. Which is really not that surprising: it's a "host" vs "target" platform distinction. |
stuhood
changed the title
Platform-aware vs. agnostic processes
Some processes care about which platform they are run on, such as building a PEX (when
--platformis not specified). Whereas others do not matter, like runningcount-loc: we expect the result to be identical regardless of the platform.For platform-aware processes, it's crucial we include the Platform in the cache key.
Status quo
Currently, processes default to being platform-agnostic. A plugin author opts in to platform awareness by setting
platformon theProcess.Issues
#13682 messes things up: When running processes with Docker, it is important we always use the same
--platformevery time. Otherwise, Docker gives this warningGenerally, I also believe it is unsafe to default to the possibly dangerous thing, marking something as platform-agnostic. Instead, we should default to the safe semantics, and let others opt-in to something being agnostic.
Proposal
platformonProcess, and instead for now always set it for everyProcess.Processlikeplatform_agnostic: bool, which will causeplatformto not factor into the cache key.The text was updated successfully, but these errors were encountered: