[PATCH 0/2] Avoid Bash wrapper in 'guix' package

DoneSubmitted by Ludovic Courtès.
Details
3 participants
  • Ludovic Courtès
  • Maxime Devos
  • Vagrant Cascadian
Owner
unassigned
Severity
normal
L
L
Ludovic Courtès wrote on 4 May 15:25 +0200
(address . guix-patches@gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20210504132542.10540-1-ludo@gnu.org
Hello!
The first patch here addresses a locale warning coming from Bashthat I noticed while using 1.3.0rc1 on Debian. Concretely, withoutthis patch, users will see the Bash locale warning every time‘guix substitute’ starts (so at least once per session).
The second patch is stylistic: it avoids missing phases, which Ifind more readable.
Tested with a native x86_64-linux build and with‘--target=aarch64-linux-gnu’ from x86_64-linux.
I’d like to have these in ‘version-1.3.0’.
Thoughts?
Ludo’.
Ludovic Courtès (2): gnu: guix: Avoid Bash wrapper. gnu: guix: Phases refer to #:system and #:target.
gnu/packages/package-management.scm | 74 +++++++++++++++-------------- 1 file changed, 38 insertions(+), 36 deletions(-)
-- 2.31.1
L
L
Ludovic Courtès wrote on 4 May 15:39 +0200
[PATCH 1/2] gnu: guix: Avoid Bash wrapper.
(address . 48224@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20210504133958.11011-1-ludo@gnu.org
The Bash wrapper created by 'wrap-program' creates an extraindirection and may annoyingly emit locale warnings:
/gnu/store/…-bash-minimal-5.0.16/bin/bash: warning: setlocale: LC_ALL: cannot change locale (wtf)
This warning would typically show up when running Guix, as produced by'guix pack guix', on a foreign distro, annihilating efforts made in1d4ab335b22a93e01c2eb1eb3e93fc6534157040 and8a973abc6f7eebfcd8a904bfbb99cb9f86f66ef0.
* gnu/packages/package-management.scm (guix)[arguments]: In'wrap-program' phase, remove 'string-join' call for PATH and GOPATH.Replace 'wrap-program' call with a 'substitute*' form. Remove (whentarget ...) form.[inputs]: Remove "bash-minimal" added in commit38b9af7c92344a17b6680ebd2aeea14171f84a1c and no longer needed.--- gnu/packages/package-management.scm | 56 ++++++++++++++++------------- 1 file changed, 31 insertions(+), 25 deletions(-)
Toggle diff (78 lines)diff --git a/gnu/packages/package-management.scm b/gnu/packages/package-management.scmindex 827166c938..1a637f9ec8 100644--- a/gnu/packages/package-management.scm+++ b/gnu/packages/package-management.scm@@ -326,31 +326,38 @@ $(prefix)/etc/openrc\n"))) (open-pipe* OPEN_READ (string-append guile "/bin/guile") "-c" "(display (effective-version))")))- (path (string-join- (map (cut string-append <>- "/share/guile/site/"- effective)- (delete #f deps*))- ":"))- (gopath (string-join- (map (cut string-append <>- "/lib/guile/" effective- "/site-ccache")- (delete #f deps*))- ":"))+ (path (map (cut string-append <>+ "/share/guile/site/"+ effective)+ (delete #f deps*)))+ (gopath (map (cut string-append <>+ "/lib/guile/" effective+ "/site-ccache")+ (delete #f deps*))) (locpath (string-append locales "/lib/locale"))) - (wrap-program (string-append out "/bin/guix")- `("GUILE_LOAD_PATH" ":" prefix (,path))- `("GUILE_LOAD_COMPILED_PATH" ":" prefix (,gopath))- `("GUIX_LOCPATH" ":" suffix (,locpath)))-- (when target- ;; XXX Touching wrap-program rebuilds world- (let ((bash (assoc-ref inputs "bash")))- (substitute* (string-append out "/bin/guix")- (("^#!.*/bash") (string-append "#! " bash "/bin/bash")))))- #t)))+ ;; Modify 'guix' directly instead of using+ ;; 'wrap-command'. This avoids the indirection+ ;; through Bash, which in turn avoids getting Bash's+ ;; own locale warnings.+ (substitute* (string-append out "/bin/guix")+ (("!#")+ (string-append+ "!#\n\n"+ (object->string+ `(set! %load-path (append ',path %load-path)))+ "\n"+ (object->string+ `(set! %load-compiled-path+ (append ',gopath %load-compiled-path)))+ "\n"+ (object->string+ `(let ((path (getenv "GUIX_LOCPATH")))+ (setenv "GUIX_LOCPATH"+ (if path+ (string-append path ":" ,locpath)+ ,locpath))))+ "\n\n")))))) ;; The 'guix' executable has 'OUT/libexec/guix/guile' as ;; its shebang; that should remain unchanged, thus remove@@ -405,8 +412,7 @@ $(prefix)/etc/openrc\n"))) `(("boot-guile/i686" ,(bootstrap-guile-origin "i686-linux"))) '()) ,@(if (%current-target-system)- `(("bash" ,bash-minimal)- ("xz" ,xz))+ `(("xz" ,xz)) '()) ;; Tests also rely on these bootstrap executables.-- 2.31.1
L
L
Ludovic Courtès wrote on 4 May 15:39 +0200
[PATCH 2/2] gnu: guix: Phases refer to #:system and #:target.
(address . 48224@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20210504133958.11011-2-ludo@gnu.org
* gnu/packages/package-management.scm (guix)[arguments]: In'copy-bootstrap-guile' and 'wrap-program' phases, refer to #:systemand #:target instead of unquoting (%current-system)and (%current-target-system).--- gnu/packages/package-management.scm | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
Toggle diff (45 lines)diff --git a/gnu/packages/package-management.scm b/gnu/packages/package-management.scmindex 1a637f9ec8..5c6937adb8 100644--- a/gnu/packages/package-management.scm+++ b/gnu/packages/package-management.scm@@ -261,11 +261,9 @@ $(prefix)/etc/openrc\n"))) (intern (assoc-ref inputs "boot-guile") #f) ;; On x86_64 some tests need the i686 Guile.- ,@(if (and (not (%current-target-system))- (string=? (%current-system)- "x86_64-linux"))- '((intern (assoc-ref inputs "boot-guile/i686") #f))- '())+ (when (and (not target)+ (string=? system "x86_64-linux"))+ (intern (assoc-ref inputs "boot-guile/i686") #f)) ;; Copy the bootstrap executables. (for-each (lambda (input)@@ -299,9 +297,9 @@ $(prefix)/etc/openrc\n"))) ;; Make sure the 'guix' command finds GnuTLS, ;; Guile-JSON, and Guile-Git automatically. (let* ((out (assoc-ref outputs "out"))- (guile ,@(if (%current-target-system)- '((assoc-ref native-inputs "guile"))- '((assoc-ref inputs "guile"))))+ (guile (if target+ (assoc-ref native-inputs "guile")+ (assoc-ref inputs "guile"))) (avahi (assoc-ref inputs "guile-avahi")) (gcrypt (assoc-ref inputs "guile-gcrypt")) (guile-lib (assoc-ref inputs "guile-lib"))@@ -318,9 +316,7 @@ $(prefix)/etc/openrc\n"))) (locales (assoc-ref inputs "glibc-utf8-locales")) (deps (list gcrypt json sqlite gnutls git bs ssh zlib lzlib zstd guile-lib))- (deps* ,@(if (%current-target-system)- '(deps)- '((cons avahi deps))))+ (deps* (if target deps (cons avahi deps))) (effective (read-line (open-pipe* OPEN_READ-- 2.31.1
M
M
Maxime Devos wrote on 4 May 21:21 +0200
09cee506693a26af13b7fbc318f7a09528161bbc.camel@telenet.be
Toggle quote (3 lines)> The second patch is stylistic: it avoids missing phases, which I> find more readable.
I don't see any missing phases. (I read this as ‘The second patch isstylistic: it avoids making the existence of a phase dependent on%current-target-system.’)
Toggle quote (7 lines)> - (guile ,@(if (%current-target-system)> - '((assoc-ref native-inputs "guile"))> - '((assoc-ref inputs "guile"))))> + (guile (if target> + (assoc-ref native-inputs "guile")> + (assoc-ref inputs "guile")))
Something I tend to do is (assoc-ref (or native-inputs inputs) "guile")
Do you have any particular preference?
Toggle quote (7 lines)> (deps (list gcrypt json sqlite gnutls git> bs ssh zlib lzlib zstd guile-lib))> - (deps* ,@(if (%current-target-system)> - '(deps)> - '((cons avahi deps))))> + (deps* (if target deps (cons avahi deps)))
Why not simply ;; avahi is #f (not in 'inputs') when cross-compiling. ;; Remove it. (deps* (delete #f avahi))? Then, when guile-avahi becomes cross-compilable at some point, we onlyneed to adjust 'propagated-inputs' and not anything else.
Also, was this code (deps* ,@(if (%current-target-system) '(deps) ...)) needed inthe first place? guile2.2-guix inherits its phases from guix, and guile2.2-guix doesnot have a guile-zlib or guile-lzlib input.
Greetings,Maxime.
-----BEGIN PGP SIGNATURE-----
iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYJGetxccbWF4aW1lZGV2b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7lKCAQDII0Euh3ERQWLQkV5IwF9598e97EFyt3bCCrCOSwfdwQD+L08KkO2TPoYUOc9FQzDcEBWyjskAmMYJP2Ld35e+qQI==feig-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 5 May 01:05 +0200
Re: bug#48224: [PATCH 0/2] Avoid Bash wrapper in 'guix' package
(name . Maxime Devos)(address . maximedevos@telenet.be)
878s4uksm8.fsf_-_@gnu.org
Hi,
Pushed to ‘master’ with followup commits to address the ‘guile2.2-guix’issues you mentioned:
dd3e4fe6e7 gnu: guile2.2-guix: Add missing dependencies. 2238bd8ddc gnu: guile-lzlib: Add Guile 2.2 variant. 2a9af22540 gnu: guile-zlib: Add Guile 2.2 variant. c3f20a6678 gnu: guix: Phases refer to #:system, #:target, and #:native-inputs. e42bfd236e gnu: guix: Avoid Bash wrapper. 55f7cd701c gnu: guix: Add run-time dependency on Guile-Lib.
I’ll cherry-pick to ‘version-1.3.0’.
‘guile2.2-guix’ fails tests/swh.scm though, because (guix swh) relies on#:verify-certificate?, which Guile 2.2 didn’t have. I wonder howVagrant addressed that for Debian?
Thanks,Ludo’.
V
V
Vagrant Cascadian wrote on 5 May 01:26 +0200
(address . 48224@debbugs.gnu.org)
87y2cursh6.fsf@yucca
On 2021-05-05, Ludovic Courtès wrote:
Toggle quote (16 lines)> Pushed to ‘master’ with followup commits to address the ‘guile2.2-guix’> issues you mentioned:>> dd3e4fe6e7 gnu: guile2.2-guix: Add missing dependencies.> 2238bd8ddc gnu: guile-lzlib: Add Guile 2.2 variant.> 2a9af22540 gnu: guile-zlib: Add Guile 2.2 variant.> c3f20a6678 gnu: guix: Phases refer to #:system, #:target, and #:native-inputs.> e42bfd236e gnu: guix: Avoid Bash wrapper.> 55f7cd701c gnu: guix: Add run-time dependency on Guile-Lib.>> I’ll cherry-pick to ‘version-1.3.0’.>> ‘guile2.2-guix’ fails tests/swh.scm though, because (guix swh) relies on> #:verify-certificate?, which Guile 2.2 didn’t have. I wonder how> Vagrant addressed that for Debian?
Heavy handedly sprinkling (test-skip 1):
https://salsa.debian.org/debian/guix/-/blob/debian/devel/debian/patches/0029-tests-swh.scm-Disable-tests-the-fail-with-guile-2.2.patch https://salsa.debian.org/debian/guix/-/blob/debian/devel/debian/patches/0032-Disable-origin-visit-no-snapshots-test.patch
It would be nice to be able to automatically skip tests using knownguile 3 features when building with guile 2.2; maybe that will requireflagging each test individually or maybe there is a clever way to dothat dynamically?

live well, vagrant
-----BEGIN PGP SIGNATURE-----
iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCYJHYFQAKCRDcUY/If5cWqnzcAP96/2Vyig4NV5DdhMbUTbV/2/LvmecpvNodX/zmIJ5/eQD/X5IlPnckxLN8lph4II1y5uTE1a7l1vvC4pTP/NitNAg==6OVb-----END PGP SIGNATURE-----
L
L
Ludovic Courtès wrote 4 days ago
control message for bug #48224
(address . control@debbugs.gnu.org)
87y2cn60bd.fsf@gnu.org
close 48224 quit
L
L
Ludovic Courtès wrote 4 days ago
Re: bug#48224: [PATCH 0/2] Avoid Bash wrapper in 'guix' package
(name . Vagrant Cascadian)(address . vagrant@reproducible-builds.org)
87tunb6078.fsf_-_@gnu.org
Hi,
Vagrant Cascadian <vagrant@reproducible-builds.org> skribis:
Toggle quote (9 lines)>> ‘guile2.2-guix’ fails tests/swh.scm though, because (guix swh) relies on>> #:verify-certificate?, which Guile 2.2 didn’t have. I wonder how>> Vagrant addressed that for Debian?>> Heavy handedly sprinkling (test-skip 1):>> https://salsa.debian.org/debian/guix/-/blob/debian/devel/debian/patches/0029-tests-swh.scm-Disable-tests-the-fail-with-guile-2.2.patch> https://salsa.debian.org/debian/guix/-/blob/debian/devel/debian/patches/0032-Disable-origin-visit-no-snapshots-test.patch
V
V
Vagrant Cascadian wrote 11 hours ago
(name . Ludovic Courtès)(address . ludo@gnu.org)
87fsyrqot0.fsf@yucca
On 2021-05-09, Ludovic Courtès wrote:
Toggle quote (16 lines)> Vagrant Cascadian <vagrant@reproducible-builds.org> skribis:>>> ‘guile2.2-guix’ fails tests/swh.scm though, because (guix swh) relies on>>> #:verify-certificate?, which Guile 2.2 didn’t have. I wonder how>>> Vagrant addressed that for Debian?>>>> Heavy handedly sprinkling (test-skip 1):>>>> https://salsa.debian.org/debian/guix/-/blob/debian/devel/debian/patches/0029-tests-swh.scm-Disable-tests-the-fail-with-guile-2.2.patch>> https://salsa.debian.org/debian/guix/-/blob/debian/devel/debian/patches/0032-Disable-origin-visit-no-snapshots-test.patch>> This patch addresses the problem:>> https://git.savannah.gnu.org/cgit/guix.git/commit/?h=version-1.3.0&id=626e61959089b9c7d1faa5dd6b15f8ed94f551fb>> (It’s in 1.3.0rc2.)
Just tested 1.3.0 with the tests enabled again, and they passed! Thanks!

live well, vagrant
-----BEGIN PGP SIGNATURE-----
iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCYJyg6wAKCRDcUY/If5cWqm7SAQCdC1WGi4X9tHObc6SMH7dcQZV9uDJSA7wNgyW0lPVLdAEAvTb+6sW0fC2OI2a/mhZojik9BC8tga0RPsfV0FrdRAE==LEf5-----END PGP SIGNATURE-----
?