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

Platform-specific-ness is carried via deps #9982

Closed
wants to merge 1 commit into from

Conversation

illicitonion
Copy link
Contributor

If e.g. an optional feature is only enabled on certain platforms,
meaning a transitive dependency is only enabled on certain platforms,
currently that information can't be worked out from the output of
cargo metadata.

With this change, when a dependency is configured for a specific
platform, its dependencies are by default also configured for that
specific platform, unless they explicitly have a different platform
attached to them.

This makes it easier to translate crates.io packages for building with
non-cargo build systems, because this platform-specific behaviour of
feature resolution is now observable in cargo metadata, rather than
requiring those systems to track and re-implement feature resolution
themselves with regard to platforms.

Fixes #9863.

@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 19, 2021
If e.g. an optional feature is only enabled on certain platforms,
meaning a transitive dependency is only enabled on certain platforms,
currently that information can't be worked out from the output of
`cargo metadata`.

With this change, when a dependency is configured for a specific
platform, its dependencies are by default also configured for that
specific platform, unless they explicitly have a different platform
attached to them.

This makes it easier to translate crates.io packages for building with
non-cargo build systems, because this platform-specific behaviour of
feature resolution is now observable in `cargo metadata`, rather than
requiring those systems to track and re-implement feature resolution
themselves with regard to platforms.

Fixes rust-lang#9863.
@joshtriplett
Copy link
Member

What happens if a dependency is target-specific by one path through the dependency tree, but not target-specific by another path through the dependency tree?

@illicitonion
Copy link
Contributor Author

What happens if a dependency is target-specific by one path through the dependency tree, but not target-specific by another path through the dependency tree?

Currently not the correct thing! I've locally added a test that it should either represent that as one unified platform:

"dep_kinds": [
  {
    "kind": null,
    "target": null
  }
]

or noting that there were multiple platforms that led to this dep:

"dep_kinds": [
  {
    "kind": null,
    "target": "x86_64-apple-darwin"
  },
  {
    "kind": null,
    "target": null
  }
]

Working on updating the code - thanks for the feedback!

@illicitonion
Copy link
Contributor Author

@rustbot label +S-waiting-on-author -S-waiting-on-review

@rustbot
Copy link
Collaborator

rustbot commented Oct 26, 2021

Error: The feature relabel is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please let @rust-lang/release know if you're having trouble with this bot.

@joshtriplett joshtriplett added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2021
@illicitonion
Copy link
Contributor Author

After chatting with @ehuss (thanks!) we ascertained:

  1. The current metadata being output is correct for resolver v1, which didn't factor platform-feature combinations into its resolution.
  2. Resolver v2 would fix this, but there's currently no plan to expose resolver v2 output from metadata (see https://rust-lang.github.io/rfcs/2957-cargo-features2.html#cargo-metadata).

Closing this for now in favour of working out how to express resolver v2 output from cargo metadata.

bors added a commit that referenced this pull request Dec 18, 2022
Enable triagebot's relabel functionality

### What does this PR try to resolve?

This fixes the following failure that rustbot currently posts whenever someone tries to use "<b>`@</b><b>rustbot</b>` label" in this repository.

> **Error**: The feature `relabel` is not enabled in this repository.
> To enable it add its section in the `triagebot.toml` in the root of the repository.

Unauthenticated relabel has been enabled in rust-lang/rust for nearly 4 years. People overwhelmingly use it in good faith.

<br>

### How should we test and review this PR?

Compare against https://github.com/rust-lang/rust/blob/1.66.0/triagebot.toml.

Also skim through the 7 pages of labels on https://github.com/rust-lang/cargo/labels, whether it makes sense the ones I decided to allow arbitrary GitHub users to apply.

<br>

### Additional information

Attempted uses of "<b>`@</b><b>rustbot</b>` label", that failed, but this PR would allow:

- #10343 (comment)
- #10243 (comment)
- #9982 (comment)
- #9128 (comment)
- #9067 (comment)
- #8441 (comment)
- #11432 (comment)
- #8841 (comment)
- #10820 (comment)
- #10572 (comment)
- #9114 (comment)
- #8980 (comment)
- #9064 (comment)
- #8726 (comment)
- #8089 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo metadata doesn't indicate that feature-enabled transitive platform-specific deps are platform-specific
5 participants