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 libc from MSVC targets #124050

Merged
merged 10 commits into from
May 21, 2024
Merged

Remove libc from MSVC targets #124050

merged 10 commits into from
May 21, 2024

Conversation

saethlin
Copy link
Member

@saethlin saethlin commented Apr 17, 2024

@ChrisDenton started working on a project to remove libc from Windows MSVC targets. I'm completing that work here.

The primary change is to cfg out the dependency in library/. And then there's a lot of test patching. Happy to separate this more if people want.

@rustbot
Copy link
Collaborator

rustbot commented Apr 17, 2024

r? @compiler-errors

rustbot has assigned @compiler-errors.
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. labels Apr 17, 2024
@saethlin saethlin added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2024
@bors
Copy link
Contributor

bors commented Apr 17, 2024

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

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc PG-exploit-mitigations Project group: Exploit mitigations T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Apr 17, 2024
@rust-log-analyzer

This comment has been minimized.

@saethlin saethlin force-pushed the less-sysroot-libc branch 3 times, most recently from 0a688d3 to 45d4e81 Compare April 18, 2024 16:13
@saethlin saethlin changed the title Remove libc from more tests Remove libc from MSVC targets Apr 18, 2024
@bors
Copy link
Contributor

bors commented Apr 18, 2024

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

@saethlin
Copy link
Member Author

@ChrisDenton Is there anything else that needs to be done to remove libc from MSVC targets? I feel like this was almost too easy.

@saethlin saethlin added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Apr 18, 2024
@ChrisDenton
Copy link
Contributor

There's also #123976.

Also I found the std_detect crate is pulling in libc even if unused. See std uses the default features "std_detect_file_io", and "std_detect_dlsym_getauxval"

default = ["std_detect_file_io", "std_detect_dlsym_getauxval", "panic-unwind"]

Which in the std-detect crate enables the libc feature which in turn enables the libc dependency: https://github.com/rust-lang/stdarch/blob/7df81ba8c3e2d02c2ace0c5a6f4f32d800c09e56/crates/std_detect/Cargo.toml#L37

However, libc isn't used for those two features on Windows (it is currently used for a third, not default enabled. feature). But I'm not sure how to have a dependency enabled when it's both feature and target specific.

@ChrisDenton
Copy link
Contributor

Rebasing on this PR and excluding libc from std_detect I'm still seeing the following UI test failures:

failures:
    [ui] tests\ui\attributes\unix_sigpipe\unix_sigpipe-inherit.rs
    [ui] tests\ui\attributes\unix_sigpipe\unix_sigpipe-not-used.rs#without_feature
    [ui] tests\ui\attributes\unix_sigpipe\unix_sigpipe-not-used.rs#with_feature
    [ui] tests\ui\attributes\unix_sigpipe\unix_sigpipe-sig_dfl.rs
    [ui] tests\ui\attributes\unix_sigpipe\unix_sigpipe-rustc_main.rs
    [ui] tests\ui\attributes\unix_sigpipe\unix_sigpipe-sig_ign.rs
    [ui] tests\ui\error-codes\E0259.rs
    [ui] tests\ui\feature-gates\rustc-private.rs
    [ui] tests\ui\foreign\foreign-fn-linkname.rs
    [ui] tests\ui\foreign\foreign2.rs
    [ui] tests\ui\imports\issue-37887.rs
    [ui] tests\ui\lint\unnecessary-extern-crate.rs
    [ui] tests\ui\meta\no_std-extern-libc.rs
    [ui] tests\ui\process\no-stdio.rs
    [ui] tests\ui\runtime\stdout-during-shutdown.rs
    [ui] tests\ui\wait-forked-but-failed-child.rs

@saethlin
Copy link
Member Author

Ah-ha! That makes more sense.

@saethlin
Copy link
Member Author

saethlin commented Apr 18, 2024

Now we should be down to

    [ui] tests\ui\foreign\foreign2.rs
    [ui] tests\ui\foreign\foreign-fn-linkname.rs
    [ui] tests\ui\runtime\stdout-during-shutdown.rs
    [ui] tests\ui\wait-forked-but-failed-child.rs

These tests are all pretty unix-y, but I'm going to educate myself a bit and see if there's a way to make a windows equivalent in some cases.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Apr 21, 2024

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

@saethlin
Copy link
Member Author

Fix for the nomicon: rust-lang/nomicon#450

bors added a commit to rust-lang-ci/rust that referenced this pull request May 19, 2024
CI: fix toolstate publishing

Toolstate publishing after something broke was not working (discovered [here](rust-lang#124050 (comment))). The toolstate env. vars should only be needed for the publishing step, so I moved them there.

The toolstate script is also being checked in `mingw-check` on PR and auto CI, but it doesn't really seem to do anything, and it shouldn't require the token.
@bors
Copy link
Contributor

bors commented May 20, 2024

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

@saethlin
Copy link
Member Author

@bors r=ChrisDenton

@bors
Copy link
Contributor

bors commented May 20, 2024

📌 Commit 39ef149 has been approved by ChrisDenton

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 20, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 20, 2024
…sDenton

Remove libc from MSVC targets

`@ChrisDenton` started working on a project to remove libc from Windows MSVC targets. I'm completing that work here.

The primary change is to cfg out the dependency in `library/`. And then there's a lot of test patching. Happy to separate this more if people want.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#124050 (Remove libc from MSVC targets)
 - rust-lang#124283 (Note for E0599 if shadowed bindings has the method.)
 - rust-lang#125123 (Fix `read_exact` and `read_buf_exact` for `&[u8]` and `io:Cursor`)
 - rust-lang#125158 (hir pretty: fix block indent)
 - rust-lang#125298 (Add codegen test for array comparision opt)
 - rust-lang#125332 (Update books)

Failed merges:

 - rust-lang#125310 (Move ~100 tests from tests/ui to subdirs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#124050 (Remove libc from MSVC targets)
 - rust-lang#124283 (Note for E0599 if shadowed bindings has the method.)
 - rust-lang#125123 (Fix `read_exact` and `read_buf_exact` for `&[u8]` and `io:Cursor`)
 - rust-lang#125158 (hir pretty: fix block indent)
 - rust-lang#125308 (track cycle participants per root)
 - rust-lang#125332 (Update books)
 - rust-lang#125333 (switch to the default implementation of `write_vectored`)
 - rust-lang#125346 (Remove some `Path::to_str` from `rustc_codegen_llvm`)

Failed merges:

 - rust-lang#125310 (Move ~100 tests from tests/ui to subdirs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8903de3 into rust-lang:master May 21, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2024
Rollup merge of rust-lang#124050 - saethlin:less-sysroot-libc, r=ChrisDenton

Remove libc from MSVC targets

``@ChrisDenton`` started working on a project to remove libc from Windows MSVC targets. I'm completing that work here.

The primary change is to cfg out the dependency in `library/`. And then there's a lot of test patching. Happy to separate this more if people want.
@saethlin saethlin deleted the less-sysroot-libc branch May 21, 2024 03:39
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 24, 2024
CI: fix toolstate publishing

Toolstate publishing after something broke was not working (discovered [here](rust-lang/rust#124050 (comment))). The toolstate env. vars should only be needed for the publishing step, so I moved them there.

The toolstate script is also being checked in `mingw-check` on PR and auto CI, but it doesn't really seem to do anything, and it shouldn't require the token.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants