GNU bug report logs -
#48319
[PATCH] Check if #:tests? is unconditionally #t; not acceptable when cross-compiling
Previous Next
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.
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):
[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):
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):
[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):
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.