GNU bug report logs - #48319
[PATCH] Check if #:tests? is unconditionally #t; not acceptable when cross-compiling

Previous Next

Package: guix-patches;

Reported by: Maxime Devos <maximedevos <at> telenet.be>

Date: Sun, 9 May 2021 16:49:01 UTC

Severity: normal

Tags: patch

Done: Mathieu Othacehe <othacehe <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 48319 in the body.
You can then email your comments to 48319 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#48319; Package guix-patches. (Sun, 09 May 2021 16:49:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Maxime Devos <maximedevos <at> telenet.be>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Sun, 09 May 2021 16:49:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: guix-patches <at> gnu.org
Subject: [PATCH] Check if #:tests? is unconditionally #t; not acceptable
 when cross-compiling
Date: Sun, 09 May 2021 14:35:57 +0200
[Message part 1 (text/plain, inline)]
Hi guix,

This patch defines a linter detecting

  (arguments `(#:tests? #t et cetera))

in package definitions. This is unlikely to work well when
cross-compiling (some exceptions apply, e.g. take a look
at the 'fennel' package). Also, it is simply unnecessary
when compiling natively.

Fortunately, I failed to find a package that fails to cross-compile
due to this particular reason. (They failed to cross-compile due to
other reasons, or the 'check' target did nothing.)

I didn't try all packages flagged by the new linter, though.

On top of 75af43162e58a0b3fdc804963809ecb801fb81b7.

Greetings,
Maxime.
[0001-lint-tests-true-Check-if-tests-are-enabled-when-cros.patch (text/x-patch, inline)]
From daf537fe6e99b308424cb89106d254efa9ff0781 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos <at> telenet.be>
Date: Sun, 9 May 2021 14:08:12 +0200
Subject: [PATCH] lint: tests-true: Check if tests are enabled when
 cross-compiling.

* guix/lint.scm
  (check-tests-true): New linter.
  (%local-checkers)[tests-true]: Add it.
* tests/lint.scm ("tests-true: #:tests? does not need to be set to #t")
  ("tests-true: absent #:tests? is acceptable")
  ("tests-true: #:tests? #f is acceptable")
  ("tests-true: #:tests? #t acceptable when compiling natively"): Test it.
---
 guix/lint.scm  | 29 +++++++++++++++++++++++++++++
 tests/lint.scm | 23 +++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/guix/lint.scm b/guix/lint.scm
index 1bebfe03d3..d1cbc9d300 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -12,6 +12,7 @@
 ;;; Copyright © 2020 Chris Marusich <cmmarusich <at> gmail.com>
 ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
 ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
+;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -95,6 +96,7 @@
             check-archival
             check-profile-collisions
             check-haskell-stackage
+            check-tests-true
 
             lint-warning
             lint-warning?
@@ -190,6 +192,29 @@
                      #:field 'name)))
      (else '()))))
 
+(define (check-tests-true package)
+  "Check whether PACKAGE explicitly requests to run tests, which is
+superfluous when building natively and incorrect when cross-compiling."
+  (define (tests-explicitly-enabled?)
+    (apply (lambda* (#:key tests? #:allow-other-keys)
+             (eq? tests? #t))
+           (package-arguments package)))
+  (if (and (tests-explicitly-enabled?)
+           ;; Some packages, e.g. gnutls, set #:tests?
+           ;; differently depending on whether it is being
+           ;; cross-compiled.
+           (parameterize ((%current-target-system "aarch64-linux-gnu"))
+             (tests-explicitly-enabled?)))
+      ;; Actually, #:tests? *should* not be (unconditionally) set to #t,
+      ;; but that wording would suggest that tests should be disabled,
+      ;; which is not the case.
+      (list (make-warning package
+                          ;; TRANSLATORS: #:tests? and #t are Scheme constants
+                          ;; and must not be translated.
+                          (G_ "#:tests? does not need to be explicitly set to #t")
+                          #:field 'arguments))
+      '()))
+
 (define (properly-starts-sentence? s)
   (string-match "^[(\"'`[:upper:][:digit:]]" s))
 
@@ -1481,6 +1506,10 @@ them for PACKAGE."
      (name        'name)
      (description "Validate package names")
      (check       check-name))
+   (lint-checker
+     (name        'tests-true)
+     (description "Check if tests are explicitly enabled")
+     (check       check-tests-true))
    (lint-checker
      (name        'description)
      (description "Validate package descriptions")
diff --git a/tests/lint.scm b/tests/lint.scm
index a2c8665142..46830aad01 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -277,6 +277,29 @@
    (let ((pkg (dummy-package "under_score")))
      (check-name pkg))))
 
+(test-equal "tests-true: #:tests? does not need to be set to #t"
+  "#:tests? does not need to be explicitly set to #t"
+  (single-lint-warning-message
+   (let ((pkg (dummy-package "x" (arguments '(#:tests? #t)))))
+     (check-tests-true pkg))))
+
+(test-equal "tests-true: absent #:tests? is acceptable"
+  '()
+  (let ((pkg (dummy-package "x")))
+    (check-tests-true pkg)))
+
+(test-equal "tests-true: #:tests? #f is acceptable"
+  '()
+  (let ((pkg (dummy-package "x" (arguments '(#:tests? #f)))))
+    (check-tests-true pkg)))
+
+(test-equal "tests-true: #:tests? #t acceptable when compiling natively"
+  '()
+  (let ((pkg (dummy-package "x"
+                            (arguments
+                             `(#:tests? ,(not (%current-target-system)))))))
+    (check-tests-true pkg)))
+
 (test-equal "inputs: pkg-config is probably a native input"
   "'pkg-config' should probably be a native input"
   (single-lint-warning-message
-- 
2.31.1

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#48319; Package guix-patches. (Fri, 04 Jun 2021 12:52:01 GMT) Full text and rfc822 format available.

Message #8 received at 48319 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 48319 <at> debbugs.gnu.org
Subject: Re: bug#48319: [PATCH] Check if #:tests? is unconditionally #t; not
 acceptable when cross-compiling
Date: Fri, 04 Jun 2021 14:50:53 +0200
Hello Maxime,

> +                          ;; TRANSLATORS: #:tests? and #t are Scheme constants
> +                          ;; and must not be translated.
> +                          (G_ "#:tests? does not need to be explicitly set to #t")
> +                          #:field 'arguments))

This patch looks fine, and I remember fixing multiple packages failing
to cross-compile because #:tests was hard-coded to #t.

I'm not sure about the warning message though. What about:

"#:tests? must not be explicitly set to #t"

as "does not need" implies it could be sometimes correct, whereas it is
always a mistake.

WDYT?

Thanks,

Mathieu




Information forwarded to guix-patches <at> gnu.org:
bug#48319; Package guix-patches. (Sat, 05 Jun 2021 09:57:02 GMT) Full text and rfc822 format available.

Message #11 received at 48319 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Mathieu Othacehe <othacehe <at> gnu.org>
Cc: 48319 <at> debbugs.gnu.org
Subject: Re: bug#48319: [PATCH] Check if #:tests? is unconditionally #t; not
 acceptable when cross-compiling
Date: Sat, 05 Jun 2021 11:56:25 +0200
[Message part 1 (text/plain, inline)]
Mathieu Othacehe schreef op vr 04-06-2021 om 14:50 [+0200]:
> Hello Maxime,
> 
> > +                          ;; TRANSLATORS: #:tests? and #t are Scheme constants
> > +                          ;; and must not be translated.
> > +                          (G_ "#:tests? does not need to be explicitly set to #t")
> > +                          #:field 'arguments))
> 
> This patch looks fine, and I remember fixing multiple packages failing
> to cross-compile because #:tests was hard-coded to #t.
> 
> I'm not sure about the warning message though. What about:
> 
> "#:tests? must not be explicitly set to #t"
> 
> as "does not need" implies it could be sometimes correct, whereas it is
> always a mistake.

The new warning message "#:tests? must not be explicitly set to #t"
seems reasonable to me. I'm currently working on other things though
(guix and other things), so don't expect a revised patch soon.

Greetings,
Maxime.
[signature.asc (application/pgp-signature, inline)]

Reply sent to Mathieu Othacehe <othacehe <at> gnu.org>:
You have taken responsibility. (Sun, 06 Jun 2021 16:52:02 GMT) Full text and rfc822 format available.

Notification sent to Maxime Devos <maximedevos <at> telenet.be>:
bug acknowledged by developer. (Sun, 06 Jun 2021 16:52:02 GMT) Full text and rfc822 format available.

Message #16 received at 48319-done <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 48319-done <at> debbugs.gnu.org
Subject: Re: bug#48319: [PATCH] Check if #:tests? is unconditionally #t; not
 acceptable when cross-compiling
Date: Sun, 06 Jun 2021 18:51:17 +0200
Hey,

> The new warning message "#:tests? must not be explicitly set to #t"
> seems reasonable to me. I'm currently working on other things though
> (guix and other things), so don't expect a revised patch soon.

I edited the message, removed the associated comment and fixed the
relevant test accordingly. Pushed as
82b0e27de109b38ed44f67434a96460c4a7f9217.

Thanks,

Mathieu




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 05 Jul 2021 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 267 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.