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

[meta] update rust to 1.84 #7334

Merged
merged 8 commits into from
Jan 15, 2025
Merged

Conversation

sunshowers
Copy link
Contributor

@sunshowers sunshowers commented Jan 10, 2025

We're moving three versions ahead, so this is something we should do with some caution. However there's at least one important illumos-specific fix that's in 1.84 (rust-lang/rust#132984) so we should definitely move over.

I've verified that rust-lang/rust#132064 no longer affects us. To be more precise, its impact has been mostly taken care of -- there's still a small regression, see this comment, but the Rust team has said it's one that should get better in the next few months.

Depends on #7333.

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
@sunshowers sunshowers marked this pull request as draft January 10, 2025 23:13
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
@sunshowers sunshowers marked this pull request as ready for review January 11, 2025 04:58
@sunshowers sunshowers requested a review from iliana January 11, 2025 04:58
Copy link
Contributor

@iliana iliana left a comment

Choose a reason for hiding this comment

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

I'm wondering if we should mark the unused lifetime lint that seems to be most of this diff as an allowed lint. It seems like it really ought to be categorized as a style lint instead IMO, and we disable style lints by default:

omicron/Cargo.toml

Lines 277 to 279 in d9a6a8b

[workspace.lints.clippy]
# Clippy's style nits are useful, but not worth keeping in CI.
style = { level = "allow", priority = -1 }

(I have been meaning to go back through the list of style lints we presently have disabled and update our list of warned style lints.)

// https://github.com/rust-lang/rust-clippy/issues/13923 (ideally we'd have
// used `expect` but on 1.84, it says that it's unfulfilled even though it
// is fulfilled?)
#[allow(clippy::needless_lifetimes)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[allow(clippy::needless_lifetimes)]
#[expect(clippy::needless_lifetimes)]

(1.81 release notes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see the comment :( it didn't work

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh that seems bad. I wonder if we can minimize it and file an issue.

Copy link
Contributor Author

@sunshowers sunshowers Jan 14, 2025

Choose a reason for hiding this comment

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

It's a little tricky because the needless_lifetimes thing has been fixed in clippy main -- would need to find some other case where expect doesn't work.

@sunshowers
Copy link
Contributor Author

I'm wondering if we should mark the unused lifetime lint that seems to be most of this diff as an allowed lint.

Ah -- so needless_lifetimes is actually a very old lint, but over time it's been extended to catch more cases. I think it's a valuable lint to have.

@sunshowers sunshowers changed the base branch from sunshowers/spr/main.meta-update-rust-to-184 to main January 14, 2025 20:57
Created using spr 1.3.6-beta.1
@sunshowers sunshowers enabled auto-merge (squash) January 14, 2025 20:58
Created using spr 1.3.6-beta.1
@sunshowers sunshowers merged commit 414318d into main Jan 15, 2025
16 checks passed
@sunshowers sunshowers deleted the sunshowers/spr/meta-update-rust-to-184 branch January 15, 2025 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants