Guile not in native-inputs when it should

  • Open
  • quality assurance status badge
Details
One participant
  • Maxime Devos
Owner
unassigned
Submitted by
Maxime Devos
Severity
normal
Merged with
M
M
Maxime Devos wrote on 17 Mar 2021 22:58
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . bug-guix@gnu.org)
c34c473b38b2c659947d684d357a31a2a3ece480.camel@telenet.be
Hi Guix,

(In response to bug#47027, but opened as a new bug.)

On Wed, 2021-03-17 at 21:52 +0100, Ludovic Courtès wrote:
Toggle quote (9 lines)
> Hi,
>
> Maxime Devos <maximedevos@telenet.be> skribis:
> [...]
> > Shouldn't the "guile" input be included in the native-inputs
> > as well (perhaps only native-inputs suffices), for cross-compilation?
>
> Yes it should, good point.

FWIW, I tried to write a linter to catch these kind of issues.
(If there's a "guile" input, then there usually should also be
a "guile" native-input.) Currently, it has too many false positives
for my taste. I most likely won't be working on it in the near future
though. (Preliminary patch attached)

Toggle quote (2 lines)
> ./pre-int-env guix lint -t "check-inputs-should-also-be-native"

(Output attached)

Some suspicious things:
* guile-config & others are missing a "guile" in the native-inputs
* clipmenu & others use "wrap-script" to define wrapper scripts
(in this case "guile" does not have to be in native-inputs).
The "wrap-script" procedure from (guix build utils) uses the
"which" procedure to determine where guile is located ...
but this is incorrect when cross-compiling!

(It is possible to override the "guile" binary used with a
keyword argument).

(I assume inputs in "inputs" do not contribute to the $PATH
in a cross-compilation environment; only "native-inputs" should
contribute to $PATH)

idk if it is feasible or if there are complications, but
IMHO the inputs in "inputs" shouldn't contribute to $PATH
at all (not even when not cross-compilation), only inputs
in $PATH.

There seems to be plenty of low-hanging cross-compilation fruit here!

Greetings,
Maxime
From c4798e6154275a2de41c1d5a35bc723091d4e1a4 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Wed, 17 Mar 2021 22:56:26 +0100
Subject: [PATCH] lint: Check whether guile should be in native-inputs.

TODO less false positives (or negatives?)
TODO proper message

* guix/lint.scm
(check-inputs-should-also-be-native): ???
(%local-checkers)[inputs-should-also-be-native]: New ???.
---
guix/lint.scm | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

Toggle diff (70 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 311bc94cc3..d0cde23665 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -11,6 +11,7 @@
;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net>
;;; Copyright © 2020 Chris Marusich <cmmarusich@gmail.com>
;;; Copyright © 2020 Timothy Sample <samplet@ngyro.com>
+;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -75,6 +76,7 @@
#:use-module (ice-9 rdelim)
#:export (check-description-style
check-inputs-should-be-native
+ check-inputs-should-also-be-native
check-inputs-should-not-be-an-input-at-all
check-patch-file-names
check-patch-headers
@@ -347,6 +349,36 @@ of a package, and INPUT-NAMES, a list of package specifications such as
#:field 'inputs))
(package-input-intersection inputs input-names))))
+#|
+(define (suspect-input->native-names package)
+ ;; Guile's compiled .go code is architecture
+ `(,@(if (string-prefix? "guile" (package-name package))
+ '("guile")
+ '()))
+|#
+
+(define (check-inputs-should-also-be-native package)
+ ;; Emit a warning if some inputs of PACKAGE are likely to belong to its
+ ;; native inputs as well.
+ (guard (c ((package-cross-build-system-error? c) '()))
+ (let ((inputs (package-inputs package))
+ (native-inputs
+ ;; Pretend we're cross-compiling,
+ ;; as some packages only add the "guile" input
+ ;; to native-inputs when %current-target-system is not #f.
+ (parameterize ((%current-target-system (%current-system)))
+ (package-native-inputs package)))
+ (input-names
+ '("guile")))
+ (filter-map (lambda (input)
+ (and (not (assoc input native-inputs))
+ (make-warning
+ package
+ (G_ "'~a' should probably also be a native input")
+ (list input)
+ #:field 'inputs)))
+ (package-input-intersection inputs input-names)))))
+
(define (check-inputs-should-not-be-an-input-at-all package)
;; Emit a warning if some inputs of PACKAGE are likely to should not be
;; an input at all.
@@ -1449,6 +1481,10 @@ them for PACKAGE."
(name 'description)
(description "Validate package descriptions")
(check check-description-style))
+ (lint-checker
+ (name 'inputs-should-also-be-native)
+ (description "Identify inputs that should aso be native inputs")
+ (check check-inputs-should-also-be-native))
(lint-checker
(name 'inputs-should-be-native)
(description "Identify inputs that should be native inputs")
--
2.30.2
-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFJ7oRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7m1rAP917fx/VFi8xzRiljXHnnQmwd7V
zIkd/0w5QtQlzVho6QD/QJNX30d5V1UlDDyf4n7fdOcgOkeoC4+hVQFSdQNmAAM=
=6JPn
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 18 Mar 2021 08:18
(address . 47221@debbugs.gnu.org)
5197a04d52addefcf4e01f2a15f4657e5b9d4658.camel@telenet.be
I made a spelling error in the command:
./pre-inst-env guix lint -c "inputs-should-also-be-native"

I forgot to attach the output of "guix lint -c ..." (now attached).
-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFL+zhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7v81AQDgEzbBMtma1Ibp/753bg90fEHQ
uD034CNR7MMZha2krQD/ZkeDEIKg692h5LB8rhTQZWvbAv5iS3ADka3AnjaAIAc=
=02GI
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 18 Mar 2021 10:29
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47221@debbugs.gnu.org)
d62b3877cce0f3a6c16c62a7047ac70e5fc790d6.camel@telenet.be
On Wed, 2021-03-17 at 22:58 +0100, Maxime Devos wrote:
Toggle quote (10 lines)
> [...]
> Some suspicious things:
> * [...]
> * clipmenu & others use "wrap-script" to define wrapper scripts

> (in this case "guile" does not have to be in native-inputs).
> The "wrap-script" procedure from (guix build utils) uses the
> "which" procedure to determine where guile is located ...
> but this is incorrect when cross-compiling!

Demonstration (host system: x86-64-linux with a childhurd running, without qemu binfmt),
using the "bats" package (the "bats" package is choosen because it doesn't have many
dependencies and it uses wrap-script). ("bats" actually uses wrap-script correctly,
so first remove the following line
":" (assoc-ref %build-inputs "guile") "/bin"
from the package definition to simulate a misbehaving package)

./pre-inst-env guix build --system=i586-gnu --target=x86-64-linux bats
--> tcl fails to build with plenty of failing test cases
^ TODO submit a bug report, for now try without tests

./pre-inst-env guix build --system=i586-gnu --target=x86-64-linux bats --without-tests=tcl
^ TODO this hangs the childhurd (something about paging?)

./pre-inst-env guix build --target=aarch64-linux bats
(warning: this takes some time building the cross-compiler)
--> install.sh: line 15: /gnu/store/...-coreutils-8.32/bin/install: cannot execute binary file: Exec format error

After adding "coreutils" to the native-inputs:

./pre-inst-env guix build --target=aarch64-linux bats
(success! --> some /gnu/store/something path $STORE_ITEM)

Let's look at $STORE_ITEM/bin/bats:

(start snip)
#!#f --no-auto-compile
#!#; Guix wrapper
#\-(begin (let ((current (getenv "PATH"))) (setenv "PATH" (if current (string-append "/gnu/store/qrj2w7a8ms7rkyvqhnrv8wrvqnbwv9bm-bash-
5.0.16/bin:/gnu/store/n8awazyldv9hbzb7pjcw76hiifmvrpvd-coreutils-8.32/bin:/gnu/store/3xi5vprn92r0jcb03lk9ykind5pi789j-grep-3.4/bin:/path-
not-set" ":" current) "/gnu/store/qrj2w7a8ms7rkyvqhnrv8wrvqnbwv9bm-bash-5.0.16/bin:/gnu/store/n8awazyldv9hbzb7pjcw76hiifmvrpvd-coreutils-
8.32/bin:/gnu/store/3xi5vprn92r0jcb03lk9ykind5pi789j-grep-3.4/bin:/path-not-set"))))
#\-(let ((cl (command-line))) (apply execl "/gnu/store/qrj2w7a8ms7rkyvqhnrv8wrvqnbwv9bm-bash-5.0.16/bin/bash" (car cl) (cons (car cl)
(append (quote ("")) cl))))
#!/gnu/store/qrj2w7a8ms7rkyvqhnrv8wrvqnbwv9bm-bash-5.0.16/bin/bash

set -e

BATS_READLINK='true'
[...]
(end snip)

I was worried for a moment that the inputs in "inputs" would contribute to
$PATH even when cross-compiling, but this turns out not to be the case.
However, I believe "wrap-script" should raise some kind of exception
instead of trying to use "#f" as interpreter.

--
Btw., "wrap-program" also uses "which" (but for finding the shell),
but fixing that would entail a world-rebuild as "wrap-program" doesn't
have a keyword argument

Toggle quote (2 lines)
> Greetings,
> Maxime
-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFMdmBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7kOiAP92XH51udUvXGF9KDMzyI7/dkaq
44I9spmKT0sq1mQ5XQEAzuJ+fa/ivVNmJY67j76x+QhzC+v1OYIUgvnJ1Yz0DgA=
=HWeS
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 18 Mar 2021 15:01
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47221@debbugs.gnu.org)
81fb51c76013333b8080893491ce298ed55e93f5.camel@telenet.be
This fixes some uses of wrap-script.
Attachment: file
-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFNdVxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7krkAPoD00QpDd1VX333nQMJVml1E7K/
VscmwEfN4ys8/C9ZQAEApSSZEIxafgo8sk4Y3s7BjRzISMQnR5SSIfhpK/hC6Q8=
=NiwU
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 18 Mar 2021 20:08
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47221@debbugs.gnu.org)
a74a539698867b44c5f99e951220453cac9adf9b.camel@telenet.be
On Thu, 2021-03-18 at 15:01 +0100, Maxime Devos wrote:
Toggle quote (2 lines)
> This fixes some uses of wrap-script.

There's a bug in the patch (a missing #:input argument),
so don't apply yet. Will be fixed in the next revision
(along with more cross-compilation fixes).
-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFOlDxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7k3wAP9KLeHEaJF8mh0NXUP/k12c/sxD
d33QO6Vi3JSGBAO0cQEAybgYYfJdqHR+wV9+R8LNu2ZGo7Arp5MNLalQrRs4hAo=
=wmkR
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 20 Mar 2021 22:45
[PATCH v2]: Correct some inputs / native-inputs issues with guile
(address . 47221@debbugs.gnu.org)
b5f76f81dab79fa42fd8584ca2279a0a6b5299db.camel@telenet.be
Hi Guix,

The first patch ‘gnu: Explicitely pass the guile binary to wrap-script.’
does what it say what it does. This is important for picking up the guile
binary for the architecture in --target instead of the guile from native-inputs.

(The unpatched & patched package definitions do not have a guile native-input,
so before this patch they wouldn't pick up a guile at all -- packages in inputs
do not contribute to the $PATH when cross-compiling.)

The second patch ‘gnu: Make guile packages cross-compilable when the fix is trivial.’
only touches guile libraries. It adds guile to the native-inputs when required,
sometimes it adds guile to inputs and sometimes it copies propagated-inputs & inputs
to native-inputs when cross-compiling. It also fixes some other cross-compilations
issues like autoconf being in inputs instead of native-inputs.

The second patch only touches gnu/packages/guile-xyz.scm; other packages are ignored.
Also ignored are: emacsy-minimal (there have been patches and/or bug reports lately)
guile-bash (retired project) and guile-studio (it is an Emacs package).

Suggested testing method:

(start shell script)
# BAD_PACKAGES: fails to compile
BAD_PACKAGES="guile2.0-commonmark guile3.0-ncurses-with-gpm guile-dbi guile2.2-pfds"

# OK_PACKAGES: compiles & cross-compiles
OK_PACKAGES="guildhall guile-daemon guile-dsv guile-filesystem guile2.0-filesystem guile2.2-filesystem guile-syntax-highlight guile2.2-
syntax-highlight guile-sjson guile-sparql guile-email guile-mastodon guile-parted guile2.2-parted guile-config guile-hall guile-wisp
guile-commonmark guile-stis-parser guile-persist guile-file-names guile-srfi-158 guile-semver jupyter-guile-kernel guile-ics srfi-64-
driver guile-websocket g-wrap guile-webutils guile-srfi-159 guile-torrent guile-irc guile-machine-code guile2.2-sjson guile2.2-dsv
guile2.2-email guile2.2-config guile2.2-hall guile2.2-ics guile2.2-wisp guile2.2-commonmark guile2.2-semver guile2.2-webutils guile-redis
guile2.2-redis guile2.0-redis guile-irregex guile2.0-irregex guile2.2-irregex mcron guile2.2-mcron guile-srfi-89 guile-srfi-145 guile-
srfi-180 guile-jpeg guile-hashing guile2.2-hashing guile-packrat guile-ac-d-bus guile-lens guile2.2-lens guile-rdf guile-jsonld guile-
struct-pack guile-laesare guile-mkdir-p guile-jwt guile-r6rs-protobuf guile-shapefile schmutz guile-cbor guile-8sync guile-squee guile2.2-
squee guile-colorized guile2.2-colorized guile-pfds guile-prometheus guile-aa-tree guile-simple-zmq guile2.2-simple-zmq guile-debbugs
guile-email-latest guile-miniadapton guile-lib guile2.0-lib guile2.2-lib guile-minikanren guile2.0-minikanren guile2.2-minikanren python-
on-guile"

# NOT_CROSS_COMPILABLE: self-describing (compiles natively)
NOT_CROSS_COMPILABLE="guile-cv guile-gi guile-ncurses g-golf guile-picture-language guile-sly guile-aspell guile-fibers guile-sodium
guile-reader guile-udev haunt guile2.2-haunt guile2.0-haunt guile2.2-ncurses guile-ncurses-with-gpm guile-xosd artanis guile-xapian
guile2.2-xapian guile-newt guile2.2-newt guile2.2-reader guile-avahi guile2.0-pg guile-libyaml guile-eris guile-ffi-fftw "

make
# replace aarch64 with architecture of choice and maybe adjust -M and -c
./pre-inst-env guix build $OK_PACKAGES $NOT_CROSS_COMPILABLE -M6 -c1 --fallback
./pre-inst-env guix build $OK_PACKAGES -M6 -c1 --target=aarch64-linux-gnu --fallback
make as-derivation
(end shell script)

Greetings,
Maxime
Attachment: file
Attachment: file
-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFZtCxccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7gRJAP9YZ6sjJ+93F/bLz4z2VlLh3Hs+
azlObB3BmOCh9Aj81gD9FxOB0gjLfwlbyWIlMdt6v1i0Wsh5RoUNUR/JqitxWwU=
=79Ym
-----END PGP SIGNATURE-----


M
?