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 to nightly rustc to 2023-04-19 #31381

Merged
merged 12 commits into from
May 11, 2023

Conversation

ryoqun
Copy link
Member

@ryoqun ryoqun commented Apr 28, 2023

Problem

I'm impatient. ;)

and our nightly is too old

Summary of changes

Update nightly.

@ryoqun ryoqun requested a review from yihau April 28, 2023 01:32
@yihau
Copy link
Member

yihau commented Apr 28, 2023

just updated docker images and retried the pipeline! let's see how it goes 👻

@yihau
Copy link
Member

yihau commented Apr 28, 2023

good and bad news. failed but is lint stuff 🤞

Screenshot 2023-04-28 at 12 28 37 PM

@ryoqun ryoqun marked this pull request as draft April 28, 2023 14:02
@ryoqun ryoqun changed the title Update to nightly rustc to 2023-04-19 wip: Update to nightly rustc to 2023-04-19 Apr 28, 2023
@codecov
Copy link

codecov bot commented Apr 30, 2023

Codecov Report

Merging #31381 (f50e37d) into master (f985f73) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           master   #31381     +/-   ##
=========================================
  Coverage    81.2%    81.2%             
=========================================
  Files         733      733             
  Lines      204948   206460   +1512     
=========================================
+ Hits       166584   167850   +1266     
- Misses      38364    38610    +246     

@ryoqun ryoqun force-pushed the nightly-rust-2023-04-19 branch from 54440a2 to 28a1487 Compare May 2, 2023 04:16
scripts/coverage.sh Outdated Show resolved Hide resolved
@ryoqun ryoqun force-pushed the nightly-rust-2023-04-19 branch 2 times, most recently from 8b06485 to 6a1044a Compare May 4, 2023 05:05
gossip/src/cluster_info.rs Outdated Show resolved Hide resolved
core/src/serve_repair.rs Outdated Show resolved Hide resolved
@ryoqun ryoqun force-pushed the nightly-rust-2023-04-19 branch from 6a1044a to 54b9ed1 Compare May 4, 2023 12:55
@@ -29,7 +29,7 @@ fi
if [[ -n $RUST_NIGHTLY_VERSION ]]; then
nightly_version="$RUST_NIGHTLY_VERSION"
else
nightly_version=2023-01-22
nightly_version=2023-04-19
Copy link
Contributor

Choose a reason for hiding this comment

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

Now's our chance!

Suggested change
nightly_version=2023-04-19
nightly_version=2023-04-20

Copy link
Member Author

Choose a reason for hiding this comment

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

hm? why bumping the nightly version is needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

For maximum memes :)

(but no, not needed)

@ryoqun ryoqun force-pushed the nightly-rust-2023-04-19 branch from 4106add to 0add968 Compare May 5, 2023 12:34
@@ -994,58 +995,94 @@ mod tests {
}

// the old bpf_loader in-program deserializer bpf_loader::id()
#[allow(clippy::type_complexity)]
#[deny(unsafe_op_in_unsafe_fn)]
Copy link
Member Author

Choose a reason for hiding this comment

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

personally, i want this to be enabled at tree-wide. but this is different story.

Comment on lines +1019 to +1026
// rustc inserts debug_assert! for misaligned pointer dereferences when
// deserializing, starting from [1]. so, use std::mem::transmute as the last resort
// while preventing clippy from complaining to suggest not to use it.
// [1]: https://github.com/rust-lang/rust/commit/22a7a19f9333bc1fcba97ce444a3515cb5fb33e6
// as for the ub nature of the misaligned pointer dereference, this is
// acceptable in this code, given that this is cfg(test) and it's cared only with
// x86-64 and the target only incurs some performance penalty, not like segfaults
// in other targets.
Copy link
Member Author

Choose a reason for hiding this comment

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

@dmakarov this is fyi. I think you'll need to update the original code at sdk/program/entrypoint{,_deprecated}.rs when updating platform-tool to be based on rust toolchain 1.70+.

I already chatted with @Lichtso and @alessandrod. hope this comment does make sense for the uninformed. :)

@ryoqun ryoqun changed the title wip: Update to nightly rustc to 2023-04-19 Update to nightly rustc to 2023-04-19 May 6, 2023
@ryoqun ryoqun requested review from alessandrod and Lichtso May 6, 2023 12:50
@ryoqun ryoqun marked this pull request as ready for review May 6, 2023 12:50
@ryoqun ryoqun force-pushed the nightly-rust-2023-04-19 branch from e3f9571 to d0080e2 Compare May 6, 2023 12:51
@ryoqun ryoqun requested a review from brooksprumo May 6, 2023 12:51
@@ -75,7 +75,7 @@ fi

_ ci/order-crates-for-publishing.py

nightly_clippy_allows=()
nightly_clippy_allows=(--allow=clippy::redundant_clone --allow clippy::match-result-ok)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these not be fixed first? Are there a lot? Can cargo clippy --fix resolve them automatically?

Copy link
Member Author

Choose a reason for hiding this comment

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

to be honest, i haven't looked into it at all and i'm just too tired at this moment. ;)

(rant time: this rust bump was simply cursed: sscache upstream debug and fix (thanks for the hard work by @yihau), grcov racing-condition bug (which turned out false-positive), the new misaligned debug_assert (see below diff), odd sigill in coverage update (ref: #31099); i just wanted Arc::into_inner(), which was merged a day after the nightly before #31487...)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah bummer, sorry about all that :(

Looks like this may be a clippy bug: rust-lang/rust-clippy#10577. One reporter indicated that the issue appeared between 3/16 and 3/18; I tried 3/17 and it has the bug. I tired 3/16 and the compiler crashed (maybe some other bug). I tried 3/15, and it has a clippy bug too (same one)? 3/13 seems to also have the bug...

That's as far as I've made it so far. In the meantime, I've put up #31525 and #31526 to fixup some clippy lints.

Copy link
Member Author

Choose a reason for hiding this comment

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

the checks step just passed with ef8bb88. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

@Lichtso could you review changes in this file? i think it's the only review-worth diff in this pr to merge. :)

@ryoqun ryoqun force-pushed the nightly-rust-2023-04-19 branch from e62229a to ef8bb88 Compare May 9, 2023 05:06
@ryoqun ryoqun force-pushed the nightly-rust-2023-04-19 branch from ef8bb88 to f50e37d Compare May 11, 2023 02:50
@ryoqun ryoqun merged commit 4d4dddc into solana-labs:master May 11, 2023
@@ -75,7 +75,7 @@ fi

_ ci/order-crates-for-publishing.py

nightly_clippy_allows=()
nightly_clippy_allows=(--allow=clippy::redundant_clone)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add this @ryoqun ?

Copy link
Member Author

@ryoqun ryoqun May 17, 2023

Choose a reason for hiding this comment

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

@CriesofCarrots in short, I was exhausted for bumping nightly this time, which took months. (details: #31381 (comment))

just saw #31690. sorry for trouble because of inter-branch clippy inconsistency. to be honest, i didn't anticipated
this.

after initial triage from @brooksprumo, i sensed fixing this isn't straigtforward: #31381 (comment)

so, i opted to merge this pr in as-is. (this urgency is coming from the fact my upcoming pr will be blocked on rustc 1.70+)

seems @apfitzge is starting to tackle on this: #31685.

initially, i thought this exception isn't end of world thing, but seems to cause actual annoyance. I'll work on this.

Why did you add this @ryoqun ?

so, put differently, crude way to solicit collaboration. ;)

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.

5 participants