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

Remove unstable cfg target(...) compact feature #130780

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Sep 24, 2024

This PR removes the unstable cfg_target_compact feature from #96901, which permits compact target(...) cfgs (e.g. cfg(target(os = "linux")) -> cfg(target_os = "linux")).

The feature:

  • is only used in one crate: GitHub Search (not even in the compiler)
  • has received no activity on the tracking issue
  • has not received any stabilization request since it's nightly introduction (~2yrs ago)
  • was part of a bigger feature that was already withdrawn
  • is limited to target_* cfgs
  • it's not implemented by Cargo

All this to say that I don't think the feature pulls its weight.

@rustbot label +I-lang-nominated
Closes #96901

@rustbot
Copy link
Collaborator

rustbot commented Sep 24, 2024

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. labels Sep 24, 2024
@lqd
Copy link
Member

lqd commented Sep 24, 2024

Wasn’t this helping to remove some use cases for build scripts?

@Urgau
Copy link
Member Author

Urgau commented Sep 24, 2024

The part that was supposed to help with build script was the cfg(target = "...") part of the RFC, but that part was already withdrawn.

@lqd
Copy link
Member

lqd commented Sep 24, 2024

IIRC the target abi was only matchable in build scripts, is that still the case today (with and without this PR)? To be able to lower the need for build scripts in the ecosystem we need cfgs to be at least isomorphic…

@Urgau
Copy link
Member Author

Urgau commented Sep 24, 2024

This PR doesn't change anything for matching on specific target cfgs components. This PR only removes the shorthand cfg(target(os = "linux")) -> cfg(target_os = "linux").

As for the target abi, the target_abi cfg was stabilized in 1.78.

@lqd
Copy link
Member

lqd commented Sep 24, 2024

Good. Thank you for the clarifications.

@bors

This comment was marked as resolved.

@Urgau Urgau force-pushed the withdrawn-cfg-target-compact branch 2 times, most recently from 79578af to 9832308 Compare October 5, 2024 07:56
@bors

This comment was marked as resolved.

@Urgau Urgau force-pushed the withdrawn-cfg-target-compact branch from 9832308 to 1de1d84 Compare October 7, 2024 12:50
@bors

This comment was marked as resolved.

@Urgau Urgau force-pushed the withdrawn-cfg-target-compact branch from 1de1d84 to 1f2c377 Compare October 16, 2024 15:38
@bors
Copy link
Contributor

bors commented Oct 23, 2024

☔ The latest upstream changes (presumably #132027) made this pull request unmergeable. Please resolve the merge conflicts.

@BoxyUwU BoxyUwU added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2024
@Urgau Urgau force-pushed the withdrawn-cfg-target-compact branch from 1f2c377 to 7c50b97 Compare October 27, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-lang-nominated Nominated for discussion during a lang team meeting. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue for RFC 3239: Add target configuration
5 participants