GNU bug report logs - #65426
[PATCH] lint: Check that (cc-for-target) and friends are used.

Previous Next

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

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

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
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH] lint: Check that (cc-for-target) and friends are used.
Date: Mon, 21 Aug 2023 15:07:20 +0200

"CC=gcc" is almost always incorrect; people often just don't
notice the incorrectness because they are compiling natively.
I don't know of any packages in Guix where "CC=gcc" is correct.

"guix style" partially made things worse, so I partially ignored it.

* guix/lint.scm (check-compiler-for-target): New linter.
* tests/lint.scm
("compiler-for-target: unconditional CC=gcc is unacceptable")
("compiler-for-target: looks through G-expressions")
("compiler-for-target: (cc-for-target) is acceptable"): Test it.

(Remove before applying)
I was cross-compiling something (libical), which failed, so I wrote a
linter to prevent a class of cross-compilation problems.  I
accidentally tested for some other class of cross-compilation
problems, but whatever, this is still useful.

p.s.: Does anyone know any Git documentation in the scissors stuff?
---
 guix/lint.scm  | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 tests/lint.scm | 23 ++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/guix/lint.scm b/guix/lint.scm
index d173563e51..a517b96a88 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -12,7 +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, 2022 Maxime Devos <maximedevos <at> telenet.be>
+;;; Copyright © 2021-2023 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -114,6 +114,7 @@ (define-module (guix lint)
             check-profile-collisions
             check-haskell-stackage
             check-tests-true
+            check-compiler-for-target
 
             lint-warning
             lint-warning?
@@ -311,6 +312,49 @@ (define (check-tests-true package)
                           #:field 'arguments))
       '()))
 
+(define (check-compiler-for-target package)
+  "Check that cross-compilers are used when cross-compiling, by inspecting
+#:make-flags."
+  (define (make-compiler-warning variable=value)
+    (define =-index (string-index variable=value #\=))
+    (define variable (substring variable=value 0 =-index))
+    (define value (substring variable=value (+ =-index 1)))
+    (make-warning package
+                  (G_ "'~0@*~a' should be set to '~1@*~a' instead of '~2@*~a'")
+                  (list variable
+                        (match variable
+                          ("AR" "(ar-for-target)")
+                          ("AS" "(as-for-target)")
+                          ("CC" "(cc-for-target)")
+                          ("CXX" "(cxx-for-target)")
+                          ("LD" "(ld-for-target)")
+                          ("PKG_CONFIG" "(pkg-config-for-target)"))
+                        value)
+                  #:field 'arguments))
+  (define (find-incorrect-compilers l)
+    (match l
+      ((or "AR=ar"
+           "AS=as"
+           ;; 'cc' doesn't actually exist in Guix, but if it did,
+           ;; it would be incorrect to use it w.r.t. cross-compilation.
+           "CC=cc" "CC=gcc" "CC=clang"
+           "CXX=g++"
+           "LD=ld"
+                "PKG_CONFIG=pkg-config")
+       (list (make-compiler-warning l)))
+      ((x . y)
+       (append (find-incorrect-compilers x)
+               (find-incorrect-compilers y)))
+      (_ '())))
+  (parameterize ((%current-target-system "aarch64-linux-gnu"))
+    (apply (lambda* (#:key make-flags #:allow-other-keys)
+             (define make-flags/sexp
+               (if (gexp? make-flags/sexp)
+                   (gexp->approximate-sexp make-flags)
+                   make-flags))
+             (find-incorrect-compilers make-flags/sexp))
+           (package-arguments package))))
+
 (define (properly-starts-sentence? s)
   (string-match "^[(\"'`[:upper:][:digit:]]" s))
 
@@ -1864,6 +1908,10 @@ (define %local-checkers
      (name        'tests-true)
      (description "Check if tests are explicitly enabled")
      (check       check-tests-true))
+   (lint-checker
+     (name        'compiler-for-target)
+     (description "Check that cross-compilers are used when cross-compiling")
+     (check       check-compiler-for-target))
    (lint-checker
      (name        'description)
      (description "Validate package descriptions")
diff --git a/tests/lint.scm b/tests/lint.scm
index b91bd053c5..45359d3783 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -10,7 +10,7 @@
 ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
-;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
+;;; Copyright © 2021, 2023 Maxime Devos <maximedevos <at> telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -342,6 +342,27 @@ (define (warning-contains? str warnings)
                              `(#:tests? ,(not (%current-target-system)))))))
     (check-tests-true pkg)))
 
+(test-equal "compiler-for-target: unconditional CC=gcc is unacceptable"
+  "'CC' should be set to '(cc-for-target)' instead of 'gcc'"
+  (single-lint-warning-message
+   (check-compiler-for-target
+    (dummy-package "x" (arguments '(#:make-flags '("CC=gcc")))))))
+
+
+(test-equal "compiler-for-target: looks through G-expressions"
+  "'CC' should be set to '(cc-for-target)' instead of 'gcc'"
+  (single-lint-warning-message
+   (check-compiler-for-target
+    (dummy-package "x" (arguments '(#:make-flags #~'("CC=gcc")))))))
+
+(test-equal "compiler-for-target: (cc-for-target) is acceptable"
+  '()
+  (check-compiler-for-target
+   (dummy-package "x"
+		  (arguments
+		   (list #:make-flags
+			 #~(list (string-append "CC=" (cc-for-target))))))))
+
 ;; The emacs-build-system sets #:tests? #f by default.
 (test-equal "tests-true: #:tests? #t acceptable for emacs packages"
   '()

base-commit: 707682ac75a81f41a478c2c51672ca49b98fa6eb
prerequisite-patch-id: b6e78e4ce45fc555f83ef70ba4b158046f750dc3
-- 
2.41.0

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

From: Maxime Devos <maximedevos <at> telenet.be>
To: 65426 <at> debbugs.gnu.org
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH v2] lint: Check that (cc-for-target) and friends are used.
Date: Mon, 21 Aug 2023 15:59:53 +0200

"CC=gcc" is almost always incorrect; people often just don't
notice the incorrectness because they are compiling natively.
For an exception, see tzdata.

"guix style" partially made things worse, so I partially ignored it.

* guix/lint.scm (check-compiler-for-target): New linter.
* tests/lint.scm
("compiler-for-target: unconditional CC=gcc is unacceptable")
("compiler-for-target: looks through G-expressions")
("compiler-for-target: (cc-for-target) is acceptable")
("compiler-for-target: CC=gcc is acceptable when target=#false"):
Test it.

(Remove before applying)
I forgot the 'tzdata' case in the previous version, even though I
added the #:target #false myself IIRC ...  I also fixed a spacing
error.
---
 guix/lint.scm  | 56 +++++++++++++++++++++++++++++++++++++++++++++++++-
 tests/lint.scm | 32 ++++++++++++++++++++++++++++-
 2 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/guix/lint.scm b/guix/lint.scm
index d173563e51..7ccf52dec1 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -12,7 +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, 2022 Maxime Devos <maximedevos <at> telenet.be>
+;;; Copyright © 2021-2023 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brice Waegeneire <brice <at> waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -114,6 +114,7 @@ (define-module (guix lint)
             check-profile-collisions
             check-haskell-stackage
             check-tests-true
+            check-compiler-for-target
 
             lint-warning
             lint-warning?
@@ -311,6 +312,55 @@ (define (check-tests-true package)
                           #:field 'arguments))
       '()))
 
+(define (check-compiler-for-target package)
+  "Check that cross-compilers are used when cross-compiling, by inspecting
+#:make-flags."
+  (define (make-compiler-warning variable=value)
+    (define =-index (string-index variable=value #\=))
+    (define variable (substring variable=value 0 =-index))
+    (define value (substring variable=value (+ =-index 1)))
+    (make-warning package
+                  (G_ "'~0@*~a' should be set to '~1@*~a' instead of '~2@*~a'")
+                  (list variable
+                        (match variable
+                          ("AR" "(ar-for-target)")
+                          ("AS" "(as-for-target)")
+                          ("CC" "(cc-for-target)")
+                          ("CXX" "(cxx-for-target)")
+                          ("LD" "(ld-for-target)")
+                          ("PKG_CONFIG" "(pkg-config-for-target)"))
+                        value)
+                  #:field 'arguments))
+  (define (find-incorrect-compilers l)
+    (match l
+      ((or "AR=ar"
+           "AS=as"
+           ;; 'cc' doesn't actually exist in Guix, but if it did,
+           ;; it would be incorrect to use it w.r.t. cross-compilation.
+           "CC=cc" "CC=gcc" "CC=clang"
+           "CXX=g++"
+           "LD=ld"
+           "PKG_CONFIG=pkg-config")
+       (list (make-compiler-warning l)))
+      ((x . y)
+       (append (find-incorrect-compilers x)
+               (find-incorrect-compilers y)))
+      (_ '())))
+  (parameterize ((%current-target-system "aarch64-linux-gnu"))
+    (apply (lambda* (#:key (target 'not-set)
+		     make-flags #:allow-other-keys)
+             (define make-flags/sexp
+               (if (gexp? make-flags/sexp)
+                   (gexp->approximate-sexp make-flags)
+                   make-flags))
+	     ;; Some packages like 'tzdata' are never cross-compiled;
+	     ;; the compilers are only used to build tools for
+	     ;; compiling the rest of the package.
+	     (if (eq? target '#false)
+		 '()
+		 (find-incorrect-compilers make-flags/sexp)))
+           (package-arguments package))))
+
 (define (properly-starts-sentence? s)
   (string-match "^[(\"'`[:upper:][:digit:]]" s))
 
@@ -1864,6 +1914,10 @@ (define %local-checkers
      (name        'tests-true)
      (description "Check if tests are explicitly enabled")
      (check       check-tests-true))
+   (lint-checker
+     (name        'compiler-for-target)
+     (description "Check that cross-compilers are used when cross-compiling")
+     (check       check-compiler-for-target))
    (lint-checker
      (name        'description)
      (description "Validate package descriptions")
diff --git a/tests/lint.scm b/tests/lint.scm
index b91bd053c5..a52a82237b 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -10,7 +10,7 @@
 ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
-;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
+;;; Copyright © 2021, 2023 Maxime Devos <maximedevos <at> telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -342,6 +342,36 @@ (define (warning-contains? str warnings)
                              `(#:tests? ,(not (%current-target-system)))))))
     (check-tests-true pkg)))
 
+(test-equal "compiler-for-target: unconditional CC=gcc is unacceptable"
+  "'CC' should be set to '(cc-for-target)' instead of 'gcc'"
+  (single-lint-warning-message
+   (check-compiler-for-target
+    (dummy-package "x" (arguments '(#:make-flags '("CC=gcc")))))))
+
+
+(test-equal "compiler-for-target: looks through G-expressions"
+  "'CC' should be set to '(cc-for-target)' instead of 'gcc'"
+  (single-lint-warning-message
+   (check-compiler-for-target
+    (dummy-package "x" (arguments '(#:make-flags #~'("CC=gcc")))))))
+
+(test-equal "compiler-for-target: (cc-for-target) is acceptable"
+  '()
+  (check-compiler-for-target
+   (dummy-package "x"
+		  (arguments
+		   (list #:make-flags
+			 #~(list (string-append "CC=" (cc-for-target))))))))
+
+(test-equal "compiler-for-target: CC=gcc is acceptable when target=#false"
+  '()
+  (check-compiler-for-target
+   ;; This (dummy) package consists purely of architecture-independent data.
+   (dummy-package "tzdata"
+		  (arguments
+		   (list #:target #false
+			 #:make-flags #~(list "CC=gcc"))))))
+
 ;; The emacs-build-system sets #:tests? #f by default.
 (test-equal "tests-true: #:tests? #t acceptable for emacs packages"
   '()

base-commit: 707682ac75a81f41a478c2c51672ca49b98fa6eb
prerequisite-patch-id: b6e78e4ce45fc555f83ef70ba4b158046f750dc3
-- 
2.41.0

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 65426-done <at> debbugs.gnu.org, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#65426] [PATCH v2] lint: Check that (cc-for-target) and
 friends are used.
Date: Fri, 08 Sep 2023 18:36:43 +0200

Hi Maxime,

Maxime Devos <maximedevos <at> telenet.be> skribis:

> "CC=gcc" is almost always incorrect; people often just don't
> notice the incorrectness because they are compiling natively.
> For an exception, see tzdata.
>
> "guix style" partially made things worse, so I partially ignored it.
>
> * guix/lint.scm (check-compiler-for-target): New linter.
> * tests/lint.scm
> ("compiler-for-target: unconditional CC=gcc is unacceptable")
> ("compiler-for-target: looks through G-expressions")
> ("compiler-for-target: (cc-for-target) is acceptable")
> ("compiler-for-target: CC=gcc is acceptable when target=#false"):
> Test it.

Finally applied, thanks!  It’s a much welcome improvement.

Ludo’.

Previous Next

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