* [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).