unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / Atom feed
* bug#50299: The check-tests-true lint check is incorrect for Emacs packages
@ 2021-08-31 15:25 Maxim Cournoyer
  2021-09-07 12:34 ` bug#50299: lint: check-tests-true: Allow #:tests? #t for emacs-build-system Maxime Devos
  0 siblings, 1 reply; 3+ messages in thread
From: Maxim Cournoyer @ 2021-08-31 15:25 UTC (permalink / raw)
  To: 50299

Hi,

The emacs-build-system has its #:tests? argument set to #f by default,
which means users must explicitly specify its value as #t to enable test
suites.

The check-tests-true lint check seems to ignore this and goes on to
recommend, for example:

--8<---------------cut here---------------start------------->8---
gnu/packages/emacs-xyz.scm:15998:7: emacs-groovy-modes@2.0-0.99eaf70: #:tests? must not be explicitly set to #t
--8<---------------cut here---------------end--------------->8---

which is wrong.

Thanks,

Maxim




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

* bug#50299: lint: check-tests-true: Allow #:tests? #t for emacs-build-system.
  2021-08-31 15:25 bug#50299: The check-tests-true lint check is incorrect for Emacs packages Maxim Cournoyer
@ 2021-09-07 12:34 ` Maxime Devos
  2021-09-23  3:41   ` bug#50299: The check-tests-true lint check is incorrect for Emacs packages Maxim Cournoyer
  0 siblings, 1 reply; 3+ messages in thread
From: Maxime Devos @ 2021-09-07 12:34 UTC (permalink / raw)
  To: 50299


[-- Attachment #1.1: Type: text/plain, Size: 66 bytes --]

Hi

The attached patch should fix this.

Greetings,
Maxime.

[-- Attachment #1.2: 0001-lint-check-tests-true-Allow-tests-t-for-emacs-build-.patch --]
[-- Type: text/x-patch, Size: 2701 bytes --]

From c8efd59560509381920cbfe915279bd16077552d Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Tue, 7 Sep 2021 14:25:43 +0200
Subject: [PATCH] lint: check-tests-true: Allow #:tests? #t for
 emacs-build-system.

emacs-build-system sets #:tests? #f by default, so the linter
shouldn't warn if #:tests? #t is set for packages using
emacs-build-system.

* guix/lint.scm (check-tests-true): Do not warn if the build system
  is emacs-build-system.

Fixes: <https://issues.guix.gnu.org/50299>
Reported-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 guix/lint.scm  |  5 +++++
 tests/lint.scm | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/guix/lint.scm b/guix/lint.scm
index 8e80aae938..f708465ed8 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -34,6 +34,7 @@
   #:use-module (guix store)
   #:autoload   (guix base16) (bytevector->base16-string)
   #:use-module (guix base32)
+  #:use-module (guix build-system emacs)
   #:use-module (guix diagnostics)
   #:use-module (guix download)
   #:use-module (guix ftp-client)
@@ -279,6 +280,10 @@ superfluous when building natively and incorrect when cross-compiling."
              (eq? tests? #t))
            (package-arguments package)))
   (if (and (tests-explicitly-enabled?)
+           ;; emacs-build-system sets #:tests? #f by default, therefore
+           ;; writing #:tests? #t in package definitions using
+           ;; emacs-build-system is reasonable.
+           (not (eq? emacs-build-system (package-build-system package)))
            ;; Some packages, e.g. gnutls, set #:tests?
            ;; differently depending on whether it is being
            ;; cross-compiled.
diff --git a/tests/lint.scm b/tests/lint.scm
index 0f51b9ef79..6aa2ac00c1 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -35,6 +35,7 @@
   #:use-module (guix tests http)
   #:use-module (guix download)
   #:use-module (guix git-download)
+  #:use-module (guix build-system emacs)
   #:use-module (guix build-system gnu)
   #:use-module (guix packages)
   #:use-module (guix lint)
@@ -324,6 +325,15 @@
                              `(#:tests? ,(not (%current-target-system)))))))
     (check-tests-true pkg)))
 
+;; The emacs-build-system sets #:tests? #f by default.
+(test-equal "tests-true: #:tests? #t acceptable for emacs packages"
+  '()
+  (let ((pkg (dummy-package "x"
+                            (build-system emacs-build-system)
+                            (arguments
+                             `(#:tests? #t)))))
+    (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.33.0


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

* bug#50299: The check-tests-true lint check is incorrect for Emacs packages
  2021-09-07 12:34 ` bug#50299: lint: check-tests-true: Allow #:tests? #t for emacs-build-system Maxime Devos
@ 2021-09-23  3:41   ` Maxim Cournoyer
  0 siblings, 0 replies; 3+ messages in thread
From: Maxim Cournoyer @ 2021-09-23  3:41 UTC (permalink / raw)
  To: Maxime Devos; +Cc: 50299

Hello Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

> Hi
>
> The attached patch should fix this.
>
> Greetings,
> Maxime.
>
> From c8efd59560509381920cbfe915279bd16077552d Mon Sep 17 00:00:00 2001
> From: Maxime Devos <maximedevos@telenet.be>
> Date: Tue, 7 Sep 2021 14:25:43 +0200
> Subject: [PATCH] lint: check-tests-true: Allow #:tests? #t for
>  emacs-build-system.
>
> emacs-build-system sets #:tests? #f by default, so the linter
> shouldn't warn if #:tests? #t is set for packages using
> emacs-build-system.
>
> * guix/lint.scm (check-tests-true): Do not warn if the build system
>   is emacs-build-system.
>
> Fixes: <https://issues.guix.gnu.org/50299>
> Reported-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> ---
>  guix/lint.scm  |  5 +++++
>  tests/lint.scm | 10 ++++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/guix/lint.scm b/guix/lint.scm
> index 8e80aae938..f708465ed8 100644
> --- a/guix/lint.scm
> +++ b/guix/lint.scm
> @@ -34,6 +34,7 @@
>    #:use-module (guix store)
>    #:autoload   (guix base16) (bytevector->base16-string)
>    #:use-module (guix base32)
> +  #:use-module (guix build-system emacs)
>    #:use-module (guix diagnostics)
>    #:use-module (guix download)
>    #:use-module (guix ftp-client)
> @@ -279,6 +280,10 @@ superfluous when building natively and incorrect when cross-compiling."
>               (eq? tests? #t))
>             (package-arguments package)))
>    (if (and (tests-explicitly-enabled?)
> +           ;; emacs-build-system sets #:tests? #f by default, therefore
> +           ;; writing #:tests? #t in package definitions using
> +           ;; emacs-build-system is reasonable.
> +           (not (eq? emacs-build-system (package-build-system package)))
>             ;; Some packages, e.g. gnutls, set #:tests?
>             ;; differently depending on whether it is being
>             ;; cross-compiled.

Grepping for (tests? #f), I found texlive-build also defaults to #f, so
should be treated the same.

OK to push with such a change.

Thank you!

Maxim




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

end of thread, other threads:[~2021-09-23  3:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-31 15:25 bug#50299: The check-tests-true lint check is incorrect for Emacs packages Maxim Cournoyer
2021-09-07 12:34 ` bug#50299: lint: check-tests-true: Allow #:tests? #t for emacs-build-system Maxime Devos
2021-09-23  3:41   ` bug#50299: The check-tests-true lint check is incorrect for Emacs packages Maxim Cournoyer

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 NNTP newsgroup(s).