Bump MSRV from 1.63.0 to 1.74.0 for all crates in the repo#4926
Bump MSRV from 1.63.0 to 1.74.0 for all crates in the repo#4926apoelstra merged 5 commits intorust-bitcoin:masterfrom
Conversation
If I update it in internals, our uses of `rust_version!` with values below 1.74 break. In particular the cond_const! macro breaks. So I will do that in the next commit, so the diff for this one is simple. We may want to put some work into getting a single source of truth here... I had to change this constant in 29 files, plus internals/Cargo.toml.
|
Ran my local CI on the last commit -- the first one (update MSRV but not for So 0d1f410 should be good to go. |
| writeln!(macro_file, "/// ```")?; | ||
| writeln!(macro_file, "/// bitcoin_internals::rust_version! {{")?; | ||
| writeln!(macro_file, "/// if >= 1.70 {{")?; | ||
| writeln!(macro_file, "/// if >= 1.78 {{")?; |
There was a problem hiding this comment.
I don't understand the reason for this change? Also the lines below need updating as well.
In 84a9df1
There was a problem hiding this comment.
Updating in what way?
And the reason is that the rust_version! macro doesn't compile if you give it rust versions before our MSRV. It has an "unknown Rust version" panic case.
There was a problem hiding this comment.
Woops context is dropped. There are output messages that have the 1.70 in them. Why 1.78 and not 1.74?
There was a problem hiding this comment.
Because 1.74 is the MSRV. Anything "conditionally compiled on 1.74" will be unconditionally compiled.
| assert_eq!(max.to_sat_per_kwu_ceil(), u64::MAX / 4_000); | ||
| assert_eq!(max.to_sat_per_vb_ceil(), u64::MAX / 1_000_000); | ||
| assert_eq!(max.to_sat_per_kvb_ceil(), u64::MAX / 1_000); | ||
| assert_eq!(max.to_sat_per_kwu_ceil(), u64::MAX / 4_000 + 1); | ||
| assert_eq!(max.to_sat_per_vb_ceil(), u64::MAX / 1_000_000 + 1); | ||
| assert_eq!(max.to_sat_per_kvb_ceil(), u64::MAX / 1_000 + 1); |
There was a problem hiding this comment.
So our ceil functions weren't actually ceil?
In 3bdc48f - props for remembering these.
|
The last patch is pretty curly, not sure how much weight my ack has. |
|
Oh the |
...and delete all conditional compilation based on earlier MSRVs.
There is one `is_some_and` usage, but everything else is about using the new `div_ceil` method. I didn't look closely at any of these but there were a lot of them and I'll bet at least one was actually a panic bug that's fixed now.
In rust-bitcoin#4838 we used saturating addition to simulate `div_ceil`, which gives incorrect results on extreme values but at least doesn't panic. Similar in the followup PR rust-bitcoin#4847. (These aren't detected by clippy, but I remember them.)
I grepped the codebase for `as \*` and found a few places where we were doing explicit pointer casts but we could've been using std methods. The methods are safer because they don't allow changing mutability (unless we use cast_const or cast_mut, which don't allow changing the type). There are a few instances remaining where we cast to pointers to unsized types. These are actually fat raw pointers and they don't have std methods. Now they stand out because of the use of `as *`. This commit might require a bit of thought to review, and it turns out it has nothing to do with the MSRV bump (which gave us cast_const and cast_mut, but actually we don't use those, thankfully..). As a bit of trivia, though, the new cast_mut and cast_const methods were originally proposed and implemented by our own Kixunil: rust-lang/rust#92657
0d1f410 to
9b7e920
Compare
|
Oh that's just in code that generates docs - face palm. |
|
Normally would let something like this sit for a bit, but it's blocking push-decoding and I don't see any path forward where we don't bump at least to 1.65 for that reason. If people complain we can reduce the MSRV from 1.74 to 1.65 and I wouldn't be heartbroken. (But as I've said elsewhere, I actually hope we can increase it to 1.76.) |
Bump MSRV from 1.63 to 1.74 for all crates in this repo.
Closes #3339.
We should open a new tracking issue.