Skip to content

Allow rust staticlib to work with MSVC's /WHOLEARCHIVE - #129257

Merged
bors merged 2 commits into
rust-lang:masterfrom
ChrisDenton:rename-null-descriptor
Aug 22, 2024
Merged

Allow rust staticlib to work with MSVC's /WHOLEARCHIVE#129257
bors merged 2 commits into
rust-lang:masterfrom
ChrisDenton:rename-null-descriptor

Conversation

@ChrisDenton

@ChrisDentonChrisDenton commented Aug 18, 2024

Copy link
Copy Markdown
Member

This fixes#129020 by renaming the __NULL_IMPORT_DESCRIPTOR to prevent conflicts.

try-job: dist-i686-msvc

@rustbotrustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 18, 2024
@ChrisDenton

Copy link
Copy Markdown
MemberAuthor

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 18, 2024
…r=<try>
Allow rust staticlib to work with MSVC's /WHOLEARCHIVE
This renames the `__NULL_IMPORT_DESCRIPTOR` to prevent conflicts.
try-job: x86_64-msvc
try-job: i686-msvc
r? ghost
@bors

bors commented Aug 18, 2024

Copy link
Copy Markdown
Collaborator

⌛ Trying commit f64ecf1 with merge 04f2fce...

@bors

bors commented Aug 19, 2024

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions
Build commit: 04f2fce (04f2fcee5f890f2ff221d91b27754fb2fb2ba779)

@ChrisDenton

Copy link
Copy Markdown
MemberAuthor

The ar_archive_writer crate has been update so this should be ready for review. Hopefully the rmake test is at least a bit nicer now,

r? jieyouxu

@ChrisDenton
ChrisDenton marked this pull request as ready for review August 19, 2024 17:52
@rustbot

Copy link
Copy Markdown
Collaborator

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

@jieyouxujieyouxu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this LGTM, feel free to r=me with or without addressing the nit.

Comment threadcompiler/rustc_codegen_ssa/src/back/archive.rs
Comment threadtests/run-make/msvc-wholearchive/rmake.rs Outdated
Comment threadtests/run-make/msvc-wholearchive/rmake.rs Outdated
@jieyouxu

Copy link
Copy Markdown
Member

@bors rollup=iffy (funny linker things)

@ChrisDenton
ChrisDentonforce-pushed the rename-null-descriptor branch from 50a28af to d4a14b1CompareAugust 19, 2024 18:27
@ChrisDenton

Copy link
Copy Markdown
MemberAuthor

Since the changes are cosmetic and check passed...

@bors r=jieyouxu

@bors

bors commented Aug 19, 2024

Copy link
Copy Markdown
Collaborator

📌 Commit d4a14b1 has been approved by jieyouxu

It is now in the queue for this repository.

@borsbors 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 Aug 19, 2024
//! It ensures we can use `/WHOLEARCHIVE` to link a rust staticlib into DLL
//! using the MSVC linker

//@ only-msvc

@mati865mati865Aug 19, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would be nice (but not required) to also cover windows-gnullvm that has the same issue.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I've not tried gnullvm specifically but the llvm linker has another issue which I think needs solving on their end first (which is why even the msvc test skips lld-link).

.idata$4 should not refer to special section 0

LLVM seems to not properly recognise .idata$4 and .idata$5 as special unless they're specifically in short import libraries. incidentally this also makes it break on MSVC long import libraries. Though since they haven't been in use since the 90's I guess there's not been much pressure to support it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We probably should open a new issue to track this case?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Maybe? How do we usually track upstream issues?

@jieyouxujieyouxuAug 20, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have a special label for waiting on LLVM: S-waiting-for-llvm for describing the dragon is eepy (and better if it links to a llvm upstream issue)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Though since they haven't been in use since the 90's I guess there's not been much pressure to support it.

ld.bfd used by windows-gnu still uses them as it still cannot output short import libraries 😉

jieyouxu added a commit to jieyouxu/rust that referenced this pull request Aug 20, 2024
…, r=jieyouxu
Allow rust staticlib to work with MSVC's /WHOLEARCHIVE
This fixesrust-lang#129020 by renaming the `__NULL_IMPORT_DESCRIPTOR` to prevent conflicts.
@jieyouxujieyouxu mentioned this pull request Aug 20, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 20, 2024
Rollup of 6 pull requests
Successful merges:
- rust-lang#127623 (fix: fs::remove_dir_all: treat internal ENOENT as success)
- rust-lang#128627 (Special case DUMMY_SP to emit line 0/column 0 locations on DWARF platforms.)
- rust-lang#129187 (bootstrap: fix clean's remove_dir_all implementation)
- rust-lang#129190 (Added f16 and f128 to tests/ui/consts/const-float-bits-conv.rs)
- rust-lang#129231 (improve submodule updates)
- rust-lang#129257 (Allow rust staticlib to work with MSVC's /WHOLEARCHIVE)
r? `@ghost`
`@rustbot` modify labels: rollup
@jieyouxu

jieyouxu commented Aug 20, 2024

Copy link
Copy Markdown
Member

I'm suspecting this PR caused the failure in #129300 (comment):

Caused by:
process didn't exit successfully: `C:\a\rust\rust\build\i686-pc-windows-msvc\stage1-rustc\release\build\crossbeam-utils-7d693aa43cb4fd79\build-script-build` (exit code: 0xc0000135, STATUS_DLL_NOT_FOUND)
[RUSTC-TIMING] smallvec test:false 0.460
error: failed to run custom build command for `proc-macro2 v1.0.86`
Caused by:
Caused by:
process didn't exit successfully: `C:\a\rust\rust\build\i686-pc-windows-msvc\stage1-rustc\release\build\proc-macro2-6de43c34e6077ea1\build-script-build` (exit code: 0xc0000135, STATUS_DLL_NOT_FOUND)

Temporarily kicking this out of the queue to check if it's this or another bootstrap PR.

@bors r-

@borsbors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 20, 2024
@bors

bors commented Aug 21, 2024

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@borsbors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 21, 2024
@ChrisDenton

Copy link
Copy Markdown
MemberAuthor

@bors retry spurious "failed to remove file" on x86_64-msvc-ext again

@borsbors 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 Aug 21, 2024
@jieyouxu

Copy link
Copy Markdown
Member

pinging back #127883

@bors

bors commented Aug 22, 2024

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 40af214 with merge 5dbffc5...

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2024
…r=jieyouxu
Allow rust staticlib to work with MSVC's /WHOLEARCHIVE
This fixesrust-lang#129020 by renaming the `__NULL_IMPORT_DESCRIPTOR` to prevent conflicts.
try-job: dist-i686-msvc
@rust-log-analyzer

Copy link
Copy Markdown
Collaborator

The job x86_64-msvc-ext failed! Check out the build log: (web)(plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] miri test:false 4.390
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`
Caused by:
Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
local time: Thu, Aug 22, 2024 3:16:53 AM
network time: Thu, 22 Aug 2024 03:16:54 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors

bors commented Aug 22, 2024

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@borsbors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 22, 2024
@jieyouxu

Copy link
Copy Markdown
Member

@bors retry (#127883 windows ci failure 2: electric boogaloo)

@borsbors 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 Aug 22, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Aug 22, 2024
…, r=jieyouxu
Allow rust staticlib to work with MSVC's /WHOLEARCHIVE
This fixesrust-lang#129020 by renaming the `__NULL_IMPORT_DESCRIPTOR` to prevent conflicts.
try-job: dist-i686-msvc
@jieyouxujieyouxu mentioned this pull request Aug 22, 2024
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 22, 2024
…, r=jieyouxu
Allow rust staticlib to work with MSVC's /WHOLEARCHIVE
This fixesrust-lang#129020 by renaming the `__NULL_IMPORT_DESCRIPTOR` to prevent conflicts.
try-job: dist-i686-msvc
@tgross35tgross35 mentioned this pull request Aug 22, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2024
Rollup of 7 pull requests
Successful merges:
- rust-lang#126985 (Implement `-Z embed-source` (DWARFv5 source code embedding extension))
- rust-lang#128349 (Enable `f16` on x86 and x86-64)
- rust-lang#128876 (Ship MinGW-w64 runtime DLLs along with `rust-lld.exe` for `-pc-windows-gnu` targets)
- rust-lang#129190 (Add f16 and f128 to tests/ui/consts/const-float-bits-conv.rs)
- rust-lang#129257 (Allow rust staticlib to work with MSVC's /WHOLEARCHIVE)
- rust-lang#129386 (Use a LocalDefId in ResolvedArg.)
- rust-lang#129400 (Update `compiler_builtins` to `0.1.120`)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors

bors commented Aug 22, 2024

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 40af214 with merge 5ad98b4...

@bors

bors commented Aug 22, 2024

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing 5ad98b4 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Aug 22, 2024
@bors
bors merged commit 5ad98b4 into rust-lang:masterAug 22, 2024
@rustbotrustbot added this to the 1.82.0 milestone Aug 22, 2024
@ChrisDenton
ChrisDenton deleted the rename-null-descriptor branch August 22, 2024 18:18
@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (5ad98b4): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-1.7%[-1.7%, -1.7%]1
All ❌✅ (primary)--0

Max RSS (memory usage)

Results (primary 2.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)
2.7%[2.7%, 2.7%]1
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
--0
All ❌✅ (primary)2.7%[2.7%, 2.7%]1

Cycles

Results (secondary 9.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
9.6%[9.6%, 9.6%]1
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
--0
All ❌✅ (primary)--0

Binary size

Results (secondary -0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-0.0%[-0.0%, -0.0%]1
All ❌✅ (primary)--0

Bootstrap: 749.851s -> 749.145s (-0.09%)
Artifact size: 338.94 MiB -> 338.93 MiB (-0.00%)

@jieyouxujieyouxu added the CI-spurious-fail-msvc CI spurious failure: target env msvc label Oct 15, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-run-makeArea: port run-make Makefiles to rmake.rsA-testsuiteArea: The testsuite used to check the correctness of rustcCI-spurious-fail-msvcCI spurious failure: target env msvcmerged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

Rust 1.79+ stdlib is unusable with MSVC when creating static libraries

7 participants

@ChrisDenton@bors@rustbot@jieyouxu@rust-log-analyzer@rust-timer@mati865