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

Do not try to build LLVM with Zlib on Windows #85762

Merged
merged 1 commit into from
May 30, 2021

Conversation

mati865
Copy link
Contributor

@mati865 mati865 commented May 27, 2021

Fixes #85422
Fixes #85624

We do not install Zlib on the CI but recent builds somehow started picking it's shared version.
To avoid relying on CI binaries so let's explicitly disable it.

We do not install Zlib on the CI but recent builds somehow started picking it's shared version.
To avoid relying on CI binaries so let's explicitly disable it.
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 27, 2021
@mati865
Copy link
Contributor Author

mati865 commented May 27, 2021

This is untested, can I have @bors try please?

@tmiasko
Copy link
Contributor

tmiasko commented May 27, 2021

@bors try

@bors
Copy link
Contributor

bors commented May 27, 2021

⌛ Trying commit 0100e7ed0e8ce6f751a48a480d1ac6e5b99eafc4 with merge f0c2c0aa6ba5b8eceb6adf9bd81195452e550cc1...

@bors
Copy link
Contributor

bors commented May 27, 2021

☀️ Try build successful - checks-actions
Build commit: f0c2c0aa6ba5b8eceb6adf9bd81195452e550cc1 (f0c2c0aa6ba5b8eceb6adf9bd81195452e550cc1)

@@ -181,7 +181,7 @@ impl Step for Llvm {
.define("LLVM_TARGET_ARCH", target_native.split('-').next().unwrap())
.define("LLVM_DEFAULT_TARGET_TRIPLE", target_native);

if target != "aarch64-apple-darwin" {
if target != "aarch64-apple-darwin" && !target.contains("windows") {
Copy link
Contributor

Choose a reason for hiding this comment

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

this will also match other windows targets, for example x86_64-pc-windows-msvc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, currently windows-msvc doesn't link Zlib at all but who knows what is going to change within GitHub Actions CI...

@mati865
Copy link
Contributor Author

mati865 commented May 28, 2021

Okay this worked:

$ ntldd /c/Users/mateusz/.rustup/toolchains/f0c2c0aa6ba5b8eceb6adf9bd81195452e550cc1/lib/rustlib/x86_64-pc-windows-gnu/bin/rust-lld.exe
        ADVAPI32.dll => C:\Windows\SYSTEM32\ADVAPI32.dll (0x0000018861b80000)
        libgcc_s_seh-1.dll => D:\msys64\mingw64\bin\libgcc_s_seh-1.dll (0x0000018861960000)
        KERNEL32.dll => C:\Windows\SYSTEM32\KERNEL32.dll (0x0000018868b00000)
        msvcrt.dll => C:\Windows\SYSTEM32\msvcrt.dll (0x0000018868b00000)
        ole32.dll => C:\Windows\SYSTEM32\ole32.dll (0x0000018868b00000)
        libwinpthread-1.dll => D:\msys64\mingw64\bin\libwinpthread-1.dll (0x0000018861980000)
        SHELL32.dll => C:\Windows\SYSTEM32\SHELL32.dll (0x0000018868f90000)

$ /c/Users/mateusz/.rustup/toolchains/f0c2c0aa6ba5b8eceb6adf9bd81195452e550cc1/lib/rustlib/x86_64-pc-windows-gnu/bin/rust-lld -v
lld is a generic driver.
Invoke ld.lld (Unix), ld64.lld (macOS), lld-link (Windows), wasm-ld (WebAssembly) instead

For the completeness this is nightly (the error is misleading):

$ ntldd /c/Users/mateusz/.rustup/toolchains/nightly-x86_64-pc-windows-gnu/lib/rustlib/x86_64-pc-windows-gnu/bin/rust-lld.exe
        zlib1__.dll => not found
        ADVAPI32.dll => C:\Windows\SYSTEM32\ADVAPI32.dll (0x00000184c2800000)
        libgcc_s_seh-1.dll => D:\msys64\mingw64\bin\libgcc_s_seh-1.dll (0x00000184bb8d0000)
        KERNEL32.dll => C:\Windows\SYSTEM32\KERNEL32.dll (0x00000184c28b0000)
        msvcrt.dll => C:\Windows\SYSTEM32\msvcrt.dll (0x00000184c28b0000)
        ole32.dll => C:\Windows\SYSTEM32\ole32.dll (0x00000184c2800000)
        libwinpthread-1.dll => D:\msys64\mingw64\bin\libwinpthread-1.dll (0x00000184bb8f0000)
        SHELL32.dll => C:\Windows\SYSTEM32\SHELL32.dll (0x00000184c2d30000)

$ /c/Users/mateusz/.rustup/toolchains/nightly-x86_64-pc-windows-gnu/lib/rustlib/x86_64-pc-windows-gnu/bin/rust-lld
C:/Users/mateusz/.rustup/toolchains/nightly-x86_64-pc-windows-gnu/lib/rustlib/x86_64-pc-windows-gnu/bin/rust-lld.exe: error while loading shared libraries: libwinpthread-1.dll: cannot open shared object file: No such file or directory

@mati865 mati865 force-pushed the disable-zlib-on-windows branch from 0100e7e to 53bf79e Compare May 28, 2021 00:18
@nagisa
Copy link
Member

nagisa commented May 28, 2021

cc @rylev

@Mark-Simulacrum
Copy link
Member

Hmm... I'm worried that in the long run this may not be the right fix, in the sense that we should either link in zlib statically or otherwise avoid this. For now it seems OK though. @bors r+

@bors
Copy link
Contributor

bors commented May 29, 2021

📌 Commit 53bf79e has been approved by Mark-Simulacrum

@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 29, 2021
@mati865
Copy link
Contributor Author

mati865 commented May 30, 2021

The problem with Windows CI is it doesn't use Docker like Linux so building Zlib will slow down everything. Unless we use prebuilt Zlib from MSYS2 which we never did so far.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 30, 2021
…Mark-Simulacrum

Do not try to build LLVM with Zlib on Windows

Fixes rust-lang#85422
Fixes rust-lang#85624

We do not install Zlib on the CI but recent builds somehow started picking it's shared version.
To avoid relying on CI binaries so let's explicitly disable it.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 30, 2021
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#85285 (Add eslint checks to CI)
 - rust-lang#85709 (Use correct edition when parsing `:pat` matchers)
 - rust-lang#85762 (Do not try to build LLVM with Zlib on Windows)
 - rust-lang#85770 (Remove `--print unversioned-files` from rustdoc )
 - rust-lang#85781 (Add documentation for aarch64-apple-ios-sim target)
 - rust-lang#85801 (Add `String::extend_from_within`)
 - rust-lang#85817 (Fix a typo)
 - rust-lang#85818 (Don't drop `PResult` without handling the error)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 957badb into rust-lang:master May 30, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 30, 2021
@mati865 mati865 deleted the disable-zlib-on-windows branch April 1, 2022 15:14
akoeplinger added a commit to dotnet/llvm-project that referenced this pull request Dec 13, 2022
CMake is accidentally picking up zlib from the CI environment, see actions/runner-images#6627 (comment).

Disable zlib on Windows since we don't want the dependency there, this is also what Rust did a while back: rust-lang/rust#85762
akoeplinger added a commit to dotnet/llvm-project that referenced this pull request Dec 13, 2022
CMake is accidentally picking up zlib from the CI environment, see actions/runner-images#6627 (comment), the build log now has this line:
```
  -- Found ZLIB: C:/Strawberry/c/lib/libz.a (found version "1.2.11") 
```

Disable zlib on Windows since we don't want the dependency there, this is also what Rust did a while back: rust-lang/rust#85762
akoeplinger added a commit to dotnet/llvm-project that referenced this pull request Dec 14, 2022
CMake is accidentally picking up zlib from the CI environment, see actions/runner-images#6627 (comment), the build log now has this line:
```
  -- Found ZLIB: C:/Strawberry/c/lib/libz.a (found version "1.2.11")
```

Disable zlib on Windows since we don't want the dependency there, this is also what Rust did a while back: rust-lang/rust#85762

(cherry picked from commit ea445e4)
akoeplinger added a commit to dotnet/llvm-project that referenced this pull request Dec 14, 2022
CMake is accidentally picking up zlib from the CI environment, see actions/runner-images#6627 (comment), the build log now has this line:
```
  -- Found ZLIB: C:/Strawberry/c/lib/libz.a (found version "1.2.11")
```

Disable zlib on Windows since we don't want the dependency there, this is also what Rust did a while back: rust-lang/rust#85762

(cherry picked from commit ea445e4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
8 participants