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

rls-preview missing Rust 1.29 for x86_64-pc-windows-gnu #54206

Closed
willem66745 opened this issue Sep 13, 2018 · 27 comments
Closed

rls-preview missing Rust 1.29 for x86_64-pc-windows-gnu #54206

willem66745 opened this issue Sep 13, 2018 · 27 comments
Labels
C-bug Category: This is a bug. O-windows-gnu Toolchain: GNU, Operating system: Windows regression-from-stable-to-stable Performance or correctness regression from one stable version to another. stable-accepted Accepted for backporting to the compiler in the stable channel. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue.

Comments

@willem66745
Copy link

See also rust-lang/rls#993

RLS was still available with Rust 1.28 for x86_64-pc-windows-gnu

For Rust 1.29:

$ rustup component list --toolchain=stable-x86_64-pc-windows-gnu                                                                                         
cargo-x86_64-pc-windows-gnu (default)
clippy-preview-x86_64-pc-windows-gnu
llvm-tools-preview-x86_64-pc-windows-gnu
rust-analysis-x86_64-pc-windows-gnu
rust-docs-x86_64-pc-windows-gnu (default)
rust-mingw-x86_64-pc-windows-gnu (default)
rust-src
rust-std-aarch64-apple-ios
rust-std-aarch64-fuchsia
rust-std-aarch64-linux-android
rust-std-aarch64-unknown-linux-gnu
rust-std-aarch64-unknown-linux-musl
rust-std-arm-linux-androideabi
rust-std-arm-unknown-linux-gnueabi
rust-std-arm-unknown-linux-gnueabihf
rust-std-arm-unknown-linux-musleabi
rust-std-arm-unknown-linux-musleabihf
rust-std-armv5te-unknown-linux-gnueabi
rust-std-armv5te-unknown-linux-musleabi
rust-std-armv7-apple-ios
rust-std-armv7-linux-androideabi
rust-std-armv7-unknown-linux-gnueabihf
rust-std-armv7-unknown-linux-musleabihf
rust-std-armv7s-apple-ios
rust-std-asmjs-unknown-emscripten
rust-std-i386-apple-ios
rust-std-i586-pc-windows-msvc
rust-std-i586-unknown-linux-gnu
rust-std-i586-unknown-linux-musl
rust-std-i686-apple-darwin
rust-std-i686-linux-android
rust-std-i686-pc-windows-gnu
rust-std-i686-pc-windows-msvc
rust-std-i686-unknown-freebsd
rust-std-i686-unknown-linux-gnu
rust-std-i686-unknown-linux-musl
rust-std-mips-unknown-linux-gnu
rust-std-mips-unknown-linux-musl
rust-std-mips64-unknown-linux-gnuabi64
rust-std-mips64el-unknown-linux-gnuabi64
rust-std-mipsel-unknown-linux-gnu
rust-std-mipsel-unknown-linux-musl
rust-std-powerpc-unknown-linux-gnu
rust-std-powerpc64-unknown-linux-gnu
rust-std-powerpc64le-unknown-linux-gnu
rust-std-s390x-unknown-linux-gnu
rust-std-sparc64-unknown-linux-gnu
rust-std-sparcv9-sun-solaris
rust-std-thumbv6m-none-eabi
rust-std-thumbv7em-none-eabi
rust-std-thumbv7em-none-eabihf
rust-std-thumbv7m-none-eabi
rust-std-wasm32-unknown-emscripten
rust-std-wasm32-unknown-unknown
rust-std-x86_64-apple-darwin
rust-std-x86_64-apple-ios
rust-std-x86_64-fuchsia
rust-std-x86_64-linux-android
rust-std-x86_64-pc-windows-gnu (default)
rust-std-x86_64-pc-windows-msvc
rust-std-x86_64-rumprun-netbsd
rust-std-x86_64-sun-solaris
rust-std-x86_64-unknown-cloudabi
rust-std-x86_64-unknown-freebsd
rust-std-x86_64-unknown-linux-gnu
rust-std-x86_64-unknown-linux-gnux32
rust-std-x86_64-unknown-linux-musl
rust-std-x86_64-unknown-netbsd
rust-std-x86_64-unknown-redox
rustc-x86_64-pc-windows-gnu (default)
rustfmt-preview-x86_64-pc-windows-gnu

For rust 1.28 it was available:

$ rustup component list --toolchain=1.28.0-x86_64-pc-windows-gnu          
cargo-x86_64-pc-windows-gnu (default)
**rls-preview-x86_64-pc-windows-gnu**
rust-analysis-x86_64-pc-windows-gnu
rust-docs-x86_64-pc-windows-gnu (default)
rust-mingw-x86_64-pc-windows-gnu (default)
rust-src
rust-std-aarch64-apple-ios
rust-std-aarch64-linux-android
rust-std-aarch64-unknown-fuchsia
rust-std-aarch64-unknown-linux-gnu
rust-std-aarch64-unknown-linux-musl
rust-std-arm-linux-androideabi
rust-std-arm-unknown-linux-gnueabi
rust-std-arm-unknown-linux-gnueabihf
rust-std-arm-unknown-linux-musleabi
rust-std-arm-unknown-linux-musleabihf
rust-std-armv5te-unknown-linux-gnueabi
rust-std-armv5te-unknown-linux-musleabi
rust-std-armv7-apple-ios
rust-std-armv7-linux-androideabi
rust-std-armv7-unknown-linux-gnueabihf
rust-std-armv7-unknown-linux-musleabihf
rust-std-armv7s-apple-ios
rust-std-asmjs-unknown-emscripten
rust-std-i386-apple-ios
rust-std-i586-pc-windows-msvc
rust-std-i586-unknown-linux-gnu
rust-std-i586-unknown-linux-musl
rust-std-i686-apple-darwin
rust-std-i686-linux-android
rust-std-i686-pc-windows-gnu
rust-std-i686-pc-windows-msvc
rust-std-i686-unknown-freebsd
rust-std-i686-unknown-linux-gnu
rust-std-i686-unknown-linux-musl
rust-std-mips-unknown-linux-gnu
rust-std-mips-unknown-linux-musl
rust-std-mips64-unknown-linux-gnuabi64
rust-std-mips64el-unknown-linux-gnuabi64
rust-std-mipsel-unknown-linux-gnu
rust-std-mipsel-unknown-linux-musl
rust-std-powerpc-unknown-linux-gnu
rust-std-powerpc64-unknown-linux-gnu
rust-std-powerpc64le-unknown-linux-gnu
rust-std-s390x-unknown-linux-gnu
rust-std-sparc64-unknown-linux-gnu
rust-std-sparcv9-sun-solaris
rust-std-thumbv6m-none-eabi
rust-std-thumbv7em-none-eabi
rust-std-thumbv7em-none-eabihf
rust-std-thumbv7m-none-eabi
rust-std-wasm32-unknown-emscripten
rust-std-wasm32-unknown-unknown
rust-std-x86_64-apple-darwin
rust-std-x86_64-apple-ios
rust-std-x86_64-linux-android
rust-std-x86_64-pc-windows-gnu (default)
rust-std-x86_64-pc-windows-msvc
rust-std-x86_64-rumprun-netbsd
rust-std-x86_64-sun-solaris
rust-std-x86_64-unknown-cloudabi
rust-std-x86_64-unknown-freebsd
rust-std-x86_64-unknown-fuchsia
rust-std-x86_64-unknown-linux-gnu
rust-std-x86_64-unknown-linux-gnux32
rust-std-x86_64-unknown-linux-musl
rust-std-x86_64-unknown-netbsd
rust-std-x86_64-unknown-redox
rustc-x86_64-pc-windows-gnu (default)
rustfmt-preview-x86_64-pc-windows-gnu
@estebank estebank added O-windows-gnu Toolchain: GNU, Operating system: Windows T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. A-rls labels Sep 13, 2018
@crlf0710
Copy link
Member

Can this be nominated? I think this worth a point one release.

@kennytm kennytm added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-release Relevant to the release subteam, which will review and decide on the PR/issue. labels Sep 14, 2018
@Kroc
Copy link

Kroc commented Sep 15, 2018

This broke Rust-in-VSCode for me :( Surely this should have been noticed before release

@pietroalbini pietroalbini added I-nominated T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Sep 16, 2018
@pietroalbini
Copy link
Member

cc @rust-lang/infra @rust-lang/release @rust-lang/dev-tools

@Mark-Simulacrum
Copy link
Member

Unfortunately I think we didn't notice this and I'm not sure if the fix is easy. @nrc is I think the primary point of contact for RLS, though.

@pietroalbini
Copy link
Member

We should probably have some checks on RCS to prevent publishing a beta/stable without tools though, at least for tier 1 platforms.

@Mark-Simulacrum
Copy link
Member

Yes; toolstate is intended to be that check but it doesn't track windows-gnu today. I'm not actually sure why, cc @kennytm

@pietroalbini
Copy link
Member

I don't think we use toolstate for macOS either.

@kennytm
Copy link
Member

kennytm commented Sep 16, 2018

@Mark-Simulacrum toolstate never checked the content of the dist builds, it only records the test result of the ./x.py test jobs on Linux-GNU and Windows-MSVC.

@Mark-Simulacrum
Copy link
Member

@kennytm Is there a reason we're limited to just those platforms, though? Can we expand that to include all tier-1 platforms?

@pietroalbini
Copy link
Member

I'd also add a check on RCS anyway, to make sure we're shipping all the components.

@kennytm
Copy link
Member

kennytm commented Sep 16, 2018

@Mark-Simulacrum it was not a requested feature 😅. I don't think our CI budget allows testing the tools on all tier-1 platforms, but we could certainly modify rustbuild to fail if RLS/Clippy etc cannot be built on beta and stable.

@Mark-Simulacrum
Copy link
Member

Hm, well, presumably we do (attempt) to build them so we could at least have that state recorded in toolstate, right?

@kennytm
Copy link
Member

kennytm commented Sep 17, 2018

@Mark-Simulacrum we could; nowadays the effort would be duplicating https://mexus.github.io/rustup-components-history/ though.

@mati865
Copy link
Contributor

mati865 commented Sep 17, 2018

@alexcrichton recently changed build system for curl-sys
and pthread seems not be linked anymore:

C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$destroy_thread_sync_data+0x1a): undefined reference to `pthread_mutex_destroy'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$getaddrinfo_thread+0x4a): undefined reference to `pthread_mutex_lock'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$getaddrinfo_thread+0x60): undefined reference to `pthread_mutex_unlock'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$getaddrinfo_thread+0x74): undefined reference to `pthread_mutex_unlock'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$destroy_async_data.isra.1+0x1b): undefined reference to `pthread_mutex_lock'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$destroy_async_data.isra.1+0x2e): undefined reference to `pthread_mutex_unlock'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$Curl_resolver_is_resolved+0x30): undefined reference to `pthread_mutex_lock'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$Curl_resolver_is_resolved+0x3d): undefined reference to `pthread_mutex_unlock'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-asyn-thread.o):asyn-thread.c:(.text$Curl_resolver_getaddrinfo+0x1d6): undefined reference to `pthread_mutex_init'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-curl_threads.o):curl_threads.c:(.text$Curl_thread_create+0x4a): undefined reference to `pthread_create'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-curl_threads.o):curl_threads.c:(.text$Curl_thread_destroy+0x11): undefined reference to `pthread_detach'
          C:\projects\rust\build\x86_64-pc-windows-gnu\stage2-tools\x86_64-pc-windows-gnu\release\deps\libcurl_sys-ea7e23a986c75ec7.rlib(libcurl_la-curl_threads.o):curl_threads.c:(.text$Curl_thread_join+0x12): undefined reference to `pthread_join'

Full error is here

@alexcrichton
Copy link
Member

Ok so as to what the underlying technical problem is here, thanks @mati865 for the ping and the error message. The RLS is failing to build on rustc 1.29.0+ because of the curl-sys. It looks like it's failing to link the RLS due to the missing pthread_* symbols. Cargo, which also depends on curl-sys, is somehow miraculously linking correctly. I'm not gonna dig into this though as curl-sys should not be depending on pthread symbols anyway.

The major difference between 1.28.0 (where this last build) and 1.29.0 is that in 1.29.0 we are using curl-sys 0.4.7 and in 1.28.0 we're using curl-sys 0.4.5. The changes in these versions notably include an update to the curl submodule. There's a ton of changes here, so presumably "some build system thing changed".

I believe this should be fixed on the most recent version of curl-sys which forgoes curl's build system for our own, which doesn't differentiate between MinGW/MSVC as much and doesn't do things like detect the pthread symbols. I will send a PR to the master branch to update the current nightly.

For 1.29.0 we would not want to do this, however, as it's a pretty significant change to the curl-sys crate. Instead a more localized fix would likely be to figure out how to inject -lpthread somewhere in the build. If we want to do a point release we should probably make a branch of the RLS which adds a build script that links to -lpthread on MinGW.

@nrc or @rust-lang/dev-tools do y'all have a feeling for how urgent such an issue like this is? I'll work to fix this on nightly (likely 1.31+, maybe 1.30 if we're lucky), but I'm not planning to figure out the 1.29 backport (if any) just yet

@alexcrichton
Copy link
Member

I've opened #54301 to update the curl-sys crate on our nightly builds and see how that goes, which should hopefully fix this on nightly.

@mati865
Copy link
Contributor

mati865 commented Sep 17, 2018

MSYS2 had the same pthread issues (very long time ago). I've even made workarounds for Rust package (I had no experience with Rust back then): patch, build script. Although those workarounds don't link pthread to the every lib Rust builds

If GCC build with pthread then most of libs built with it will depend on pthread library. I guess that's why there is -nodefaultlibs passed from Rust.
I think building curl on Windows should default to win32 threads but if there is curl installed from MSYS2 then curl-sys will skip building it from source and pick one from MSYS2 (the one with pthread enabled).

Passing -lpthread directly to the linker will definitely let it build but it require users to clear their target/ directory and other caches.

For the reference here is link to the job: https://ci.appveyor.com/project/rust-lang/rust/build/1.0.8939/job/gcsfnc7jqhh2co99?fullLog=true

@nrc
Copy link
Member

nrc commented Sep 18, 2018

@nrc or @rust-lang/dev-tools do y'all have a feeling for how urgent such an issue like this is? I'll work to fix this on nightly (likely 1.31+, maybe 1.30 if we're lucky), but I'm not planning to figure out the 1.29 backport (if any) just yet

I don't have an idea of how many RLS users are on Windows-GNU, so I'm not sure how urgent this is. I suspect this is not worth backporting, especially with all the other edition stuff going on.

@Kroc
Copy link

Kroc commented Sep 18, 2018

When there was the choice between "make this work now" & "download a few gigs and spend an hour installing MSVC", I went with the GNU option -- if it compiles and runs that will do for now.

@Mark-Simulacrum Mark-Simulacrum added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Sep 19, 2018
alexcrichton added a commit to alexcrichton/rust that referenced this issue Sep 20, 2018
This is intended to help solve rust-lang#54206 on nightly where the RLS on MinGW is
having build issues with accidentally building a `curl` library which links to
pthread symbols on Windows (where it should use native mutex locking instead).
The build system for these `*-sys` crates have all been rewritten to be based on
`cc` to bypass native build systems and platform detection to make sure we
configure them correctly.
bors added a commit that referenced this issue Sep 20, 2018
Update some `*-sys` dependencies of Cargo/RLS

This is intended to help solve #54206 on nightly where the RLS on MinGW is
having build issues with accidentally building a `curl` library which links to
pthread symbols on Windows (where it should use native mutex locking instead).
The build system for these `*-sys` crates have all been rewritten to be based on
`cc` to bypass native build systems and platform detection to make sure we
configure them correctly.
@alexcrichton
Copy link
Member

Ok nightly is indeed fixed by #54301 and the merge went much smoother than I anticipated. I will backport that PR to beta. I still think though that as-is it's a bit too risky for a stable backport.

@willem66745
Copy link
Author

willem66745 commented Sep 21, 2018

@alexcrichton:

I tried the latest nightly and indeed rls is available. But now cargo seems broken:

$ cargo +nightly-2018-09-19 check
    Finished dev [unoptimized + debuginfo] target(s) in 0.23s

$ cargo +nightly check
error: failed to initialized index git repository

Caused by:
  an unknown git error occurred

$ rustup toolchain list
nightly-2018-09-19-x86_64-pc-windows-gnu (default)
nightly-x86_64-pc-windows-gnu

This actually renders the entire toolchain not usable at all (as far I can see).

I double checked with other toolchains (msvc and Linux) and both are unaffected.

update: RLS seems to be affected as well:

{"jsonrpc":"2.0","method":"window/progress","params":{"id":"progress_0","title":"Indexing"}}
{"jsonrpc":"2.0","method":"window/showMessage","params":{"message":"Cargo failed: Error compiling dependent crate","type":1}}
{"jsonrpc":"2.0","method":"window/progress","params":{"done":true,"id":"progress_0","title":"Indexing"}}

@mati865
Copy link
Contributor

mati865 commented Sep 21, 2018

I think it's an error related to git2-rs, I was trying to readd gnu toolchain for it but most of the tests were failing. I assumed it was because of my workaround but it can more serious.

Maybe curl isn't fixed after all? Git uses it and I think it's the only thing that changed.

@alexcrichton
Copy link
Member

Thanks for the update @willem66745, that's hopefully fixed by alexcrichton/git2-rs@9c1604e, we'll work on getting that out into the beta release.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Sep 21, 2018
This commit is an attempt to fix an issue [1] in the latest betas for
MinGW where it appears that `libgit2` is no longer functional and always
returns errors. The attempted fix [2] has been published as a new
version of `libgit2-sys`, so this updates the beta branch to using that
commit to get it out and see if it fixes beta.

[1]: rust-lang#54206 (comment)
[2]: alexcrichton/git2-rs@9c1604e
bors added a commit that referenced this issue Sep 21, 2018
[beta] Update the `libgit2-sys` crate

This commit is an attempt to fix an issue [1] in the latest betas for
MinGW where it appears that `libgit2` is no longer functional and always
returns errors. The attempted fix [2] has been published as a new
version of `libgit2-sys`, so this updates the beta branch to using that
commit to get it out and see if it fixes beta.

[1]: #54206 (comment)
[2]: alexcrichton/git2-rs@9c1604e
@alexcrichton
Copy link
Member

Circling back to this, the infra side of this should largely be addressed by #54638, so I'm going to remove the T-infra tag

@nrc would you or the @rust-lang/dev-tools still want to consider a stable backport for this? It should be fixed on beta/nightly already

@alexcrichton alexcrichton removed the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Oct 2, 2018
@XAMPPRocky XAMPPRocky added the C-bug Category: This is a bug. label Oct 2, 2018
bors added a commit that referenced this issue Oct 5, 2018
1.29.2 stable point release

This point release includes a backport of #54639 (a miscompilation) and the fix for #54206 (rls missing on windows-gnu). It also backports a release notes fix (#54150).

The target date for the release is Thursday 11th.

r? @Mark-Simulacrum
cc @rust-lang/core @rust-lang/release
@pietroalbini pietroalbini added stable-accepted Accepted for backporting to the compiler in the stable channel. and removed stable-nominated Nominated for backporting to the compiler in the stable channel. labels Oct 6, 2018
@pietroalbini
Copy link
Member

pietroalbini commented Oct 6, 2018

RLS on windows-gnu will be available again on Rust 1.29.2, which is scheduled to be released next Thursday. You can try the pre-release with:

RUSTUP_DIST_SERVER=https://dev-static.rust-lang.org rustup update stable-gnu
RUSTUP_DIST_SERVER=https://dev-static.rust-lang.org rustup component add rls-preview rust-analysis rust-src

Can you check if the pre-release fixes the bug for you? Thanks!

@kilork
Copy link

kilork commented Oct 9, 2018

Actual command for me was (having gnu toolchain already installed):

RUSTUP_DIST_SERVER=https://dev-static.rust-lang.org rustup update
RUSTUP_DIST_SERVER=https://dev-static.rust-lang.org rustup component add rls-preview rust-analysis rust-src

Because RUSTUP_DIST_SERVER=https://dev-static.rust-lang.org rustup update stable tries first to install msvc for some reason and ignores gnu toolchain.

@mati865
Copy link
Contributor

mati865 commented Oct 9, 2018

@pietroalbini @kilork the actual command for Windows GNU toolchain is:

 RUSTUP_DIST_SERVER=https://dev-static.rust-lang.org rustup update stable-gnu

stable defaults to MSVC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-windows-gnu Toolchain: GNU, Operating system: Windows regression-from-stable-to-stable Performance or correctness regression from one stable version to another. stable-accepted Accepted for backporting to the compiler in the stable channel. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests