Skip to content

do not pass cdylib link args to test #15317

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
Merged
merged 1 commit into from
Mar 25, 2025
Merged

do not pass cdylib link args to test #15317

merged 1 commit into from
Mar 25, 2025

Conversation

usamoi
Copy link
Contributor

@usamoi usamoi commented Mar 16, 2025 •
edited
Loading

fixes #12663

@rustbot
Copy link
Collaborator

rustbot commented Mar 16, 2025

r? @epage

rustbot has assigned @epage.
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 A-build-execution Area: anything dealing with executing the compiler S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 16, 2025
@usamoi usamoi marked this pull request as draft March 16, 2025 14:35
@usamoi usamoi force-pushed the fix-issue-12666 branch 2 times, most recently from 4b8a167 to 1609e02 Compare March 16, 2025 16:23
@rustbot rustbot added the A-build-scripts Area: build.rs scripts label Mar 16, 2025
@usamoi usamoi marked this pull request as ready for review March 16, 2025 16:37
@epage
Copy link
Contributor

epage commented Mar 17, 2025

r? ehuss

Since you have more of the background on this

@rustbot rustbot assigned ehuss and unassigned epage Mar 17, 2025
@ehuss
Copy link
Contributor

ehuss commented Mar 18, 2025

Thanks!

I'm a little uncertain about changing Bin, SingleBin, and Example. My thinking is that if you have a flag that is required to link a bin, then the flag would also likely be required to link a unittest of the bin (since they are mostly the same thing). Can you say more about why those are being changed?

ehuss
ehuss reviewed Mar 18, 2025
View reviewed changes
weihanglo
weihanglo reviewed Mar 19, 2025
View reviewed changes
@rustbot

This comment has been minimized.

Sign in to view
@usamoi
Copy link
Contributor Author

usamoi commented Mar 19, 2025

I'm a little uncertain about changing Bin, SingleBin, and Example. My thinking is that if you have a flag that is required to link a bin, then the flag would also likely be required to link a unittest of the bin (since they are mostly the same thing). Can you say more about why those are being changed?

I initially assumed they were different based on intuition.

I recalled a case where a binary was compiled for a bare-metal target, and I used custom test frameworks to run tests in the same environment. If the current behavior changes, similar use cases could break.

So, I reverted to the previous behavior for Bin, SingleBin and Example.

ehuss
ehuss approved these changes Mar 25, 2025
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!

@ehuss ehuss added this pull request to the merge queue Mar 25, 2025
Merged via the queue into rust-lang:master with commit 6a70640 Mar 25, 2025
21 checks passed
@usamoi usamoi deleted the fix-issue-12666 branch March 26, 2025 02:21
@weihanglo weihanglo mentioned this pull request Mar 26, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2025
@bors
Auto merge of rust-lang#139005 - weihanglo:update-cargo, r=weihanglo
2216f26 
Update cargo

10 commits in 307cbfda3119f06600e43cd38283f4a746fe1f8b..a6c604d1b8a2f2a8ff1f3ba6092f9fda42f4b7e9
2025-03-20 20:00:39 +0000 to 2025-03-26 18:11:00 +0000
- fix(package): update tracking issue for `--message-format` (rust-lang/cargo#15354)
- docs(contrib): Expand the description of team meetings (rust-lang/cargo#15349)
- feat(package): add unstable `--message-format` flag  (rust-lang/cargo#15311)
- feat(complete): Added completion for `--profile` (rust-lang/cargo#15308)
- Uplift windows Cygwin DLL import libraries (rust-lang/cargo#15193)
- do not pass cdylib link args to test (rust-lang/cargo#15317)
- fix: revert the behavior checking lockfile's VCS status (rust-lang/cargo#15341)
- Temporarily ignore cargo_test_doctest_xcompile_ignores (rust-lang/cargo#15348)
- docs: fix typo in the "Shared cache" section (rust-lang/cargo#15346)
- Fix some issues with future-incompat report generation (rust-lang/cargo#15345)

r? ghost
@rustbot rustbot added this to the 1.87.0 milestone Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo left review comments

@ehuss ehuss ehuss approved these changes

Assignees

@ehuss ehuss

Labels
A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
1.87.0
Development

Successfully merging this pull request may close these issues.

cdylib extra link args apply to unit tests
5 participants
@usamoi @rustbot @epage @ehuss @weihanglo