epi-project / brane

Programmable Orchestration of Applications and Networking
Apache License 2.0
9 stars 7 forks source link

chore: Update Cargo minimal versions #92

Closed DanielVoogsgerd closed 4 days ago

DanielVoogsgerd commented 1 week ago

Minimal versions were almost universally set incorrectly. This was a huge endeavour to fix, however I tried to be diligent about it. It will be the case that some are set too high now, but too high is a lot better than the previous too low.

This only takes take of direct minimal versions. Preferably we would also ensure that the transient dependencies have the correct minimal versions, but this will suffice (for now).

I can only fully test this when #90 has been merged, as some builds fail on the base of this branch.

Lut99 commented 6 days ago

Hmm, so, from what I'm seeing, this seems to fix versions to a particular patch, as well? Is that intended? AFAIK does this not specify a minimum, but a fixed version. Is that correct?

My strategy so far is to commit ourselves to minor version numbers only, as that usually encodes breaking change in the Rust ecosystem. That way, we can call cargo update and be relatively sure we're pulling the newest (security) patches without breaking changes. And if we do need breaking changes, this will need manual intervention anyway.

But maybe you have another strategy in mind?

DanielVoogsgerd commented 5 days ago

It does not :)

From the Book:

The string "0.1.12" is a version requirement. Although it looks like a specific version [...], it actually specifies a range of versions and allows SemVer compatible updates.

This is why I was nagging about those minimal versions. Specifying a version 1, means that we support all versions >=1.0.0, <2.0.0. This is often not the case, as features we rely on can be introduced in minor versions.

With the version numbers set to the lowest version, we indicate that this is the lowest version of the dependency that could/would work. Since Cargo.toml follows SemVer and SemVer is backwards compatible for minor and patch, all subsequent versions should work as well.

When running cargo update, you will probably see that all locked versions in Cargo.lock will stay the same, as a specified version 1 will share the same most recent version as 1.2.3 for example.

I hope that clarifies it a bit, if it doesn't feel free to ask some more or shoot me an email.

Lut99 commented 4 days ago

I see. So how can I read this minimum version? Does it encode >= 0.1.12, <1.0.0?

If so, then I think Cargo's making a little too many assumptions, because I know many projects are actually scared of updating the major version and introduce breaking changes in minor versions instead (e.g., reqwest). But even if, specifying only 0.1 vs 0.1.12 makes no difference, so then I guess we might as well specify the full version.

But I learned something today! So thanks! And at the very least I agree that all 0 should be replaced with 0.1, haha. So keep at it :)

DanielVoogsgerd commented 4 days ago

Kinda, you are close.

1.2.3 would encode versions >=1.2.3, <2.0.0 as by nature of backwards compatibility all versions in that range should be compatible with 1.2.3. crate = "1.2.3" is completely semantically equivalent with crate = "^1.2.3.".

What is different in your example and I think that is also what reqwest was facing (although that would be speculation on my part without knowing the specific example) is that below version 1.0.0, so versions 0.x.y, x actually denotes a major version instead of a minor one. I think this was done so that crates could break api without releasing a 1.0.0 release which is associated with api stablization and production readiness.

With respect to with it makes no difference, that is true in most cases, but it need not be. cargo update will by default pick the latest versions for your lockfile, but there are some complications with that. But there is a flag on cargo called -Z direct-minimal-versions that will update the lock versions to the smallest combination of versions possible, i.e. the minimal versions. This is also what I used to test that these minimal versions still compile.

This is not all excessive bookkeeping, having set incorrect minimal versions has let to bugs in some cases.

There is some tooling available to make this rather tedious process more managable.

Lut99 commented 4 days ago

Ohhh, I see. I wasn't aware that 0.x.y automatically makes x the major version. That's quite neat!

Either way, yes ~ let's keep the minimum versions up. Thanks!