Test builds with minimal versions

[BurntSushi]

Steps to reproduce:

$ rustc --version
rustc 1.34.0-nightly (d8a0dd7ae 2019-01-28)
$ cargo --version
cargo 1.34.0-nightly (245818076 2019-01-27)
$ cargo generate-lockfile -Z minimal-versions
    Updating crates.io index
$ cargo build
  Downloaded autocfg v0.1.0
  Downloaded libc v0.2.0
   Compiling autocfg v0.1.0
   Compiling libc v0.2.0
   Compiling rand_os v0.1.2 (/home/andrew/clones/rand/rand_os)
error[E0425]: cannot find function `syscall` in module `libc`
   --> rand_os/src/linux_android.rs:127:15
    |
127 |         libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(),
    |               ^^^^^^^ help: a function with a similar name exists: `sysctl`

warning: use of deprecated item 'std::sync::atomic::ATOMIC_BOOL_INIT': the `new` function is now preferred
  --> rand_os/src/linux_android.rs:21:37
   |
21 | use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering};
   |                                     ^^^^^^^^^^^^^^^^
   |
   = note: #[warn(deprecated)] on by default

warning: use of deprecated item 'std::sync::atomic::ATOMIC_BOOL_INIT': the `new` function is now preferred
  --> rand_os/src/linux_android.rs:56:49
   |
56 |         static OS_RNG_INITIALIZED: AtomicBool = ATOMIC_BOOL_INIT;
   |                                                 ^^^^^^^^^^^^^^^^

warning: use of deprecated item 'std::sync::atomic::ATOMIC_BOOL_INIT': the `new` function is now preferred
   --> rand_os/src/linux_android.rs:163:36
    |
163 |     static AVAILABLE: AtomicBool = ATOMIC_BOOL_INIT;
    |                                    ^^^^^^^^^^^^^^^^

   Compiling rand_chacha v0.1.1 (/home/andrew/clones/rand/rand_chacha)
   Compiling rand_pcg v0.1.1 (/home/andrew/clones/rand/rand_pcg)
   Compiling rand v0.6.5 (/home/andrew/clones/rand)
error: aborting due to previous error

For more information about this error, try `rustc --explain E0425`.
error: Could not compile `rand_os`.
warning: build failed, waiting for other jobs to finish...
error: build failed

I think the issue here is that rand advertises that it works for all releases of libc that are 0.2 or newer, but this is actually not the case. So the dependency should be updated to require a newer version of libc.

There may be other issues lurking as well, see: crossbeam-rs/crossbeam#312

The regex crate does this check in its CI configuration to make sure its dependency specifications are always correct. See: https://github.com/rust-lang/regex/blob/60d087a23025e045ae754a345b04003c31d83d93/ci/script.sh#L53-L55 --- You might consider doing the same.

[dhardy]

On OSX we actually require a not-yet-released version of libc if using older versions of Rustc: rust-lang/libc#1232

Did you find which version of libc is required here?

[BurntSushi]

I'm on Linux. It looks like 0.2.22 is the minimal version for Linux. I think 0.2.3 added syscall (or thereabouts), but y'all also need pthread_atfork, which was added in 0.2.22.

But this is likely platform dependent. This test probably needs to be run for each supported target.

[dhardy]

Looks like we actually need libc 0.2.27 for the SYS_getrandom function.

I feel like testing the most recent lib version is more important than checking the minimal version; is it really worth having CI runs for each platform (we have several platform-specific deps) to check both?

Edit: I have enabled limited testing of min versions for the getrandom lib, which will become a dependency of this lib. Likely we will not do further min-version testing (though I wouldn't mind doing the same for Windows, if someone knows how to configure AppVeyor).

Edit 2: scratch that; -Z minimal-versions can't be used on stable and all our nightly runners already have jobs to do.

[BurntSushi]

Basically, if minimal versions doesn't work, then to me, there's a bug in the dependency specification because it's claiming support for something that doesn't actually work. The ecosystem is having trouble moving to minimal versions though, because it first needs to be adopted by core crates (like rand). Getting testing working on a common platform seems like a good place to start.

Edit 2: scratch that; -Z minimal-versions can't be used on stable and all our nightly runners already have jobs to do.

I imagine you might just add it to the end of an existing nightly runner?

[dhardy]

Sure it's a bug, but is it one that's worth fixing? The discussion seems to be here and hasn't reached a conclusion on this issue; additionally the mentioned --minimal-versions flag is not yet available on stable Cargo.

I will take some steps to address this (I already updated the minimal versions as I think are required in the getrandom crate, which will supersede the code in rand_os and thus replace its dependencies), and may add limited testing as suggested (but again probably only in the getrandom crate), however I'm also closing this issue since it does not appear there is any consensus on implementing this across the ecosystem due to low importance (i.e. we have more important bugs to fix).

[BurntSushi]

I think your link is broken? In any case, I'm a little confused here. What exactly is preventing the addition of this test to say, the rand crate itself? Is there some hidden burden that I'm missing? I've been using the minimal version check in CI for quite a while and it hasn't been any trouble at all.

[dhardy]

Sorry, fixed.

Pushing platform dependencies to getrandom helps keep CI load low. There are some extra deps in rand, but not many.

[BurntSushi]

Thanks!

[asomers]

Can we please reopen this? If rand doesn't build with its minimal dependencies, then nothing that depends on rand will. According to crates.io, that's 2462 crates.

Here's a start: you need to update the bincode dependency from 1.1.2 to 1.1.4 because bincode 1.1.3 and earlier incorrectly set its minimal autocfg dependency.

[dhardy]

Who's "we"? I personally feel like there's more than enough I'm already doing with this project that I want to support this fairly dubious goal. If you want to bump the minimum versions feel free to open PRs (but also don't expect new releases just for these version bumps).

[BurntSushi]

@dhardy

In this comment, you mention that adding this test is a lot of work. Could you say more about the work you perceive that is involved in adding a minimal version check?

[nox]

I feel like testing the most recent lib version is more important than checking the minimal version; is it really worth having CI runs for each platform (we have several platform-specific deps) to check both?

If a crate says it depends on foo 1.2.3 but it actually requires foo 1.3.0, that's a bug in that crate.

[dhardy]

@nox if a crate "depends" on version 1.2.3, Cargo would (by default) pick the latest 1.2.x version (≥1.2.3).

In contrast when talking about patch versions, any x.y.z with a newer x.y.w (w > z) is effectively obsolete. No matter whether the author wishes to still "support" x.y.z or not, Cargo does not even let you patch it. The only direct control an author still has over such a version is to yank it.

For better or worse, this means that any x.y.z where z is not the latest patch-version is effectively already obsolete and cannot be recommended.

[nox]

This is not true, you can always downgrade crates or just not update them, with cargo update -p. If you say you depend on version 1.2.3 but uses an API from 1.3.0 that’s a bug in your crate.

[dhardy]

If you say you depend on version 1.2.3 but uses an API from 1.3.0 that’s a bug in your crate.

Obviously, and one that the usual CI will pick up already.

[nox]

Obviously, and one that the usual CI will pick up already.

Now I'm confused, that's literally what this issue is asking for and what you are arguing against doing!

[dhardy]

There's a very important difference between patch versions and minor versions. This issue is about testing the minimum supported patch version.

I have no issue with PRs updating the required patch version, I just don't wish to add testing.

[nox]

No, this issue is about minimal versions. If your crate says it works with 1.2.0 but it actually requires 1.2.1, that's a bug in your crate.

[nox]

There is no difference from semver's POV between patch and minor versions on the front of version comparison and compatiblity, backwards compatibility is preserved in both a minor bump or a patch bump.

[dhardy]

But there is a very real difference from the point of view of Cargo:

1. Cargo always selects the latest patch version unless you are very specific about not doing so, or use a very selective update (cargo update -p ...)
2. It is possible to patch any minor version with an update. However, for patch versions, only the latest can be updated. One should therefore assume that any patch version with a newer patch version published is already unsupported, without requiring authors to be anal about yanking all old versions.

[nox]

• Cargo always selects the latest patch version unless you are very specific about not doing so, or use a very selective update (cargo update -p ...)

Yes, just like it will select 1.3.0 if it exists and you say you depend on 1.2.3. Cargo makes no difference about that. It will always use the most recent one, which end users may downgrade with cargo update -p. That's something we do often in Servo because of various reasons which arise when you have a 600 crates graph.

2. It is possible to patch any minor version with an update. However, for patch versions, only the latest can be updated. One should therefore assume that any patch version with a newer patch version published is already unsupported, without requiring authors to be anal about yanking all old versions.

Real world says otherwise, and sometimes people add entirely new APIs in patch versions and whatnot. No idea what you mentioned yanking though.

[nox]

The part I'm confused about the most is that if you already have CI to prevent from depending on 1.2.3 when you actually need 1.3.0, then it costs absolutely nothing more to prevent from depending on 1.2.3 when you actually need 1.2.4: just make a test run with the minimal versions.

[dhardy]

Yes, just like it will select 1.3.0 if it exists and you say you depend on 1.2.3

It does? It does. That invalidates that argument I guess.

No idea what you mentioned yanking though.

Any patch version with a newer patch release for the same major.minor.

[dhardy]

On reflection, I believe we should add some testing with minimal versions as requested.

I guess at this point I owe @BurntSushi and others an apology for dismissing this request, however I would like to request that arguments be based on evidence (e.g. minimal version testing may have prevented the rand 0.6.5 + rand_core 0.3.0 issue; perhaps there are similar issues that may have been avoided in the 2462 dependencies mentioned) rather than opinion ("it's a bug and therefore should be fixed").

[asomers]

Here are some real-world consequences: because rand does not work with its minimal versions, I can't add a minimal versions check to the nix, blosc-rs, mio-aio, or tokio-file crates.

[dhardy]

Fix in #874; a review would be appreciated. Note that I had to add a couple of indirect dependencies to make things work 😬