Relax serde back after upstream issue resolution

[pinkforest]

Fixes #614

I recommend relaxing back now that the issue has been resolved >= 1.0.184

EDIT: Current approach is set to minimum 1.0.184

[codecov]

Codecov Report

Merging #615 (d0b3910) into main (500f8e4) will not change coverage.
The diff coverage is n/a.

❗ Current head d0b3910 differs from pull request most recent head 8026b7b. Consider uploading reports for the commit 8026b7b to get more accurate results

@@          Coverage Diff          @@
##            main    #615   +/-   ##
=====================================
  Coverage   98.6%   98.6%           
=====================================
  Files         79      79           
  Lines       9034    9034           
=====================================
  Hits        8912    8912           
  Misses       122     122           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[pinkforest]

Is there a way to create SemVer range like we did here: rustsec/advisory-db#1417

We were able to use:

[versions]
patched = [">= 0.2.8, < 0.3.0-rc.1", ">= 0.3.0-rc.2"]

But with Cargo's dependency resolution I don't see / remembe a way how did you define multiple ranges.

Alternatively - as per current HEAD - there is an approach to cfg-gate where by:

• opt-in RUSTFLAGS='--cfg precompiled="allow"' -> opts-into full SemVer range
• By default it only allows >= 1.0.175

Please let me know which way would be mergeable and I will adjust per preference.

Still looking a way to do multi-range scoping but I am not sure this was possible.

[pinkforest]

@epage do we remember if there was a possibility in Cargo to do multi-range to exclude patch versions within the range like we did in rustsec/advisory-db#1417 ?

Also is this stable approach and are there any gotchas that would motivate to perhaps use unrestricted range ?

Maintainer/s may not be amenable to SemVer relaxation that allows the whole range across all SemVer - any other recommended approaches ?

Cc/ @bjorn3 @Nemo157 any other reasonably approaches other than relaxing simple most permissive range w/o gating ?

My preference would be just go back to >= 1.0.126 but that may or may not be mergeable - see: #614 (comment)

[AlexTMjugador]

If time really wants to go with this route, can Cargo's platform-specific dependencies feature be used to make all these constraints exclusive to the x86_64-unknown-linux-gnu target? Precompiled binaries never were a concern for other platforms, as serde used traditional macro expansion instead.

[kayabaNerve]

Is it guaranteed the blob will only execute on x86_64-unknown-linux-gnu and not any other supported target? If not, the presence on system of an unknown executable may still be considered a security issue.

(I legitimately don't know if some 32-bit Linux systems can run 64-bit executables via compatibility layers, and would question if the binary runs on linux-musl, freebsd, or so on)

Regardless, this seems like an over-relaxation given the default should be fine for anyone not worried about msrv, and for msrv-concerned parties, a pre-171 cfg would work fine. It also means we don't have to worry about Windows/macOS devs of time-dependents creating builds which don't work on Linux due to Linux having more requirements.

[AlexTMjugador]

Is it guaranteed the blob will only execute on x86_64-unknown-linux-gnu and not any other supported target?

Yes, that is guaranteed at build-time via target flags, as you can see in the source code: https://github.com/serde-rs/serde/blob/bfcd44704f847ac5a9f3072e102e803b5ebbef31/precompiled/serde_derive/src/lib.rs#L18-L22

The binary is still downloaded as a part of the serde_derive crate files for other platforms, but it's unused.

Edit:

It also means we don't have to worry about Windows/macOS devs of time-dependents creating builds which don't work on Linux due to Linux having more requirements.

Fair enough. In my opinion, time should not impose these restrictions, and should wait until the binaries of the affected versions are undoubtedly reproduced. But I will respect other choices.

[kayabaNerve]

That wasn't my question.

I understand serde_derive will only execute it on x86_64-unknown-linux-gnu. I'm curious if it's only executable on x86_64-unknown-linux-gnu.

I don't personally believe time should have such platform-specific Cargo.toml validity.

[AlexTMjugador]

I understand serde_derive will only execute it on x86_64-unknown-linux-gnu. I'm curious if it's only executable on x86_64-unknown-linux-gnu.

If QEMU is installed and binfmt_misc set up for that, then no, potentially any other Linux target can execute those binaries. But serde_derive won't do that, and requiring QEMU for executing those binaries narrows the cases where they may be harmful.

[kayabaNerve]

I repeat I wouldn't be surprised if musl systems could run it (especially if they have gcompat). It should be able to if nothing is dynamically linked (which isn't still present on musl targets). FreeBSD also advertises Linux compatibility, and while it's optional, the question is now how many FreeBSD users have so opted in.

Unless it's known for sure it's not planting an in-the-future-executable blob, on any system, I'd personally still consider the binary blob a security risk.

[jhpratt]

Given that Cargo doesn't have support for multiple ranges, my preferred solution at this point is to set the requirement to 1.0.184. Depending on the output of #611, it can be relaxed back to what it was before. The output of that seems nearly certain at this point, but there's still a few hours left. The range can still be expanded if and when the binaries are fully reproduced.

With regard to cfg-gating other ranges, I think that'll cause more confusion than anything.

[pinkforest]

Ok - given >= 1.0.184 is the preference here.

EDIT: Ignore below - checked the reverse-dependencies this should not cause problems.
~~
Risks

This would break anyone who uses <= 1.0.171 elsewhere including when there is / are another crate/s who pinned.

Likewise anyone who has defined serde >= 1.0.184 would break with current time which is set to <= 1.0.171

There would be two strategies I believe to address this:

Approach 1: Bump time to 0.4

So people who have either used time = "0.3" or time = "0.3" can bump it in controlled manner as releasing in 0.3 track would break anyone who uses serde with any dependency that has pinned to <= 1.0.171 as time has so far done.

This approach would still regardless break anyone who has specified time = "0" which would have been illadvised regardless as it's not stable 1.0.0 yet.

Nonetheless I think this is the easiest approach to do now leaving:

• 0.3 release track with <= 1.0.171
• 0.4 release track with >= 1.0.184

This however has the downside that it may create backporting pressure to 0.3

But I think everyone should move to 0.4 regardless so would say it's less of a risk having to backport to 0.3 later.

Approach 2: cfg approach

Yes this would add additional complexity true - this could have been something we could maybe get rest of the ecosystem motivated with to have unique mechanism to weed out desired dependencies depending on target environment requirements - but yeah it's complex.

Personally I prefer both 1) 2) but just setting these out fwiw :)

I'll set the version to >= 1.0.184 and I would recommend addressing the risk at least 1) if not 2) as well~~

[Nemo157]

This would break anyone who uses <= 1.0.171 elsewhere including when there is / are another crate/s who pinned.

It won't break anything, it'll just not be possible to have those versions in the same buildgraph as the next version of time. As people relax/update their requirements too the ecosystem will naturally recover to the same state it was in, where the latest versions of everything are compatible.

[pinkforest]

Yes, there can be time 0.3 and time 0.4 in buildgraph - but I thought oen can't have time 0.3.26 and time 0.3.27 same time as it's in the 0.3 range conflciting requirement. Unless one uses --locked and it will create build problems afaik ?

I'll double check this quick since that's how I understood it before and saw it happen - unless I'm dreaming 🤔

[Nemo157]

I mean that just releasing this as 0.3.x won't break anything, there'll be a short time when it won't be possible to have the latest versions of everything in a build but once everyone has relaxed their requirements everything will be good again. There's no mitigations required unless some crates are not intending to allow use of 1.0.184+.

[pinkforest]

Yes that is what I highlighted as a risk here - the ones who pinned to lower-range - like time did - would cause problems.

I'm just trying to find who else pinned to <= 1.0.172 to see which exact deps and it may be there isn't much.

It doesn't help crates.io barfs on listing dependants under serde :)

EDIT: Just checked there doesn't seem to be anyone else than time who is left to unpin so should be good to release in 0.3

This PR should be good as-is yes then 🥳

[jhpratt]

0.4 wouldn't make sense, as it's not a breaking change. --cfg isn't ideal because of both the interaction with other crates and the non-testing in CI here (and I don't want to make CI times far worse).

The PR looks good as-is. Thank you @pinkforest and everyone else for their input! A release will be out shortly — CI takes about 25 minutes to validate everything for a proper release.