bug#43746: What to do about packages that don't support --without-tests / #:tests? #f setting

bug#43746: What to do about packages that don't support --without-tests / #:tests? #f setting

[pelzflorian (Florian Pelz)]

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

The new package transformation option --without-tests works by setting
#:tests? #f in the specified packages.  But some packages replace
their 'check phase and no longer honor #tests?.  glib for example.

Attached is an attempt to document this current behavior.  Shall I
push it?  Alternatively, it should be documented to write a check
phase that honors #:tests?.  Or the package transformation should be
changed to remove any check phase it finds.

Regards,
Florian

[-- Attachment #2: 0001-doc-Explain-why-without-tests-may-fail-with-modified.patch --]
[-- Type: text/plain, Size: 1149 bytes --]

From b55e6ee01fe674b282e7ec75d0e4c8a839262261 Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflorian@pelzflorian.de>
Date: Thu, 1 Oct 2020 15:35:52 +0200
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Subject: [PATCH] doc: Explain why '--without-tests' may fail with modified
 'check' phase.

* doc/guix.texi (Package Transformation Options): Explain.
---
 doc/guix.texi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index e8458ad8d8..6a2bd629cb 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -9350,6 +9350,12 @@ The command above installs @code{python-notebook} on top of
 rebuilds everything that depends on @code{python}, including
 @code{python-notebook} itself.
 
+Internally, @code{--without-tests} relies on changing the
+@code{#:tests?} option of a package's @code{check} phase (@pxref{Build
+Systems}).  Note that some packages use a customized @code{check} phase
+that does not respect a @code{#:tests? #f} setting.  Therefore there are
+some packages for which @code{--without-tests} cannot disable tests.
+
 @end table
 
 @node Additional Build Options
-- 
2.28.0

bug#43746: What to do about packages that don't support --without-tests / #:tests? #f setting

[Ludovic Courtès]

Hi,

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> The new package transformation option --without-tests works by setting
> #:tests? #f in the specified packages.  But some packages replace
> their 'check phase and no longer honor #tests?.  glib for example.

Oh, we should fix ‘glib’ in ‘core-updates’.

> Attached is an attempt to document this current behavior.  Shall I
> push it?  Alternatively, it should be documented to write a check
> phase that honors #:tests?.  Or the package transformation should be
> changed to remove any check phase it finds.

Hmm not sure, I think fiddling with phases is more risky or at least
could lead to more obscure errors for example with build systems that
don’t support phases, like ‘trivial-build-system’.

I’m inclined to apply the patch you propose and leaving phases
unchanged.

>>From b55e6ee01fe674b282e7ec75d0e4c8a839262261 Mon Sep 17 00:00:00 2001
> From: Florian Pelz <pelzflorian@pelzflorian.de>
> Date: Thu, 1 Oct 2020 15:35:52 +0200
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> Subject: [PATCH] doc: Explain why '--without-tests' may fail with modified
>  'check' phase.
>
> * doc/guix.texi (Package Transformation Options): Explain.

[...]

> +Internally, @code{--without-tests} relies on changing the
> +@code{#:tests?} option of a package's @code{check} phase (@pxref{Build
> +Systems}).  Note that some packages use a customized @code{check} phase
> +that does not respect a @code{#:tests? #f} setting.  Therefore there are
> +some packages for which @code{--without-tests} cannot disable tests.

I’d change the last sentence to:

  Therefore, @option{--without-tests} has no effect on these packages.

Thanks,
Ludo’.

bug#43746: What to do about packages that don't support --without-tests / #:tests? #f setting

[pelzflorian (Florian Pelz)]

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

On Thu, Oct 01, 2020 at 11:07:54PM +0200, Ludovic Courtès wrote:
> Oh, we should fix ‘glib’ in ‘core-updates’.

I verified the attached patch fixes glib on the ‘master’ branch.  I’m
not sure about adding a copyright.  Anyway.  Shall I push it to
‘core-updates’ or wait until I can test it on ‘core-updates’ without
rebuilding the world?

> I’m inclined to apply the patch you propose and leaving phases
> unchanged.
> […]
> I’d change the last sentence to:
> 
>   Therefore, @option{--without-tests} has no effect on these packages.

I changed it and pushed the patch to the documentation as
3c01fcc1bb9c086f487d9694cb91a57d7abd0880.  Thank you!

(I maybe should have written in the commit message “Fixes
<https://bugs.gnu.org/43746>.”  Oh well.  I will leave the bug open
though because glib is not fixed yet.)

Regards,
Florian

[-- Attachment #2: 0001-gnu-glib-Adhere-to-without-tests-option.patch --]
[-- Type: text/plain, Size: 1925 bytes --]

From 505cfd0fa3411a21c8794ab84473dc1dd2b8754c Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflorian@pelzflorian.de>
Date: Sat, 3 Oct 2020 00:29:56 +0200
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Subject: [PATCH] gnu: glib: Adhere to '--without-tests' option.

* gnu/packages/glib.scm (glib)[arguments]<#:phases>[configure]:
Only run tests if 'tests?' is true.
---
 gnu/packages/glib.scm | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index ca037d3b24..c703894155 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -13,6 +13,7 @@
 ;;; Copyright © 2019 Giacomo Leidi <goodoldpaul@autistici.org>
 ;;; Copyright © 2019, 2020 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2020 Nicolò Balzarotti <nicolo@nixo.xyz>
+;;; Copyright © 2020 Florian Pelz <pelzflorian@pelzflorian.de>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -325,11 +326,14 @@ shared NFS home directories.")
               (for-each (lambda (x) (apply disable x)) failing-tests)
               #t)))
         (replace 'check
-          (lambda _
-            (setenv "MESON_TESTTHREADS"
-                    (number->string (parallel-job-count)))
-            ;; Do not run tests marked as "flaky".
-            (invoke "meson" "test" "--no-suite" "flaky")))
+          (lambda* (#:key tests? #:allow-other-keys)
+            (if tests?
+                (begin
+                  (setenv "MESON_TESTTHREADS"
+                          (number->string (parallel-job-count)))
+                  ;; Do not run tests marked as "flaky".
+                  (invoke "meson" "test" "--no-suite" "flaky"))
+                #t)))
         ;; TODO: meson does not permit the bindir to be outside of prefix.
         ;; See https://github.com/mesonbuild/meson/issues/2561
         ;; We can remove this once meson is patched.
-- 
2.28.0

bug#43746: What to do about packages that don't support --without-tests / #:tests? #f setting

[Ludovic Courtès]

Hi,

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> On Thu, Oct 01, 2020 at 11:07:54PM +0200, Ludovic Courtès wrote:
>> Oh, we should fix ‘glib’ in ‘core-updates’.
>
> I verified the attached patch fixes glib on the ‘master’ branch.  I’m
> not sure about adding a copyright.  Anyway.  Shall I push it to
> ‘core-updates’ or wait until I can test it on ‘core-updates’ without
> rebuilding the world?

If you tested it on ‘master’, you can push it on ‘core-updates’.

> I changed it and pushed the patch to the documentation as
> 3c01fcc1bb9c086f487d9694cb91a57d7abd0880.  Thank you!

Great!

> (I maybe should have written in the commit message “Fixes
> <https://bugs.gnu.org/43746>.”  Oh well.  I will leave the bug open
> though because glib is not fixed yet.)

That’s OK.  :-)

> From 505cfd0fa3411a21c8794ab84473dc1dd2b8754c Mon Sep 17 00:00:00 2001
> From: Florian Pelz <pelzflorian@pelzflorian.de>
> Date: Sat, 3 Oct 2020 00:29:56 +0200
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> Subject: [PATCH] gnu: glib: Adhere to '--without-tests' option.
>
> * gnu/packages/glib.scm (glib)[arguments]<#:phases>[configure]:
> Only run tests if 'tests?' is true.

OK for ‘core-updates’, thank you!

Ludo’.

bug#43746: What to do about packages that don't support --without-tests / #:tests? #f setting

[pelzflorian (Florian Pelz)]

On Sat, Oct 03, 2020 at 12:04:37PM +0200, Ludovic Courtès wrote:
> > I verified the attached patch fixes glib on the ‘master’ branch.
> If you tested it on ‘master’, you can push it on ‘core-updates’.

Pushed as 0585a0d0d1fe6e334d36e2d851b42b47d6769546.  Thank you!

Closing, since the issue is documented now and fixing *all* other
check phases is generally not worth it.

Regards,
Florian