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

actix-tls v3 uses str_split_once and bumps MSRV to 1.52 #433

Closed
david-mcgillicuddy-moixa opened this issue Jan 4, 2022 · 4 comments · Fixed by #434
Closed

actix-tls v3 uses str_split_once and bumps MSRV to 1.52 #433

david-mcgillicuddy-moixa opened this issue Jan 4, 2022 · 4 comments · Fixed by #434

Comments

@david-mcgillicuddy-moixa
Copy link
Contributor

Hi,
For reasons I don't really want to get into, for the time being we're stuck on rust version 1.51. Actix-tls v3 uses str::split_once (and actix-http uses str::rsplit_once) which bumps the MSRV up to 1.54. For the time being we've downgraded to actix-tls 3.0.0-beta.9 which doesn't do this, which is fine for now.

However, given that it's a small helper method, would you be open to a PR that replaces its use, or at least gates it behind some kind of compile flag? Happy to submit it. I realise we have just missed the beta/rc period too, awkward.
Otherwise I guess we'll have to maintain a fork for just those methods.

It is annoying that rust/cargo doesn't currently provide much feedback on this if you're not stuck on an old version yourself, although support is coming: rust-lang/rust#65262

Thanks,
David

@robjtede
Copy link
Member

robjtede commented Jan 4, 2022

https://doc.rust-lang.org/std/primitive.str.html#method.split_once is marked as stable in 1.52. If you're really needing to stay on 1.51 then I'll consider a PR.

@david-mcgillicuddy-moixa
Copy link
Contributor Author

david-mcgillicuddy-moixa commented Jan 5, 2022

That's a good point thanks, I haven't tried a 1.52 upgrade (only 1.54 unsuccessfully) I'll do that now and get back to you.

I'm not trying to be mysterous or anything, the reason we're stuck on 1.51 is because we're stuck on Rocko version of Yocto build (for reasons I again don't especially want to get into) and meta-rust upgraded past rocko before upgrading to 1.54 (and straight past 1.52). We're working on upgrading our yocto build but given that this was some string helper fuctions it'll be a lot easier to switch out while the big build upgrade is ongoing.

If the 1.52 upgrade doesn't work I'll start on a PR thanks, would you prefer just removing the uses of split_once or feature flagging them so that users not stuck 6+ months in the past will see no changes after feature flag resolution?

@david-mcgillicuddy-moixa david-mcgillicuddy-moixa changed the title actix-tls v3 uses str_split_once and bumps MSRV to 1.54 actix-tls v3 uses str_split_once and bumps MSRV to 1.52 Jan 5, 2022
@david-mcgillicuddy-moixa
Copy link
Contributor Author

david-mcgillicuddy-moixa commented Jan 5, 2022

Okay unfortunately it's no good, between rust-1.51.0 and rust-1.52.0, the commit of LLVM used was changed from c6f9d6db7b to ea6bb2615f, which changes the minimum version of cmake from 3.4.3 to 3.13.4. Upgrading that would have chain effects and at that point we might as well upgrade the whole OS to a version that supports 1.54 anyway.

Additionally while messing around with versions I noticed that actix-web-4.0.0-beta.18 adds use of extended_key_value_attributes in some of the doc comments, which does actually takes us to 1.54 this time. Forking it is then, and I guess we can discuss the PRs as they come. Thanks for your 1.52 suggestion though.

@david-mcgillicuddy-moixa
Copy link
Contributor Author

Just an update on this, sorry to bother you again, but we have upgraded to 1.58.1 so this is no longer an issue. I can put a patch reverting this change if you want.
For those watching from home we switched to meta-rust-bin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants