Fix `sfv` breaking changes

[BSteffaniak]

This fixes the compilation errors introduced in sfv 0.11. (fixes https://github.com/cloudflare/pingora/issues/568)

Parser::{from_bytes, from_str} was merged into Parser::new: https://github.com/undef1nd/sfv/pull/158

GenericBareItem::bare_item.as_token was updated to return &TokenRef, which needs to be converted to a &str explicitly: https://github.com/undef1nd/sfv/pull/159

sfvs MSRV was set to 1.77 as well with their change: https://github.com/undef1nd/sfv/pull/160. There was already an implied 1.77 MSRV, even in 0.10.4. The update they made to 0.11.0 just made it explicit: https://github.com/undef1nd/sfv/issues/142

This means that the pingora msrv would need to be bumped as well. Assuming that means that the changes in this PR would need to be a major change in pingora.

Let me know your thoughts. Thanks!

[ibatanov]

Maybe it makes sense to fix the version to sfv = "0.11.0"? https://github.com/cloudflare/pingora/blob/main/pingora-core/Cargo.toml#L60C1-L60C10

Today I was surprised to find that the assembly pipelines in my project had collapsed.

This would avoid unpredictable behavior.

[BSteffaniak]

I believe you mean sfv = "0.10"?

I did mention that in the PR description. @zonblade has create a PR with the temporary fix here: https://github.com/cloudflare/pingora/pull/572, although I'm not sure if the full 'patch' pin is necessary.

This PR is primarily for fixing sfv for usage going forward with it. For a temporary fix, I see #572 as sufficient.

[rspendl]

I believe you mean sfv = "0.10"?

I did mention that in the PR description. @zonblade has create a PR with the temporary fix here: #572, although I'm not sure if the full 'patch' pin is necessary.

This PR is primarily for fixing sfv for usage going forward with it. For a temporary fix, I see #572 as sufficient.

Two days ago sfv 0.11.0 was published on crates.io (https://crates.io/crates/sfv) so @ibatanov was right, it's 11, not 10.

[BSteffaniak]

I believe you mean sfv = "0.10"? I did mention that in the PR description. @zonblade has create a PR with the temporary fix here: #572, although I'm not sure if the full 'patch' pin is necessary. This PR is primarily for fixing sfv for usage going forward with it. For a temporary fix, I see #572 as sufficient.

Two days ago sfv 0.11.0 was published on crates.io (https://crates.io/crates/sfv) so @ibatanov was right, it's 11, not 10.

I understand that, but he is saying the fix is to pin the version to 0.11, which is the new version that breaks current pingora.

[rspendl]

I believe you mean sfv = "0.10"? I did mention that in the PR description. @zonblade has create a PR with the temporary fix here: #572, although I'm not sure if the full 'patch' pin is necessary. This PR is primarily for fixing sfv for usage going forward with it. For a temporary fix, I see #572 as sufficient.

Two days ago sfv 0.11.0 was published on crates.io (https://crates.io/crates/sfv) so @ibatanov was right, it's 11, not 10.

I understand that, but he is saying the fix is to pin the version to 0.11, which is the new version that breaks current pingora.

My bad, I thought this is a fix of Pingora so it can run with sfv 0.11.0.

[BSteffaniak]

My bad, I thought this is a fix of Pingora so it can run with sfv 0.11.0.

I see. I understood his message as suggesting the temporary fix (as done in #572).

I think you are correct. Specifying the full version in this PR could be a good idea to make sure pingora is further protected against similar updates.

[ibatanov]

I believe you mean sfv = "0.10"?

I did mention that in the PR description. @zonblade has create a PR with the temporary fix here: #572, although I'm not sure if the full 'patch' pin is necessary.

This PR is primarily for fixing sfv for usage going forward with it. For a temporary fix, I see #572 as sufficient.

Two days ago sfv 0.11.0 was published on crates.io (https://crates.io/crates/sfv) so @ibatanov was right, it's 11, not 10.

I understand that, but he is saying the fix is to pin the version to 0.11, which is the new version that breaks current pingora.

misunderstood me, the fix is what you wrote, plus I meant to fix the version ;)

[muse254]

This is out of scope for this fix but would it also make sense to pin this dependency and any similar pre-v1? https://github.com/cloudflare/pingora/blob/main/pingora-core/Cargo.toml#L67 @BSteffaniak

[samanpa]

Can the fix above be merged? Without it, one cannot compile latest pingora-core?

[andrewhavck]

Sorry for the delay, we had an internal commit synced fixing the issue here.

https://github.com/cloudflare/pingora/pull/586/commits/cb35ff7e32dd92bad9cb83594954c924dc3bd052

[andrewhavck]

Re-opening as I believe the intention here is to actually upgrade. Breaking build is fixed though as of https://github.com/cloudflare/pingora/commit/cb35ff7e32dd92bad9cb83594954c924dc3bd052.

[zonblade]

Re-opening as I believe the intention here is to actually upgrade. Breaking build is fixed though as of https://github.com/cloudflare/pingora/commit/cb35ff7e32dd92bad9cb83594954c924dc3bd052.

oh, good good. 🤔

[BSteffaniak]

@andrewhavck Do you want to bump the MSRV to 1.77.0 in this as well?

[BSteffaniak]

I went ahead and added a commit to bump the MSRV to 1.77.0. I can revert it if desired.

Just to make sure the context around this MSRV change is clear: I mentioned on the 'quick fix' PR (https://github.com/cloudflare/pingora/pull/572#issuecomment-2758009619) that sfv 0.10.4 already technically required rust 1.77.0 to build successfully:

For extra context, I think sfv technically might have already have an implied 1.77 msrv, even in 0.10.4. The update they made to 0.11.0 just made it explicit: https://github.com/undef1nd/sfv/issues/142

[BSteffaniak]

I noticed there were some changes merged in around the MSRV. I removed the code around MSRV in this PR to limit the affected changes since the pipeline should pass with what is now on main.