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.

Sign up for GitHub

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

Jump to bottom

Stabilize the rustc-link-arg option #9557

Merged
merged 1 commit into from
Jul 26, 2021
Merged

Stabilize the rustc-link-arg option #9557

merged 1 commit into from
Jul 26, 2021

Conversation

danielframpton
Copy link
Contributor

This change removes the unstable option (tracked by #9426) and unconditionally accepts additional linker arguments (as implemented in #7811 and #8441). Documentation is moved from unstable to what appeared to be the correct location.

I am not aware of any significant concerns with the option and it appears consistent with some other existing stable linker options.

Please let me know if this is not the appropriate process or if there is anything that I am missing from the PR.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Eh2406 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 8, 2021
@ehuss ehuss mentioned this pull request Jun 10, 2021
Closed
@ehuss
Copy link
Contributor

ehuss commented Jun 22, 2021

Thanks for the PR! Sorry, we haven't been ignoring this. There were some issues with the implementation that we were trying to resolve (such as #9563). We will try to review this soon, though I think the team was generally favorable to moving forward.

@ehuss
Copy link
Contributor

ehuss commented Jun 22, 2021

The CI error can be resolved by rebasing on the latest master.

@danielframpton
Copy link
Contributor Author

Thanks for the update @ehuss

Let me know if there are any questions or other issues that I could help out with here as well.

@Hywan Hywan mentioned this pull request Jul 20, 2021
Closed
1 task
ehuss
ehuss reviewed Jul 21, 2021
View reviewed changes
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, sorry about the long delay.

src/doc/src/reference/unstable.md Show resolved Hide resolved
src/cargo/core/features.rs Show resolved Hide resolved
@ehuss ehuss assigned ehuss and unassigned Eh2406 Jul 21, 2021
@ehuss ehuss added the T-cargo Team: Cargo label Jul 21, 2021
@ehuss
Copy link
Contributor

ehuss commented Jul 21, 2021

@rfcbot fcp merge

@rust-lang/cargo, this is a stabilization for the extra-link-arg feature. This allows build scripts to pass additional linker arguments to rustc. This has been historically allowed for cdylibs, and this stabilizes for some other crate types:

  • cargo:rustc-link-arg-bins=FLAG: Pass the flag for all [[bin]] targets.
  • cargo:rustc-link-arg-bin=BIN=FLAG: Pass the flag for a specific [[bin]] target.
  • cargo:rustc-link-arg=FLAG: Pass the flag for all cargo targets of this package.

@rfcbot
Copy link
Collaborator

rfcbot commented Jul 21, 2021 •
edited by joshtriplett
Loading

Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Jul 21, 2021
@joshtriplett
Copy link
Member

Just to confirm, before stabilizing this: all of these only apply to the current crate, not to crates depending on the current crate, right?

@ehuss
Copy link
Contributor

ehuss commented Jul 21, 2021

Correct.

@rfcbot rfcbot added final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels Jul 21, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Jul 21, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@danielframpton
Copy link
Contributor Author

Thanks @ehuss for the review.

I have updated the change, let me know if there are any other further tweaks I should make before we land this.

@ehuss
Copy link
Contributor

ehuss commented Jul 26, 2021

Thanks! 🎉

@bors r+

@bors
Copy link
Contributor

bors commented Jul 26, 2021

📌 Commit e737039 has been approved by ehuss

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 26, 2021
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 26, 2021
@bors
Copy link
Contributor

bors commented Jul 26, 2021

⌛ Testing commit e737039 with merge 97007c9...

@bors
Copy link
Contributor

bors commented Jul 26, 2021

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 97007c9 to master...

@bors bors merged commit 97007c9 into rust-lang:master Jul 26, 2021
@ehuss ehuss mentioned this pull request Jul 27, 2021
Merged
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 27, 2021
@JohnTitor
Rollup merge of rust-lang#87502 - ehuss:update-cargo, r=ehuss
8a99796 
Update cargo

8 commits in cebef2951ee69617852844894164b54ed478a7da..d21c22870e58499d6c31f1bef3bf1255eb021666
2021-07-22 13:01:52 +0000 to 2021-07-26 20:23:21 +0000
- Fix version string. (rust-lang/cargo#9727)
- Allow publishing from workspace root. (rust-lang/cargo#9559)
- Better msg for wrong position (rust-lang/cargo#9723)
- Stabilize the rustc-link-arg option (rust-lang/cargo#9557)
- Warning when using features in replace (rust-lang/cargo#9681)
- Refactor if let chains to matches! macro (rust-lang/cargo#9721)
- Weather is not nice today.. (rust-lang/cargo#9720)
- Update should_use_metadata function (rust-lang/cargo#9653)
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 27, 2021
@JohnTitor
Rollup merge of rust-lang#87502 - ehuss:update-cargo, r=ehuss
bea8af9 
Update cargo

8 commits in cebef2951ee69617852844894164b54ed478a7da..d21c22870e58499d6c31f1bef3bf1255eb021666
2021-07-22 13:01:52 +0000 to 2021-07-26 20:23:21 +0000
- Fix version string. (rust-lang/cargo#9727)
- Allow publishing from workspace root. (rust-lang/cargo#9559)
- Better msg for wrong position (rust-lang/cargo#9723)
- Stabilize the rustc-link-arg option (rust-lang/cargo#9557)
- Warning when using features in replace (rust-lang/cargo#9681)
- Refactor if let chains to matches! macro (rust-lang/cargo#9721)
- Weather is not nice today.. (rust-lang/cargo#9720)
- Update should_use_metadata function (rust-lang/cargo#9653)
@crawfxrd crawfxrd mentioned this pull request Jul 27, 2021
Closed
@joshtriplett joshtriplett added the relnotes Release-note worthy label Jul 30, 2021
@rfcbot rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Jul 31, 2021
@LaurentMazare LaurentMazare mentioned this pull request Jul 31, 2021
Open
@ehuss ehuss mentioned this pull request Aug 3, 2021
Closed
@scott-wilson scott-wilson mentioned this pull request Aug 28, 2021
Open
@davepacheco davepacheco mentioned this pull request Sep 1, 2021
Closed
@ohmree ohmree mentioned this pull request Sep 3, 2021
Open
@davepacheco davepacheco mentioned this pull request Sep 3, 2021
Closed
@LaurentMazare LaurentMazare mentioned this pull request Oct 9, 2021
Closed
@tronical tronical mentioned this pull request Oct 12, 2021
Open
@ehuss ehuss added this to the 1.56.0 milestone Feb 6, 2022
@jonahbeckford jonahbeckford mentioned this pull request Jun 18, 2022
Open
This was referenced Oct 13, 2022
Merged
Open
Open
@mibu138 mibu138 mentioned this pull request Mar 5, 2023
Open
@ghost ghost mentioned this pull request Oct 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehuss ehuss ehuss left review comments

Assignees

@ehuss ehuss

Labels
disposition-merge FCP with intent to merge finished-final-comment-period FCP complete relnotes Release-note worthy S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
None yet
Milestone
1.56.0
Development

Successfully merging this pull request may close these issues.
7 participants
@danielframpton @rust-highfive @ehuss @rfcbot @joshtriplett @bors @Eh2406