rustls / webpki

WebPKI X.509 Certificate Validation in Rust
https://docs.rs/rustls-webpki/latest/webpki/
Other
94 stars 50 forks source link

Update semver-compatible dependencies #241

Closed djc closed 6 months ago

djc commented 6 months ago

To replace #240.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.19%. Comparing base (439331b) to head (a2423a0).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #241 +/- ## ======================================= Coverage 97.19% 97.19% ======================================= Files 19 19 Lines 4065 4065 ======================================= Hits 3951 3951 Misses 114 114 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

djc commented 6 months ago

So this is failing our MSRV checks because aws-lc-sys pulls in bindgen (as an optional build-dependency) and bindgen pulls in regex and regex now has an MSRV of 1.65. It's a bit tricky because AIUI the bindgen dependency is optional for aws-lc-sys on platforms for which they have pregenerated bindings, which seems to be all the tier 1 ones at this point?

We could avoid updating regex in our Cargo.lock to prove to ourselves that the library otherwise still works on 1.60 but downstream users (using the aws-lc-rs provider) will probably still think we no longer support 1.60?

ctz commented 6 months ago

I wonder if we should take 1.65? It'd mean we can use let-else, for example. The downside of that is debian stable is on 1.63.

djc commented 6 months ago

It also feels silly because it looks like regex is not really a core dependency for bindgen, it's used to provide what looks like potentially optional API (which it seems aws-lc-rs doesn't actually use)...

cpu commented 6 months ago

The downside of that is debian stable is on 1.63.

I wonder how long we'd have to wait for Debian to get 1.65 :thinking:

djc commented 6 months ago

The downside of that is debian stable is on 1.63.

I wonder how long we'd have to wait for Debian to get 1.65 :thinking:

I think for the last bump they went from 1.48 to 1.63. I don't think we'll want to wait that long...

cpu commented 6 months ago

cpu force-pushed the update-deps branch from 492824e to ec2bf76

I force pushed an update to pull in aws-lc-rs 1.6.4 since the 1.6.3 release with the bindgen issue was yanked.

error: package rustix v0.38.32 cannot be built because it requires rustc 1.63 or newer, while the currently active rustc version is 1.61.0

~It looks like we might still need to consider an MSRV bump, but to a much more reasonable 1.63. That's in Debian stable, and we were talking about taking it already over in https://github.com/rustls/rustls/pull/1885 for hashbrown. I think we should do it. Agree?~ This was confusion from forgetting to update the locked aws-lc-sys dep too.

ctz commented 6 months ago

I think we should do it. Agree?

Yep!

cpu commented 6 months ago

Ah, should have tried 1.63 as MSRV first:

error: package regex-automata v0.4.6 cannot be built because it requires rustc 1.65 or newer, while the currently active rustc version is 1.63.0

~It looks like a bindgen build dep is still bringing in regex 1.10.4. I see the MSRV build fail w/ 1.63.~ This was confusion from forgetting to update the locked aws-lc-sys dep too.

cpu commented 6 months ago

I'm confused why https://github.com/rustls/rustls/pull/1888 builds green with an MSRV of 1.61 given the above. I'm missing some detail :thinking:

cpu commented 6 months ago

I'm missing some detail 🤔

It looks like Rustls is using aws-lc-sys 0.14.1 while webpki is using 0.14.0

rustls dep tree ``` rustls v0.23.4 (/home/daniel/Code/Rust/rustls/rustls) ├── aws-lc-rs v1.6.4 │ ├── aws-lc-sys v0.14.1 │ │ ├── libc v0.2.153 │ │ └── paste v1.0.14 (proc-macro) │ │ [build-dependencies] │ │ ├── cmake v0.1.50 │ │ │ └── cc v1.0.86 │ │ ├── dunce v1.0.4 │ │ └── fs_extra v1.3.0 │ ├── mirai-annotations v1.12.0 │ ├── paste v1.0.14 (proc-macro) │ └── zeroize v1.7.0 ```

vs

rustls-webpki dep tree ``` rustls-webpki v0.102.2 (/home/daniel/Code/Rust/webpki) ├── aws-lc-rs v1.6.4 │ ├── aws-lc-sys v0.14.0 │ │ ├── libc v0.2.153 │ │ └── paste v1.0.14 (proc-macro) │ │ [build-dependencies] │ │ ├── bindgen v0.69.4 │ │ │ ├── bitflags v2.5.0 │ │ │ ├── cexpr v0.6.0 │ │ │ │ └── nom v7.1.3 │ │ │ │ ├── memchr v2.7.2 │ │ │ │ └── minimal-lexical v0.2.1 │ │ │ ├── clang-sys v1.7.0 │ │ │ │ ├── glob v0.3.1 │ │ │ │ ├── libc v0.2.153 │ │ │ │ └── libloading v0.8.3 │ │ │ │ └── cfg-if v1.0.0 │ │ │ │ [build-dependencies] │ │ │ │ └── glob v0.3.1 │ │ │ ├── itertools v0.12.1 │ │ │ │ └── either v1.10.0 │ │ │ ├── lazy_static v1.4.0 │ │ │ ├── lazycell v1.3.0 │ │ │ ├── log v0.4.21 │ │ │ ├── prettyplease v0.2.17 │ │ │ │ ├── proc-macro2 v1.0.79 │ │ │ │ │ └── unicode-ident v1.0.12 │ │ │ │ └── syn v2.0.57 │ │ │ │ ├── proc-macro2 v1.0.79 (*) │ │ │ │ ├── quote v1.0.35 │ │ │ │ │ └── proc-macro2 v1.0.79 (*) │ │ │ │ └── unicode-ident v1.0.12 │ │ │ ├── proc-macro2 v1.0.79 (*) │ │ │ ├── quote v1.0.35 (*) │ │ │ ├── regex v1.10.4 │ │ │ │ ├── regex-automata v0.4.6 │ │ │ │ │ └── regex-syntax v0.8.3 │ │ │ │ └── regex-syntax v0.8.3 │ │ │ ├── rustc-hash v1.1.0 │ │ │ ├── shlex v1.3.0 │ │ │ ├── syn v2.0.57 (*) │ │ │ └── which v4.4.2 │ │ │ ├── either v1.10.0 │ │ │ ├── home v0.5.9 │ │ │ └── rustix v0.38.32 │ │ │ ├── bitflags v2.5.0 │ │ │ └── linux-raw-sys v0.4.13 │ │ ├── cmake v0.1.50 │ │ │ └── cc v1.0.90 │ │ ├── dunce v1.0.4 │ │ └── fs_extra v1.3.0 │ ├── mirai-annotations v1.12.0 │ ├── paste v1.0.14 (proc-macro) │ └── zeroize v1.7.0 ```
djc commented 6 months ago

Maybe because aws-lc-rs has fixed their issue in 1.6.3 in 1.6.4?

ctz commented 6 months ago

I can repro that locally, but then if I do:

$ cargo update
    Updating crates.io index
    Updating aws-lc-sys v0.14.0 -> v0.14.1
    Updating syn v2.0.57 -> v2.0.58

That fixes it (because aws-lc-rs -> aws-lc-sys -> build-dependencies drops bindgen).

cpu commented 6 months ago

Maybe because aws-lc-rs has fixed their issue in 1.6.3 in 1.6.4?

I'm using aws-lc-rs 1.6.4 in both, but aws-lc-sys didn't get bumped in the lockfile here where it did in the main repo.

cpu commented 6 months ago

Ok, all fixed up. No MSRV change required.

Sorry for the confusion!

cpu commented 6 months ago

@ctz @djc Can one of you re-review since I pushed changes to this branch?

cpu commented 6 months ago

slow

I cancelled this job that looked stuck and started it again.