Repository navigation
RFC: Add required-targets for workspace package selection - #4013
Sakib25800 wants to merge 17 commits into
Conversation
ranger-ross
added
the
T-cargo
| each directly selected package. Commands that do not perform these operations, such as `clean`, | ||
| `fetch`, `tree`, and `fmt`, do not check `required-targets`, even if they accept `--target`. |
There was a problem hiding this comment.
cargo fetch --workspace --target x86_64-pc-windows-msvc seems like it should skip packages that are not compatible with x86_64-pc-windows-msvc. Likely similar for tree.
fmt does not accept --targeet
|
|
||
| # Future possibilities | ||
| [future-possibilities]: #future-possibilities | ||
|
|
There was a problem hiding this comment.
Let's add target tuples as a future possibility, linking the rationale section talking about it to it.
There would be an open question of syntax since making this field an array would make it OR the array members.
| # Future possibilities | ||
| [future-possibilities]: #future-possibilities |
There was a problem hiding this comment.
Let's add a future possibility of removing the unstable forced-target
| The `per-package-target` nightly feature defines the `forced-target` field, which forces a package | ||
| to build for a specific target-tuple. `required-targets` instead determines whether a package is | ||
| included for the selected target. It does not select a different target, so it does not replace | ||
| `forced-target` for workflows that build packages for different targets in one command. |
There was a problem hiding this comment.
I think this makes more sense as an Alternative than Prior Art
| Users can already select packages in a workspace with the flags | ||
| `--package` and `--exclude`. Cargo features can also be used to restrict which cargo-target |
There was a problem hiding this comment.
Users can already select packages in a workspace with the flags
--packageand--exclude.
Is more of an Alternative of "do nothing".
| included for the selected target. It does not select a different target, so it does not replace | ||
| `forced-target` for workflows that build packages for different targets in one command. | ||
|
|
||
| Published crates have mainly used their documentation to specify which targets they support, or they |
There was a problem hiding this comment.
Again, more Alternative and less Prior art
| Some higher-level languages and build tools have the ability to specify which platforms are compatible. | ||
|
|
||
| - Python packages have [classifiers](https://pypi.org/classifiers/) as package metadata that includes supported platforms. | ||
| - Python wheels (pre-built packages) have [platform compatibility tags](https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/#platform-compatibility-tags). | ||
| The reference explains how these are [used](https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/#use) | ||
| by installers to determine which build of a package to install. | ||
| - `npm` allows specifying which [`os`](https://docs.npmjs.com/cli/v11/configuring-npm/package-json#os) and | ||
| [`cpu`](https://docs.npmjs.com/cli/v11/configuring-npm/package-json#cpu) a package supports. These generate an | ||
| error when installing a package that does not support the platform used. | ||
| - Swift has [`package.platforms`](https://developer.apple.com/documentation/packagedescription/package/platforms) | ||
| to specify minimum deployment versions for platforms such as `macOS`, `iOS`, `watchOS`, and `tvOS`. | ||
| - [Buck](https://buck2.build/docs/concepts/configurations/#using-configuration-compatibility) | ||
| and [Bazel](https://bazel.build/concepts/platforms#skipping-incompatible-targets) | ||
| both provide `target_compatible_with`. By default, Bazel skips incompatible targets selected | ||
| through wildcard patterns and reports an error when an incompatible target is requested explicitly. |
There was a problem hiding this comment.
Would be good to compare with these prior art. Are they effectively the same? Are there benefits or things we lose with the differences?
| An always-false `required-targets` condition also skips direct workspace builds, but prevents | ||
| explicitly checking or testing the package. | ||
|
|
||
| ## Target-specific dependency resolution |
There was a problem hiding this comment.
This is less a future possibility of this RFC and more a parallel effort. I would recommend focusing on that, rather than design details.
One purpose of future possibilities is to see where the current design might go and so people can consider the impact on this RFC. That isn't happening here. People could instead get side tracked on design questions that aren't relevant to this RFC. Mentioning it at all is more to head off any questions about this being a future possibility of this RFC.
| For package selection, omitting `required-targets` has the same effect as specifying `'cfg(all())'`: | ||
| the package is eligible for every target. | ||
|
|
||
| This field supports [workspace inheritance](https://doc.rust-lang.org/cargo/reference/workspaces.html#the-package-table). |
There was a problem hiding this comment.
I'm trying to think through where workspace inheritance becomes relevant. Maybe mark this as an Unresolved question for implementation / stabilization (ie not blocking this RFCs approval)?
|
|
||
| ## Target-specific dependency resolution | ||
|
|
||
| `Cargo.lock`, and by extension, `cargo vendor`, must assume that a package may be built on any platform that has or will exist. This means that if a transitive dependency pulls in Windows-specific dependencies, `cargo vendor` will include them when run on a Linux-only application. Being able to tell `cargo vendor` what platforms to care about can reduce the space used in a repo and reduce churn. |
There was a problem hiding this comment.
Currently, cargo vendor for lot of projects which have ~3MB in dependencies download ~200MB in additional dependencies for unsupported platforms. A huge interest in being able to declare supported targets is to make cargo vendor useful.
Why does this RFC specifically spell out that this misbehaviour must continue?
There was a problem hiding this comment.
That problem is related but as we discussed in #3759, the solutions will end up being very different. This section does talk a lot about that design (a resolver.targets in .cargo/config.toml). See also my comment at #4013 (comment)
Since these can be different RFCs they should so we can have more focused conversations and move things along more smoothly. See also https://epage.github.io/dev/pr-style/#c-isolate
|
@Sakib25800 as I'm assuming this is your first RFC, I would recommend
|
This branch has not been deployed
5 participants
Summary
Add a
required-targetsfield to the[package]table ofCargo.tomlso packages can declare target requirements usingcfgsyntax.This builds on #3759 by @carloskiki, and addresses feedback from that proposal.
Rendered