Allow rust staticlib to work with MSVC's /WHOLEARCHIVE by ChrisDenton · Pull Request #129257 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation41 Commits2 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

ChrisDenton

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

try-job: dist-i686-msvc

@rustbot rustbot 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

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 18, 2024

@bors

…r=

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

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

@ChrisDenton

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

@rustbot

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

jieyouxu

Member

@jieyouxu jieyouxu left a comment • Loading

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.

@jieyouxu

@bors rollup=iffy (funny linker things)

@ChrisDenton

@ChrisDenton

Since the changes are cosmetic and check passed...

@bors r=jieyouxu

@bors

📌 Commit d4a14b1 has been approved by jieyouxu

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

Aug 19, 2024

mati865

//! It ensures we can use `/WHOLEARCHIVE` to link a rust staticlib into DLL
//! using the MSVC linker
//@ only-msvc

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.

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.

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?

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?

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)

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

@jieyouxu

…, r=jieyouxu

Allow rust staticlib to work with MSVC's /WHOLEARCHIVE

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

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 20, 2024

@bors

Rollup of 6 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

@jieyouxu

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-

@bors bors 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 bors 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

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

@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

Aug 21, 2024

@jieyouxu

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 22, 2024

@bors

…r=jieyouxu

Allow rust staticlib to work with MSVC's /WHOLEARCHIVE

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

try-job: dist-i686-msvc

@rust-log-analyzer

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 bors 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

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

@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

Aug 22, 2024

jieyouxu added a commit to jieyouxu/rust that referenced this pull request

Aug 22, 2024

@jieyouxu

…, r=jieyouxu

Allow rust staticlib to work with MSVC's /WHOLEARCHIVE

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

try-job: dist-i686-msvc

tgross35 added a commit to tgross35/rust that referenced this pull request

Aug 22, 2024

@tgross35

…, r=jieyouxu

Allow rust staticlib to work with MSVC's /WHOLEARCHIVE

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

try-job: dist-i686-msvc

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 22, 2024

@bors

Rollup of 7 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

@bors

@bors

@rust-timer

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.

mean range count
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.

mean range count
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.

mean range count
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.

mean range count
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%)

Labels

A-run-make

Area: port run-make Makefiles to rmake.rs

A-testsuite

Area: The testsuite used to check the correctness of rustc

CI-spurious-fail-msvc

CI spurious failure: target env msvc

merged-by-bors

This PR was explicitly merged by bors.

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

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.