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

Update cargo, rls #84412

Merged
merged 1 commit into from
Apr 24, 2021
Merged

Update cargo, rls #84412

merged 1 commit into from
Apr 24, 2021

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Apr 21, 2021

cargo

18 commits in 65d57e6f384c2317f76626eac116f683e2b63665..0ed318d182e465cd66071b91ac3d265af63ef8a1
2021-04-04 15:07:52 +0000 to 2021-04-23 20:54:54 +0000

rls

3 commits in 32c0fe006dcdc13e1ca0ca31de543e4436c1299e..74d1800c25498689c5b5120a1e8495fce0cd0d0d
2021-04-12 11:21:12 +0000 to 2021-04-22 21:29:51 +0000

@ehuss ehuss self-assigned this Apr 21, 2021
@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.

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

ehuss commented Apr 21, 2021

@bors r+

cc @Xanewok

@bors
Copy link
Contributor

bors commented Apr 21, 2021

📌 Commit 662d45b8f57a8b41d6f647c6b484c601049f3b62 has been approved by ehuss

@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 Apr 21, 2021
@rust-log-analyzer

This comment has been minimized.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 21, 2021

@bors r-
investigating rls failure (didn't fail locally for me)

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 21, 2021
@ehuss
Copy link
Contributor Author

ehuss commented Apr 21, 2021

I am having trouble understanding what is wrong. I can't reproduce locally, even in the x86_64-gnu-tools docker image. I have run the CI check twice and failed the same both times.

@Xanewok Any ideas?

test client_did_change_configuration_duplicated_and_unknown_settings ... FAILED
test client_init_duplicated_and_unknown_settings ... FAILED
test client_invalid_member_toml_manifest ... ok
test client_invalid_toml_manifest ... ok
test client_dependency_typo_and_fix ... FAILED
thread panicked while panicking. aborting.
[2021-04-21T21:38:59Z ERROR rls::server] Can't read message
thread '[2021-04-21T21:38:59Z ERROR rls::server] Can't read message
main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 32, kind: BrokenPipe, message: "Broken pipe" }', src/tools/rls/rls/src/server/io.rs:190:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 32, kind: BrokenPipe, message: "Broken pipe" }', src/tools/rls/rls/src/server/io.rs:190:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: test failed, to rerun pass '--test client'

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/client-c22ae4f720a9c514` (signal: 4, SIGILL: illegal instruction)

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 22, 2021

☹️ I'm a bit confused, this appears to clearly be a 15 second timeout failure:

2021-04-22T00:59:58.9156704Z Processing message: Object({"jsonrpc": String("2.0"), "method": String("window/progress"), "params": Object({"done": Null, "id": String("progress_1"), "message": Null, "percentage": Null, "title": String("Building")})})
2021-04-22T01:00:13.9165435Z thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Elapsed(())', src/tools/rls/tests/support/client/mod.rs:265:28

but I set RLS_TEST_WAIT_FOR_AGES which should raise it to 5 minutes. What am I missing?

@ehuss
Copy link
Contributor Author

ehuss commented Apr 22, 2021

Oh, whoops, I was looking at the wrong log.

@Xanewok
Copy link
Member

Xanewok commented Apr 22, 2021

sorry, I was sure I responded using the email client but I can't seem to find my response. The failure seems intermittent, so I'd just try that again. I can raise default timeout to 1 minute if that makes the CI testing easier here (not needing to set an explicit env var), would you like me to do that?

@ehuss
Copy link
Contributor Author

ehuss commented Apr 22, 2021

I raised the timeout to 5 minutes with the RLS_TEST_WAIT_FOR_AGES environment variable, and the tests seem to pass now. I'm not sure why client_dependency_typo_and_fix in particular seems to time out in 15 seconds. None of the cargo changes seem like they would affect performance. Does it seem reasonable to set that?

Since this modifies a build script, maybe @Mark-Simulacrum can review?

r? @Mark-Simulacrum

@Xanewok
Copy link
Member

Xanewok commented Apr 22, 2021

It does access the network and attempts to compile autocfg crate, so maybe it adds up and it goes over the threshold. I think it might be reasonable to bump the test timeout in this case, but I'm not sure what might be causing the slowdown here. Maybe it was a hair from crossing the threshold before and now some minor change makes it time out?

bors added a commit to rust-lang/rls that referenced this pull request Apr 22, 2021
Bump default integration test message timeout to 30s

In case we don't want to introduce another env var to rustc CI script

cc rust-lang/rust#84412

r? `@ehuss`
@rust-log-analyzer

This comment has been minimized.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 22, 2021

☹️ I'm concerned why these tests are suddenly taking much longer to run. I'll try to investigate some more.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 23, 2021

I discovered the problem, it is a bug on Cargo's side. rust-lang/cargo#9384 is causing the entire crates.io cache to be rebuilt. @Xanewok sorry about misleading you.

@Xanewok
Copy link
Member

Xanewok commented Apr 23, 2021

Nice sleuthing work! Is there something more you need from RLS side then?

@ehuss
Copy link
Contributor Author

ehuss commented Apr 23, 2021

Nice sleuthing work! Is there something more you need from RLS side then?

Nope, Alex has already posted a fix. I'll pick up and test the changes when they land.

Thanks Mark! Yea, I've been pushing various changes to try to fix things, but it looks like it was just a bug in cargo. I should be able to take it from here.

@ehuss ehuss assigned ehuss and unassigned Mark-Simulacrum Apr 23, 2021
@ehuss
Copy link
Contributor Author

ehuss commented Apr 23, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Apr 23, 2021

📌 Commit 893ca66 has been approved by ehuss

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 23, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 24, 2021
Update cargo, rls

## cargo

18 commits in 65d57e6f384c2317f76626eac116f683e2b63665..0ed318d182e465cd66071b91ac3d265af63ef8a1
2021-04-04 15:07:52 +0000 to 2021-04-23 20:54:54 +0000
- Restore crates.io's `SourceId` hash value to before (rust-lang/cargo#9397)
- Fix loading `branch=master` patches in the v3 lock transition (rust-lang/cargo#9392)
- Update changelog for 1.52 beta changes. (rust-lang/cargo#9396)
- Fix build-std updating the index on every build. (rust-lang/cargo#9393)
- Fix typo in profile docs (rust-lang/cargo#9386)
- Fix disagreement about lockfile ordering on stable/nightly (rust-lang/cargo#9384)
- Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name (rust-lang/cargo#9365)
- Fix rust-lang/cargo#9350 (cargo build -Z help is missing options) (rust-lang/cargo#9369)
- an struct -> a struct (rust-lang/cargo#9379)
- Handle man pages better on Windows. (rust-lang/cargo#9378)
- fix: better error message when dependency/workspace member missing (rust-lang/cargo#9368)
- Fix typo in book (rust-lang/cargo#9376)
- Don't re-use rustc cache when RUSTC_WRAPPER changes (rust-lang/cargo#9348)
- doc: add split-debuginfo doc in config chapter (rust-lang/cargo#9372)
- refactor: remove `CargoResultExt` (rust-lang/cargo#9367)
- Track "CARGO" in environment fingerprint. (rust-lang/cargo#9363)
- Update clippy lint allow set. (rust-lang/cargo#9356)
- Fix 'suport' typo in documentation (rust-lang/cargo#9338)

## rls

3 commits in 32c0fe006dcdc13e1ca0ca31de543e4436c1299e..74d1800c25498689c5b5120a1e8495fce0cd0d0d
2021-04-12 11:21:12 +0000 to 2021-04-22 21:29:51 +0000
- Bump default integration test message timeout to 30s (rust-lang/rls#1731)
- itertools = 0.9, fst = 0.4 (rust-lang/rls#1729)
- Update cargo (rust-lang/rls#1728)
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 24, 2021
Update cargo, rls

## cargo

18 commits in 65d57e6f384c2317f76626eac116f683e2b63665..0ed318d182e465cd66071b91ac3d265af63ef8a1
2021-04-04 15:07:52 +0000 to 2021-04-23 20:54:54 +0000
- Restore crates.io's `SourceId` hash value to before (rust-lang/cargo#9397)
- Fix loading `branch=master` patches in the v3 lock transition (rust-lang/cargo#9392)
- Update changelog for 1.52 beta changes. (rust-lang/cargo#9396)
- Fix build-std updating the index on every build. (rust-lang/cargo#9393)
- Fix typo in profile docs (rust-lang/cargo#9386)
- Fix disagreement about lockfile ordering on stable/nightly (rust-lang/cargo#9384)
- Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name (rust-lang/cargo#9365)
- Fix rust-lang/cargo#9350 (cargo build -Z help is missing options) (rust-lang/cargo#9369)
- an struct -> a struct (rust-lang/cargo#9379)
- Handle man pages better on Windows. (rust-lang/cargo#9378)
- fix: better error message when dependency/workspace member missing (rust-lang/cargo#9368)
- Fix typo in book (rust-lang/cargo#9376)
- Don't re-use rustc cache when RUSTC_WRAPPER changes (rust-lang/cargo#9348)
- doc: add split-debuginfo doc in config chapter (rust-lang/cargo#9372)
- refactor: remove `CargoResultExt` (rust-lang/cargo#9367)
- Track "CARGO" in environment fingerprint. (rust-lang/cargo#9363)
- Update clippy lint allow set. (rust-lang/cargo#9356)
- Fix 'suport' typo in documentation (rust-lang/cargo#9338)

## rls

3 commits in 32c0fe006dcdc13e1ca0ca31de543e4436c1299e..74d1800c25498689c5b5120a1e8495fce0cd0d0d
2021-04-12 11:21:12 +0000 to 2021-04-22 21:29:51 +0000
- Bump default integration test message timeout to 30s (rust-lang/rls#1731)
- itertools = 0.9, fst = 0.4 (rust-lang/rls#1729)
- Update cargo (rust-lang/rls#1728)
@JohnTitor
Copy link
Member

Failed in rollup: #84512 (comment)
@bors r- rollup=iffy

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 24, 2021
@JohnTitor
Copy link
Member

Seems like #84310 causes the failure, do we usually wait when a recent PR likely breaks the build? If not, feel free to r+ again.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 24, 2021

There are some important changes here (and it is quite a large PR). If #84310 can land separately in the next 4 days, it should pass. In 4 days, the beta no-tools-breakage starts and it won't be allowed to break rls. @RalfJung FYI

@bors r+ p=1

@bors
Copy link
Contributor

bors commented Apr 24, 2021

📌 Commit 893ca66 has been approved by ehuss

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2021
@bors
Copy link
Contributor

bors commented Apr 24, 2021

⌛ Testing commit 893ca66 with merge 2b68027...

@RalfJung
Copy link
Member

RalfJung commented Apr 24, 2021

I am not sure what this has to do with #84310? Do cargo/rls use the const_fn feature?
#84310 is now marked to not be rolled up.

@JohnTitor
Copy link
Member

@RalfJung RLS uses an auto-published rustc-* crates and sometimes a PR makes its build broken (in this case, #84310 is so, see #84512 (comment)).

@bors
Copy link
Contributor

bors commented Apr 24, 2021

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 2b68027 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 24, 2021
@bors bors merged commit 2b68027 into rust-lang:master Apr 24, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo build -Z help is missing options
9 participants