unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [RFC]: Skipping rust crate tests by default
@ 2023-10-05 12:52 Efraim Flashner
  2023-10-05 16:38 ` Felix Lechner via Development of GNU Guix and the GNU System distribution.
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Efraim Flashner @ 2023-10-05 12:52 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]

Currently for for rust crates we build the crates, run the tests, and
then in %output we only have the license files and a repackaged version
of the source.

The build system goes:
unpack source
unpack crates
patch shebangs
patch checksums of the crates
'build
'package
'check
'install

'install is clear, it does whatever the install command is.

'package repacks the source crate, after we've done any changes to it in
the snippet and later if we've gone and patched paths to binaries or
libraries. In theory this is useful with using these crates in a
GUIX_ENVIRONMENT

'check runs the test suite, which fairly often seems to need some
massaging to skip the odd test which fails or to try to skip the doc
tests, which fail far too often.

'build sounds like it just builds the package. The first thing it does
it makes sure that all the necessary crates are included in the build
environment.

IMO the 'build phase is the most important one, it's the one that lets
us know if all the cargo-inputs and cargo-development-inputs are
correct. We don't care if rust-rand-0.6 or rust-nb-connect-1 builds, we
only care that it has the correct inputs so that when we pull it in for
an actual binary or library everything builds correctly.

I propose changing the cargo-build-system to have '#:tests? #f' by
default and then enable them for packages which have a "clear output".
It will keep the benefits of knowing we have the correct inputs without
worrying about test errors we don't care about. If it fails to build
during its own 'build phase that's actually worth looking into. It will
also cut down the amount of time the CI spends building unneeded rust
crates, and lets us see which ones are actually broken.


-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
  2023-10-05 12:52 [RFC]: Skipping rust crate tests by default Efraim Flashner
@ 2023-10-05 16:38 ` Felix Lechner via Development of GNU Guix and the GNU System distribution.
  2023-10-15 10:18   ` Josselin Poiret
  2023-10-16 15:47 ` Maxim Cournoyer
  2023-10-18  8:36 ` Efraim Flashner
  2 siblings, 1 reply; 10+ messages in thread
From: Felix Lechner via Development of GNU Guix and the GNU System distribution. @ 2023-10-05 16:38 UTC (permalink / raw)
  To: Efraim Flashner, guix-devel

Hi Ephraim,

On Thu, Oct 05 2023, Efraim Flashner wrote:

> I propose changing the cargo-build-system to have '#:tests? #f' by
> default

At first sight, it appears improper to turn off tests because they
fail. Please allow me to remind everyone that build-time tests cover
only a small proportion of problems actually encountered by users.

Most packaging errors, like improper permissions or the installation of
components in a wrong path, usually go undetected.

One of Debian's solutions to that realization is autopkgtest [1] which
allows maintainers to specify a test bed that tests the *installed*
versions and not the versions in the build trees.

A common strategy for Debian maintainers is to convert the build-time
tests to an autopkgtest suite. That way, folks get the benefit of both.

Unfortunately, the setup of test beds is complicated in Debian, as the
installation of the packages being tested has to take place in
containers. In Guix, package installations are decoupled from the
running system. Guix would make that process a lot easier, faster and
more reliable!

In summary, I believe that #:test? should be turned off for all build
systems. Guix should instead test installed versions like Debian's
autopkgtest.

It would be an extra burden on contributors because such a
'test-installed phase would require more attention. It may be
worthwhile, however, because than packages could be built without
testing them---as Ephraim would like to do here.

In addition, pre-built substitutes could be tested by consumers on their
own systems. The substitutes could even be tested before they become
part of any Guix profile.

For Debian's QA tool Lintian, which I maintained for several years, the
speed-up in the development process was remarkable. As a Perl script,
the build toook seven minutes, while the build-time tests took seven
hours.

The builds were initiated with each commit in Salsa (in an online runner
sponsored by a large company).

Lintian was extreme because thousands of tests replicated much of what
happens daily in Debian. The extreme duration of build-time the tests
also took up further resources in downstream distributions. The tests
ran for each backport and for each derivative, such as Ubuntu.

For Guix, which relies on frequent rebuilds, the speed benefit could be
remarkable. Substitutes could become available for testing in perhaps
half the time.

That being said, old habits die hard. The attachment to build-time tests
is formidable. The people who maintained Lintian after me enabled them
again.

Kind regards
Felix

[1] https://salsa.debian.org/ci-team/autopkgtest/-/blob/master/doc/README.package-tests.rst


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
@ 2023-10-05 18:13 Nathan Dehnel
  0 siblings, 0 replies; 10+ messages in thread
From: Nathan Dehnel @ 2023-10-05 18:13 UTC (permalink / raw)
  To: Felix Lechner, guix-devel

As a developer, the majority of the package build failures I encounter
are from failed tests, so I agree with this proposal.

I also like the idea of clients testing their own packages instead of
trusting the substitute server.

And if the new tests would catch more packaging bugs, that would be great too.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
  2023-10-05 16:38 ` Felix Lechner via Development of GNU Guix and the GNU System distribution.
@ 2023-10-15 10:18   ` Josselin Poiret
  0 siblings, 0 replies; 10+ messages in thread
From: Josselin Poiret @ 2023-10-15 10:18 UTC (permalink / raw)
  To: Felix Lechner, Efraim Flashner, guix-devel

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

Hi Felix,

Felix Lechner via "Development of GNU Guix and the GNU System
distribution." <guix-devel@gnu.org> writes:

> In summary, I believe that #:test? should be turned off for all build
> systems. Guix should instead test installed versions like Debian's
> autopkgtest.

Let me just chime in and say that I agree: tests should not be a part of
a build.  This would:

1) avoid time bombs in tests breaking builds;
2) gain a lot of time when iterating package definitions;
3) let us control the testing environment more precisely, use
containers, mock stuff more precisely, etc.

However, this would require refactoring *all* guix package definitions,
after first coming up with a satisfying testing strategy and system.
I don't think this will ever happen unfortunately.

Best,
-- 
Josselin Poiret

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 682 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
  2023-10-05 12:52 [RFC]: Skipping rust crate tests by default Efraim Flashner
  2023-10-05 16:38 ` Felix Lechner via Development of GNU Guix and the GNU System distribution.
@ 2023-10-16 15:47 ` Maxim Cournoyer
  2023-10-17  7:46   ` Efraim Flashner
  2023-10-18  8:36 ` Efraim Flashner
  2 siblings, 1 reply; 10+ messages in thread
From: Maxim Cournoyer @ 2023-10-16 15:47 UTC (permalink / raw)
  To: guix-devel

Hi Efraim,

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

> Currently for for rust crates we build the crates, run the tests, and
> then in %output we only have the license files and a repackaged version
> of the source.
>
> The build system goes:
> unpack source
> unpack crates
> patch shebangs
> patch checksums of the crates
> 'build
> 'package
> 'check
> 'install
>
> 'install is clear, it does whatever the install command is.
>
> 'package repacks the source crate, after we've done any changes to it in
> the snippet and later if we've gone and patched paths to binaries or
> libraries. In theory this is useful with using these crates in a
> GUIX_ENVIRONMENT
>
> 'check runs the test suite, which fairly often seems to need some
> massaging to skip the odd test which fails or to try to skip the doc
> tests, which fail far too often.

Why do the doc tests often fail?  Is something wrong on our side?  As
rust aims to be fully deterministic, unless we stray too far away from
the specified input versions, it seems these failures should not happen
or be reported upstream.

> 'build sounds like it just builds the package. The first thing it does
> it makes sure that all the necessary crates are included in the build
> environment.
>
> IMO the 'build phase is the most important one, it's the one that lets
> us know if all the cargo-inputs and cargo-development-inputs are
> correct. We don't care if rust-rand-0.6 or rust-nb-connect-1 builds, we
> only care that it has the correct inputs so that when we pull it in for
> an actual binary or library everything builds correctly.
>
> I propose changing the cargo-build-system to have '#:tests? #f' by
> default and then enable them for packages which have a "clear output".
> It will keep the benefits of knowing we have the correct inputs without
> worrying about test errors we don't care about. If it fails to build
> during its own 'build phase that's actually worth looking into. It will
> also cut down the amount of time the CI spends building unneeded rust
> crates, and lets us see which ones are actually broken.

It seems useful to me to have some assurance that each crate packaged in
Guix passes its test suite, so I'm reticent to have a #:tests? #f by
default, while I sympathize with the work that is it given the sheer
amount of Rust crates.

-- 
Thanks,
Maxim


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
  2023-10-16 15:47 ` Maxim Cournoyer
@ 2023-10-17  7:46   ` Efraim Flashner
  0 siblings, 0 replies; 10+ messages in thread
From: Efraim Flashner @ 2023-10-17  7:46 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 3566 bytes --]

On Mon, Oct 16, 2023 at 11:47:53AM -0400, Maxim Cournoyer wrote:
> Hi Efraim,
> 
> Efraim Flashner <efraim@flashner.co.il> writes:
> 
> > Currently for for rust crates we build the crates, run the tests, and
> > then in %output we only have the license files and a repackaged version
> > of the source.
> >
> > The build system goes:
> > unpack source
> > unpack crates
> > patch shebangs
> > patch checksums of the crates
> > 'build
> > 'package
> > 'check
> > 'install
> >
> > 'install is clear, it does whatever the install command is.
> >
> > 'package repacks the source crate, after we've done any changes to it in
> > the snippet and later if we've gone and patched paths to binaries or
> > libraries. In theory this is useful with using these crates in a
> > GUIX_ENVIRONMENT
> >
> > 'check runs the test suite, which fairly often seems to need some
> > massaging to skip the odd test which fails or to try to skip the doc
> > tests, which fail far too often.
> 
> Why do the doc tests often fail?  Is something wrong on our side?  As
> rust aims to be fully deterministic, unless we stray too far away from
> the specified input versions, it seems these failures should not happen
> or be reported upstream.

Addressing not just the doc tests but some of the failing tests in
general:

Often it is because it is an older version of a library, released for an
older version of rust. Over time rust has been 'tightening' some of
their allowed code patterns, meaning that while in the past tests might
have passed with a warning, they now fail.

There are some packages which haven't honored rust's semver guarantee as
strongly as they might have, and those have resulted in some libraries
failing when we upgrade some of the more heavily depended upon
libraries, like serde.

It's possible that we've packaged yanked versions of crates, the
importer isn't as careful about that as it could be. I have a patch in
the bugtracker to not import yanked crates.

> > 'build sounds like it just builds the package. The first thing it does
> > it makes sure that all the necessary crates are included in the build
> > environment.
> >
> > IMO the 'build phase is the most important one, it's the one that lets
> > us know if all the cargo-inputs and cargo-development-inputs are
> > correct. We don't care if rust-rand-0.6 or rust-nb-connect-1 builds, we
> > only care that it has the correct inputs so that when we pull it in for
> > an actual binary or library everything builds correctly.
> >
> > I propose changing the cargo-build-system to have '#:tests? #f' by
> > default and then enable them for packages which have a "clear output".
> > It will keep the benefits of knowing we have the correct inputs without
> > worrying about test errors we don't care about. If it fails to build
> > during its own 'build phase that's actually worth looking into. It will
> > also cut down the amount of time the CI spends building unneeded rust
> > crates, and lets us see which ones are actually broken.
> 
> It seems useful to me to have some assurance that each crate packaged in
> Guix passes its test suite, so I'm reticent to have a #:tests? #f by
> default, while I sympathize with the work that is it given the sheer
> amount of Rust crates.
> 
> -- 
> Thanks,
> Maxim
> 

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
  2023-10-05 12:52 [RFC]: Skipping rust crate tests by default Efraim Flashner
  2023-10-05 16:38 ` Felix Lechner via Development of GNU Guix and the GNU System distribution.
  2023-10-16 15:47 ` Maxim Cournoyer
@ 2023-10-18  8:36 ` Efraim Flashner
  2023-10-18 18:46   ` Maxim Cournoyer
  2 siblings, 1 reply; 10+ messages in thread
From: Efraim Flashner @ 2023-10-18  8:36 UTC (permalink / raw)
  To: guix-devel, Maxim Cournoyer; +Cc: Ludovic Courtès

[-- Attachment #1: Type: text/plain, Size: 7458 bytes --]

On Thu, Oct 05, 2023 at 03:52:46PM +0300, Efraim Flashner wrote:
> Currently for for rust crates we build the crates, run the tests, and
> then in %output we only have the license files and a repackaged version
> of the source.
> 
> The build system goes:
> unpack source
> unpack crates
> patch shebangs
> patch checksums of the crates
> 'build
> 'package
> 'check
> 'install
> 
> 'install is clear, it does whatever the install command is.
> 
> 'package repacks the source crate, after we've done any changes to it in
> the snippet and later if we've gone and patched paths to binaries or
> libraries. In theory this is useful with using these crates in a
> GUIX_ENVIRONMENT
> 
> 'check runs the test suite, which fairly often seems to need some
> massaging to skip the odd test which fails or to try to skip the doc
> tests, which fail far too often.
> 
> 'build sounds like it just builds the package. The first thing it does
> it makes sure that all the necessary crates are included in the build
> environment.
> 
> IMO the 'build phase is the most important one, it's the one that lets
> us know if all the cargo-inputs and cargo-development-inputs are
> correct. We don't care if rust-rand-0.6 or rust-nb-connect-1 builds, we
> only care that it has the correct inputs so that when we pull it in for
> an actual binary or library everything builds correctly.
> 
> I propose changing the cargo-build-system to have '#:tests? #f' by
> default and then enable them for packages which have a "clear output".
> It will keep the benefits of knowing we have the correct inputs without
> worrying about test errors we don't care about. If it fails to build
> during its own 'build phase that's actually worth looking into. It will
> also cut down the amount of time the CI spends building unneeded rust
> crates, and lets us see which ones are actually broken.

To add some more context from other places and to try to flesh out more:

On Tue, Oct 17, 2023 at 11:41:11AM -0400, Maxim Cournoyer wrote:
> Hi Efraim,
> 
> Efraim Flashner <efraim@flashner.co.il> writes:
> 
> > IMO rust-team branch is ready to merge. We've updated rust to 1.70,
> > librsvg to 2.56.4 and many new and updated packages. We've added a phase
> > to the cargo-build-system to fail if it detects pre-built files and
> > we've set the cargo-build-system to skip the test phase by default,
> > allowing us to make sure that the packages have the correct inputs. With
> > these changes I've gotten 100% of the packages built using the
> > cargo-build-system to build successfully.
> 
> 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).

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.

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.


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.

> 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.

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
  2023-10-18  8:36 ` Efraim Flashner
@ 2023-10-18 18:46   ` Maxim Cournoyer
  2023-10-23 10:04     ` Efraim Flashner
  0 siblings, 1 reply; 10+ messages in thread
From: Maxim Cournoyer @ 2023-10-18 18:46 UTC (permalink / raw)
  To: guix-devel; +Cc: Ludovic Courtès

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
  2023-10-18 18:46   ` Maxim Cournoyer
@ 2023-10-23 10:04     ` Efraim Flashner
  2023-10-23 15:51       ` Maxim Cournoyer
  0 siblings, 1 reply; 10+ messages in thread
From: Efraim Flashner @ 2023-10-23 10:04 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: guix-devel, Ludovic Courtès

[-- Attachment #1: Type: text/plain, Size: 6948 bytes --]

On Wed, Oct 18, 2023 at 02:46:57PM -0400, Maxim Cournoyer wrote:
> 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.

The source packages are effectively repackaged tarballs. They've made
sure there's no issues with the DFSG, maybe applied some patches, built
and tested the crates, and then repackaged the tarballs.

> >
> > 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

I checked the cargo-build-system and we do actually use
--cap-lints=allow for building and for testing.

> >> 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?

For those packages, yes. For all the others they would've been disabled
for native and for cross builds.

I'm currently working on cross-building rust packages but have been
running into some problems with llvm or with rustc/cargo itself. I hope
to have it working relatively soon.

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC]: Skipping rust crate tests by default
  2023-10-23 10:04     ` Efraim Flashner
@ 2023-10-23 15:51       ` Maxim Cournoyer
  0 siblings, 0 replies; 10+ messages in thread
From: Maxim Cournoyer @ 2023-10-23 15:51 UTC (permalink / raw)
  To: guix-devel; +Cc: Ludovic Courtès

Hi Efraim!

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

[...]

>> > 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
>
> I checked the cargo-build-system and we do actually use
> --cap-lints=allow for building and for testing.

Ah!  It must be something recent, as it was not the case (and still
isn't) when I checked on the master branch.  Or else I fail to see
where/how it's specified.

And nalgebra still fails lint tests with the above --caps-lints=allow
option?  If that's so that'd suggest that packages can enforce their own
settings and that this overrides cargo flags given at the command
line... which sounds like a cargo bug.

-- 
Thanks,
Maxim


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-10-23 15:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-05 12:52 [RFC]: Skipping rust crate tests by default Efraim Flashner
2023-10-05 16:38 ` Felix Lechner via Development of GNU Guix and the GNU System distribution.
2023-10-15 10:18   ` Josselin Poiret
2023-10-16 15:47 ` Maxim Cournoyer
2023-10-17  7:46   ` Efraim Flashner
2023-10-18  8:36 ` Efraim Flashner
2023-10-18 18:46   ` Maxim Cournoyer
2023-10-23 10:04     ` Efraim Flashner
2023-10-23 15:51       ` Maxim Cournoyer
  -- strict thread matches above, loose matches on Subject: below --
2023-10-05 18:13 Nathan Dehnel

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).