Re: [RFC]: Skipping rust crate tests by default

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi Efraim,

Efraim Flashner <efraim@flashner.co.il> writes:

[...]

>> This sounds good except I don't understand how disabling the tests by
>> default help to "make sure that the packages have the correct inputs" ?
>
> When the tests are disabled, if a package shows red on the CI it means
> that either:
> A) there was a bundled shared/static library in the sources which need
> to be removed
> B) The inputs weren't correct and need to be fixed.
> What we're skipping is C) the test suite failed.
>
> When we get to the 'build phase of the cargo-build-system, cargo first
> checks that it has all of the crates listed in the Cargo.toml file, and
> that all of those crates have all of their (cargo-input) dependencies,
> and so on. If any of them are missing then the build will fail. This is
> also why we need to set #:skip-build? #t when we don't include the
> desired cargo-development-inputs.
>
> The change is mainly a quality of life improvement; it decreases the
> time that guix people and CI spend building these crates, and it makes
> it easier to see which, if any, rust packages need to be checked for
> brokenness (with the assumption that a broken or bit-rotted test suite
> isn't a problem).

I understand that maintaining the large fleet of cargo crates packaged
in Guix is a lot of work, but I think we can try some things before
#:tests? #f; I gathered some idea below.

> My premise is that the test suite of crates doesn't necessarily pass
> when built and run with a newer rust and that we shouldn't be concerned
> about it.
>
>> You've explained the rationale here:
>> <https://lists.gnu.org/archive/html/guix-devel/2023-10/msg00182.html>,
>> saying we sometimes use a newer Rust than the package tests are
>> expecting; how does it work in the Rust world?  Don't they always build
>> even older versions against the most recent compiler?  What about the
>> test suites then?  Are these not typically run by users/distributions?
>
> In general, since rust expects all of the crates to be in source form,
> the tests are only run by developers when the crate is being developed.
> If someone comes along and uses that crate as a dependency for their
> project then they don't run the tests. If they continue using that crate
> (at that version) for several years then the developer using that older
> crate as a dependency still only compiles the part of the crate they
> need for their project and they only run the tests for their project,
> not for the crates which they've used as dependencies.

OK.

> As far as distributions, I can talk for Debian that they only provide
> the crates as -dev packages, that is, as the source crates. They make
> sure that they compile (and probably that they pass the test suite) at
> the time that they are packaged, but no one is distributing pre-compiled
> crates as packages to be used as inputs for further packages.

I believe that's the more useful comparison for our discussion; I gather
that even -dev crates are built and have their test suite run when
submitted, but since source packages are not rebuilt (they are a static
archive, right?) then there's no checking that the test suite continues
working in time.

>
> For an example of a failing doc-test, from the rust-nalgebra-0.21 crate:
>
>
>    Doc-tests nalgebra
> error: unnecessary parentheses around index expression
>   --> /tmp/guix-build-rust-nalgebra-0.21.1.drv-0/nalgebra-0.21.1/src/linalg/convolution.rs:45:47
>    |
> 45 |                 conv[i] += self[u_i] * kernel[(i - u_i)];
>    |                                               ^       ^
>    |
> note: the lint level is defined here
>   --> /tmp/guix-build-rust-nalgebra-0.21.1.drv-0/nalgebra-0.21.1/src/lib.rs:78:9
>    |
> 78 | #![deny(unused_parens)]
>    |         ^^^^^^^^^^^^^
> help: remove these parentheses
>    |
> 45 -                 conv[i] += self[u_i] * kernel[(i - u_i)];
> 45 +                 conv[i] += self[u_i] * kernel[i - u_i];
>    |
>
> error: unnecessary parentheses around index expression
>   --> /tmp/guix-build-rust-nalgebra-0.21.1.drv-0/nalgebra-0.21.1/src/linalg/convolution.rs:49:53
>    |
> 49 |                         conv[i] += self[u] * kernel[(i - u)];
>    |                                                     ^     ^
>    |
> help: remove these parentheses
>    |
> 49 -                         conv[i] += self[u] * kernel[(i - u)];
> 49 +                         conv[i] += self[u] * kernel[i - u];
>    |
>
> error: aborting due to 2 previous errors
>
> error: doctest failed, to rerun pass `--doc`
>
>
> crates.io lists this version as being released more than 3 years ago and
> targeting the 2018 edition of rust. When built with our current
> rust-1.68.2 the doc test passes but not with 1.70.0.  The current
> upstream version of nalgebra is 0.32.3, so it's unlikely that they'd
> release a new version with the doc tests fixed, but I haven't contacted
> them about it.

OK.  Asking in ##rust on libera chat (unofficial channel), I got as suggestion
to call 'carge test' with the '--cap-lints=allow' option documented
here [0], which should effectively disable just the lint checks, which
is better than disabling the full test suite.

[0]  https://doc.rust-lang.org/rustc/lints/levels.html

>> For one thing the 'guix lint' command would need to be told that
>> cargo-build-system has #:tests? set to #f by default to not warn without
>> reasons that '#:tests? #t' is unnecessary.
>
> Instead of #:tests? #t I used (not (%current-target-system)), but I
> hadn't tested it with guix lint to see if that would need to be changed.

That means tests are disabled only when cross-compiling, right?  Do we
support cross-compiling rust crates?

-- 
Thanks,
Maxim