* [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package @ 2021-05-04 13:25 Ludovic Courtès 2021-05-04 13:39 ` [bug#48224] [PATCH 1/2] gnu: guix: Avoid Bash wrapper Ludovic Courtès 0 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2021-05-04 13:25 UTC (permalink / raw) To: 48224; +Cc: Ludovic Courtès, Maxim Cournoyer Hello! The first patch here addresses a locale warning coming from Bash that I noticed while using 1.3.0rc1 on Debian. Concretely, without this 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 I find 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#48224] [PATCH 1/2] gnu: guix: Avoid Bash wrapper. 2021-05-04 13:25 [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package Ludovic Courtès @ 2021-05-04 13:39 ` Ludovic Courtès 2021-05-04 13:39 ` [bug#48224] [PATCH 2/2] gnu: guix: Phases refer to #:system and #:target Ludovic Courtès 0 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2021-05-04 13:39 UTC (permalink / raw) To: 48224; +Cc: Ludovic Courtès The Bash wrapper created by 'wrap-program' creates an extra indirection 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 in 1d4ab335b22a93e01c2eb1eb3e93fc6534157040 and 8a973abc6f7eebfcd8a904bfbb99cb9f86f66ef0. * 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 (when target ...) form. [inputs]: Remove "bash-minimal" added in commit 38b9af7c92344a17b6680ebd2aeea14171f84a1c and no longer needed. --- gnu/packages/package-management.scm | 56 ++++++++++++++++------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/gnu/packages/package-management.scm b/gnu/packages/package-management.scm index 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [bug#48224] [PATCH 2/2] gnu: guix: Phases refer to #:system and #:target. 2021-05-04 13:39 ` [bug#48224] [PATCH 1/2] gnu: guix: Avoid Bash wrapper Ludovic Courtès @ 2021-05-04 13:39 ` Ludovic Courtès 2021-05-04 19:21 ` Maxime Devos 0 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2021-05-04 13:39 UTC (permalink / raw) To: 48224; +Cc: Ludovic Courtès * gnu/packages/package-management.scm (guix)[arguments]: In 'copy-bootstrap-guile' and 'wrap-program' phases, refer to #:system and #:target instead of unquoting (%current-system) and (%current-target-system). --- gnu/packages/package-management.scm | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/gnu/packages/package-management.scm b/gnu/packages/package-management.scm index 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [bug#48224] [PATCH 2/2] gnu: guix: Phases refer to #:system and #:target. 2021-05-04 13:39 ` [bug#48224] [PATCH 2/2] gnu: guix: Phases refer to #:system and #:target Ludovic Courtès @ 2021-05-04 19:21 ` Maxime Devos 2021-05-04 23:05 ` [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package Ludovic Courtès 0 siblings, 1 reply; 8+ messages in thread From: Maxime Devos @ 2021-05-04 19:21 UTC (permalink / raw) To: Ludovic Courtès, 48224 [-- Attachment #1: Type: text/plain, Size: 1778 bytes --] > 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 is stylistic: it avoids making the existence of a phase dependent on %current-target-system.’) > - (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? > (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 only need to adjust 'propagated-inputs' and not anything else. Also, was this code (deps* ,@(if (%current-target-system) '(deps) ...)) needed in the first place? guile2.2-guix inherits its phases from guix, and guile2.2-guix does not have a guile-zlib or guile-lzlib input. Greetings, Maxime. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 260 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package 2021-05-04 19:21 ` Maxime Devos @ 2021-05-04 23:05 ` Ludovic Courtès 2021-05-04 23:26 ` Vagrant Cascadian 0 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2021-05-04 23:05 UTC (permalink / raw) To: Maxime Devos; +Cc: 48224, Vagrant Cascadian 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 how Vagrant addressed that for Debian? Thanks, Ludo’. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package 2021-05-04 23:05 ` [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package Ludovic Courtès @ 2021-05-04 23:26 ` Vagrant Cascadian 2021-05-09 21:56 ` Ludovic Courtès 0 siblings, 1 reply; 8+ messages in thread From: Vagrant Cascadian @ 2021-05-04 23:26 UTC (permalink / raw) To: Ludovic Courtès, Maxime Devos; +Cc: 48224 [-- Attachment #1: Type: text/plain, Size: 1331 bytes --] On 2021-05-05, Ludovic Courtès wrote: > 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 known guile 3 features when building with guile 2.2; maybe that will require flagging each test individually or maybe there is a clever way to do that dynamically? live well, vagrant [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package 2021-05-04 23:26 ` Vagrant Cascadian @ 2021-05-09 21:56 ` Ludovic Courtès 2021-05-13 3:45 ` Vagrant Cascadian 0 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2021-05-09 21:56 UTC (permalink / raw) To: Vagrant Cascadian; +Cc: 48224, Maxime Devos Hi, 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.) HTH! Ludo’. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package 2021-05-09 21:56 ` Ludovic Courtès @ 2021-05-13 3:45 ` Vagrant Cascadian 0 siblings, 0 replies; 8+ messages in thread From: Vagrant Cascadian @ 2021-05-13 3:45 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 48224, Maxime Devos [-- Attachment #1: Type: text/plain, Size: 911 bytes --] On 2021-05-09, Ludovic Courtès wrote: > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-05-13 3:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-05-04 13:25 [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package Ludovic Courtès 2021-05-04 13:39 ` [bug#48224] [PATCH 1/2] gnu: guix: Avoid Bash wrapper Ludovic Courtès 2021-05-04 13:39 ` [bug#48224] [PATCH 2/2] gnu: guix: Phases refer to #:system and #:target Ludovic Courtès 2021-05-04 19:21 ` Maxime Devos 2021-05-04 23:05 ` [bug#48224] [PATCH 0/2] Avoid Bash wrapper in 'guix' package Ludovic Courtès 2021-05-04 23:26 ` Vagrant Cascadian 2021-05-09 21:56 ` Ludovic Courtès 2021-05-13 3:45 ` Vagrant Cascadian
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/guix.git 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 read-only IMAP folder(s) and NNTP newsgroup(s).