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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Maxime Devos <maximedevos <at> telenet.be>
Subject: bug#48319: closed (Re: bug#48319: [PATCH] Check if #:tests? is
 unconditionally #t; not acceptable when cross-compiling)
Date: Sun, 06 Jun 2021 16:52:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

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

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 48319 <at> debbugs.gnu.org.

-- 
48319: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=48319
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

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

[Message part 3 (message/rfc822, inline)]

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 4 (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)]

Previous Next

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