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.

Sign up for GitHub

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

Jump to bottom

Update to rust 1.80.0 #32896

Merged
merged 10 commits into from
Aug 7, 2024
Merged

Update to rust 1.80.0 #32896

merged 10 commits into from
Aug 7, 2024

Conversation

Hmikihiro
Copy link
Contributor

I would create a new pull request for commits failed in PR #32889 .

rustc library had some changes in 1.79 & 1.80.

LazyLock is part of standard library in 1.80
Rust1.80 can replace from lazy_static & once_cell::sync::Lazy to std::LazyLock.
I would suggest updating to rust1.80 as the first step, with replace to LazyLock later.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes (test-unit crown)

Hmikihiro added 9 commits July 31, 2024 03:40
@Hmikihiro
Update for nix
57cc462 
Signed-off-by: Hayashi Mikihiro <[email protected]>
@Hmikihiro
Update to Rust 1.80.0
bcdb362 
Signed-off-by: Hayashi Mikihiro <[email protected]>
@Hmikihiro
Rename to BindingMode from BindingAnnotation
d7496f9 
rust-lang/rust#124047
Signed-off-by: Hayashi Mikihiro <[email protected]>
@Hmikihiro
Remove TypeVariableOriginKind
94caabc 
rust-lang/rust#123016
Signed-off-by: Hayashi Mikihiro <[email protected]>
@Hmikihiro
Remove TypeVariableOrigin
2229216 
rust-lang/rust#124955
Signed-off-by: Hayashi Mikihiro <[email protected]>
@Hmikihiro
Remove LintDiagnostic::msg
32c3f90 
rust-lang/rust#125410

Signed-off-by: Hayashi Mikihiro <[email protected]>
@Hmikihiro
common.rs fmt mistake indents
22c58ed 
Signed-off-by: Hayashi Mikihiro <[email protected]>
@Hmikihiro
trace_in_no_trace.rs remove mistake space
1ad263f 
Signed-off-by: Hayashi Mikihiro <[email protected]>
@Hmikihiro
trace_in_no_trace.rs remove mistake head space
0d5eafc 
Signed-off-by: Hayashi Mikihiro <[email protected]>
@sagudev sagudev enabled auto-merge July 30, 2024 20:05
@sagudev sagudev added this pull request to the merge queue Jul 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 30, 2024
@sagudev
Copy link
Member

sagudev commented Jul 31, 2024

Do you have windows machine/will to investigate windows problem, or would you like for someone else to take over?

@theoparis theoparis mentioned this pull request Aug 1, 2024
Closed
@Hmikihiro
Copy link
Contributor Author

Do you have windows machine/will to investigate windows problem, or would you like for someone else to take over?

I don't have a Windows machine. I only investigate Github Actions. I can only reproduce in resource_thread::test_exit release build on ver1.79&1.80. I would take me a lot of time to fix, as it is too difficult to reproduce in debug build. I would like to request a takeover.

https://github.com/Hmikihiro/servo/actions/runs/10181674109

@sagudev
Copy link
Member

sagudev commented Aug 1, 2024

as it is too difficult to reproduce in debug build

Are you saying that debug builds works, but release don't?

I can only reproduce in resource_thread::test_exit release build on ver1.79

So 1.79 is bad?

@sagudev sagudev self-assigned this Aug 1, 2024
@Hmikihiro
Copy link
Contributor Author

Hmikihiro commented Aug 1, 2024 •
edited
Loading

Are you saying that debug builds works, but release don't?

Yes, I do. In debug build worked also warning are not occur.

So 1.79 is bad?

Yes, in my view, 1.79 is bad around resource_thread::test_exit.

@sagudev
Copy link
Member

sagudev commented Aug 1, 2024 •
edited
Loading

nightly-2024-04-05 is good
nightly-2024-04-07 is good
nightly-2024-04-08 is bad
nightly-2024-04-09 is bad
nightly-2024-04-10 is bad
nightly-2024-04-14 is bad

I suspect rust-lang/rust#122268 also related: rust-lang/rust#116900

I did multiple runs to make get confidence in results (as this is flaky error).

@sagudev
Copy link
Member

sagudev commented Aug 2, 2024

I used cargo-bisect-rustc and here is the report https://github.com/sagudev/servo/actions/runs/10205095025/job/28235104802#step:14:92168

Apparently caused by rust-lang/rust@6f83750, I believe it's caused by rust-lang/rust#119224

@sagudev

This comment was marked as resolved.

Sign in to view
@sagudev
Copy link
Member

sagudev commented Aug 2, 2024 •
edited
Loading

I think this happens because we panic in drop of OsIpcReceiverSet, (it's actually panic while panic), but this is in separate thread. While before this was visible rarely (we have retry 3 on unit-tests), but now rust-lang/rust#119224 causes main test thread to wait for panic_hook to not be used (so it can be freed), so this panic is visible way more often (as it actually finishes in time).

@sagudev sagudev mentioned this pull request Aug 2, 2024
Open
@sagudev sagudev changed the title Update rust 1.80.0 (reopen) Update to rust 1.80.0 Aug 2, 2024
@sagudev sagudev added the T-windows Do a try run on Windows label Aug 2, 2024
@github-actions github-actions bot removed the T-windows Do a try run on Windows label Aug 2, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2024

🔨 Triggering try run (#10217246405) for Windows

@sagudev
Workaround for #32912
b4d76e7 
It's dirty but it worked on 2000 runs

Signed-off-by: sagudev <[email protected]>
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 2, 2024

✨ Try run (#10217246405) succeeded.

@sagudev sagudev requested a review from mrobinson August 2, 2024 15:43
@nicoburns
Copy link
Contributor

I think this happens because we panic in drop of OsIpcReceiverSet, (it's actually panic while panic), but this is in separate thread. While before this was visible rarely (we have retry 3 on unit-tests), but now rust-lang/rust#119224 causes main test thread to wait for panic_hook to not be used (so it can be freed), so this panic is visible way more often (as it actually finishes in time).

I believe panic in panic being a hard unrecoverable abort is intentional in newer rustc versions. So I suspect we'll need to change the code to not do that.

@sagudev
Copy link
Member

sagudev commented Aug 5, 2024

I think this happens because we panic in drop of OsIpcReceiverSet, (it's actually panic while panic), but this is in separate thread. While before this was visible rarely (we have retry 3 on unit-tests), but now rust-lang/rust#119224 causes main test thread to wait for panic_hook to not be used (so it can be freed), so this panic is visible way more often (as it actually finishes in time).

I believe panic in panic being a hard unrecoverable abort is intentional in newer rustc versions. So I suspect we'll need to change the code to not do that.

The problem is that we panic in the first place (both panics actually happens in same place if I am reading traces correctly). Anyway given that we have workaround for #32912 (which is just sleeping to wait for transmission to finish, as the code that checks for transmission finish is panicking), I think we do not need to block landing this PR on #32912.

@sagudev sagudev mentioned this pull request Aug 5, 2024
Closed
@sagudev sagudev added this pull request to the merge queue Aug 7, 2024
Merged via the queue into servo:main with commit 3c271fb Aug 7, 2024
11 checks passed
Gae24 pushed a commit to Gae24/servo that referenced this pull request Aug 7, 2024
@Hmikihiro @sagudev @Gae24
Update to rust 1.80.0 (servo#32896)
6840920 
* Update for nix

Signed-off-by: Hayashi Mikihiro <[email protected]>

* Update to Rust 1.80.0

Signed-off-by: Hayashi Mikihiro <[email protected]>

* Rename to BindingMode from BindingAnnotation

rust-lang/rust#124047
Signed-off-by: Hayashi Mikihiro <[email protected]>

* Remove TypeVariableOriginKind

rust-lang/rust#123016
Signed-off-by: Hayashi Mikihiro <[email protected]>

* Remove TypeVariableOrigin

rust-lang/rust#124955
Signed-off-by: Hayashi Mikihiro <[email protected]>

* Remove LintDiagnostic::msg

rust-lang/rust#125410

Signed-off-by: Hayashi Mikihiro <[email protected]>

* common.rs fmt mistake indents

Signed-off-by: Hayashi Mikihiro <[email protected]>

* trace_in_no_trace.rs remove mistake space

Signed-off-by: Hayashi Mikihiro <[email protected]>

* trace_in_no_trace.rs remove mistake head space

Signed-off-by: Hayashi Mikihiro <[email protected]>

* Workaround for servo#32912

It's dirty but it worked on 2000 runs

Signed-off-by: sagudev <[email protected]>

---------

Signed-off-by: Hayashi Mikihiro <[email protected]>
Signed-off-by: sagudev <[email protected]>
Co-authored-by: Samson <[email protected]>
@Hmikihiro Hmikihiro deleted the update-rust-1.80.0-fmt branch August 7, 2024 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrobinson mrobinson mrobinson approved these changes

@sagudev sagudev sagudev approved these changes

Assignees

@sagudev sagudev

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Hmikihiro @sagudev @nicoburns @mrobinson