Package: guix-patches;
Reported by: Philip McGrath <philip <at> philipmcgrath.com>
Date: Mon, 8 Aug 2022 06:07:02 UTC
Severity: normal
Tags: patch
Done: Ludovic Courtès <ludo <at> gnu.org>
Bug is archived. No further changes may be made.
Message #26 received at 57050 <at> debbugs.gnu.org (full text, mbox):
From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at> To: Philip McGrath <philip <at> philipmcgrath.com>, 57050 <at> debbugs.gnu.org Subject: Re: [PATCH 3/6] gnu: chez-scheme: Fix use of "/bin/sh". Date: Mon, 08 Aug 2022 10:53:42 +0200
Am Montag, dem 08.08.2022 um 02:10 -0400 schrieb Philip McGrath:
> The unsuccessful attempt to execute "/bin/sh" by Chez Scheme's
> 'process'
> function seems to have caused parts of the Chez Scheme test suite to
> have been silently skipped. The issue was exposed by the upcoming
> changes to Racket's build system.
>
> * gnu/packages/patches/chez-scheme-bin-sh.patch,
> gnu/packages/patches/racket-chez-scheme-bin-sh.patch: New patches.
> * gnu/packages/patches/racket-minimal-sh-via-rktio.patch: Rename to
> ...
> * gnu/packages/patches/racket-rktio-bin-sh.patch: ... this, refresh
> with
> upstream, and shorten the macro name.
> * gnu/local.mk (dist_patch_DATA): Update accordingly.
> * gnu/packages/racket.scm (%racket-origin)[patches]: Likewise.
> (racket-vm-common-configure-flags): Likewise.
> * gnu/packages/chez.scm (chez-scheme)[origin]<patches>: Likewise.
> [inputs]: Add bash-minimal.
> [arguments]<#:configure-flags>: Set CPPFLAGS to enable new patches.
> ---
> gnu/local.mk | 4 +-
> gnu/packages/chez.scm | 5 ++
> gnu/packages/patches/chez-scheme-bin-sh.patch | 66
> +++++++++++++++++++
> .../patches/racket-chez-scheme-bin-sh.patch | 66
> +++++++++++++++++++
> ...-rktio.patch => racket-rktio-bin-sh.patch} | 28 ++++----
> gnu/packages/racket.scm | 5 +-
> 6 files changed, 158 insertions(+), 16 deletions(-)
> create mode 100644 gnu/packages/patches/chez-scheme-bin-sh.patch
> create mode 100644 gnu/packages/patches/racket-chez-scheme-bin-
> sh.patch
> rename gnu/packages/patches/{racket-minimal-sh-via-rktio.patch =>
> racket-rktio-bin-sh.patch} (79%)
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 234f21ad33..e827feed36 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -940,6 +940,7 @@ dist_patch_DATA
> = \
> %D%/packages/patches/ceph-boost-compat.patch \
> %D%/packages/patches/ceph-rocksdb-compat.patch \
> %D%/packages/patches/cheese-vala-
> update.patch \
> + %D%/packages/patches/chez-scheme-bin-
> sh.patch \
> %D%/packages/patches/chmlib-inttypes.patch \
> %D%/packages/patches/cl-asdf-config-
> directories.patch \
> %D%/packages/patches/clamav-config-llvm-libs.patch \
> @@ -1769,7 +1770,8 @@ dist_patch_DATA
> = \
> %D%/packages/patches/ripperx-missing-file.patch \
> %D%/packages/patches/rpcbind-CVE-2017-8779.patch \
> %D%/packages/patches/rtags-separate-
> rct.patch \
> - %D%/packages/patches/racket-minimal-sh-via-rktio.patch \
> + %D%/packages/patches/racket-chez-scheme-bin-sh.patch \
> + %D%/packages/patches/racket-rktio-bin-sh.patch \
> %D%/packages/patches/remake-impure-
> dirs.patch \
> %D%/packages/patches/restic-0.9.6-fix-tests-for-go1.15.patch \
> %D%/packages/patches/retroarch-LIBRETRO_DIRECTORY.patch \
> diff --git a/gnu/packages/chez.scm b/gnu/packages/chez.scm
> index 4c253effa0..66098c7a24 100644
> --- a/gnu/packages/chez.scm
> +++ b/gnu/packages/chez.scm
> @@ -34,6 +34,7 @@ (define-module (gnu packages chez)
> #:use-module (guix build-system copy)
> #:use-module (guix build-system gnu)
> #:use-module (guix build-system copy)
> + #:use-module (gnu packages bash)
> #:use-module (gnu packages compression)
> #:use-module (gnu packages ncurses)
> #:use-module (gnu packages ghostscript)
> @@ -269,6 +270,7 @@ (define-public chez-scheme
> (base32
>
> "0xchqq8cm0ka5wgpn18sjs0hh15rc3nb7xrjqbbc9al3asq0d7gc"))
> (file-name (git-file-name name version))
> + (patches (search-patches "chez-scheme-bin-sh.patch"))
> (snippet #~(begin
> (use-modules (guix build utils))
> ;; TODO: consider putting this in a (guix
> ...) or
> @@ -287,6 +289,7 @@ (define-public chez-scheme
> `(,util-linux "lib") ;<-- libuuid
> zlib
> lz4
> + bash-minimal ;<-- for process
> ncurses ;<-- for expeditor
> ;; for X11 clipboard support in expeditor:
> ;;
> https://github.com/cisco/ChezScheme/issues/9#issuecomment-222057232
> @@ -313,6 +316,8 @@ (define-public chez-scheme
> (cut memq 'threads <>))
> #~("--threads")
> #~())
> + ,(string-append "CPPFLAGS=-DGUIX_RKTIO_BIN_SH="
> + #$(file-append bash-minimal "/bin/sh"))
> "ZLIB=-lz"
> "LZ4=-llz4"
> "--libkernel"
> diff --git a/gnu/packages/patches/chez-scheme-bin-sh.patch
> b/gnu/packages/patches/chez-scheme-bin-sh.patch
> new file mode 100644
> index 0000000000..e6228e3264
> --- /dev/null
> +++ b/gnu/packages/patches/chez-scheme-bin-sh.patch
> @@ -0,0 +1,66 @@
> +From c170f0f3a326f293ee1f460a70303382966ca41b Mon Sep 17 00:00:00
> 2001
> +From: Philip McGrath <philip <at> philipmcgrath.com>
> +Date: Thu, 19 May 2022 13:41:56 -0400
> +Subject: [PATCH] patch s_process for "/bin/sh" on Guix
> +
> +This patch reuses the C preprocessor macro `GUIX_RKTIO_BIN_SH`
> +from a previous patch.
> +
> +If:
> +
> + 1. The `GUIX_RKTIO_BIN_SH` macro is defined; and
> +
> + 2. The path specified by `GUIX_RKTIO_BIN_SH` exists;
> +
> +then `s_process` will call `execl` with the file specified by
> +`GUIX_RKTIO_BIN_SH` instead of "/bin/sh".
> +
> +This patch does not change the behavior of `s_system`, which relies
> +on `system` from the C library.
> +---
> + c/prim5.c | 20 +++++++++++++++++++-
> + 1 file changed, 19 insertions(+), 1 deletion(-)
> +
> +diff --git a/c/prim5.c b/c/prim5.c
> +index 5a07893..926d68d 100644
> +--- a/c/prim5.c
> ++++ b/c/prim5.c
> +@@ -746,6 +746,22 @@ static ptr s_process(char *s, IBOOL stderrp) {
> +
> + INT tofds[2], fromfds[2], errfds[2];
> + struct sigaction act, oint_act;
> ++ /* BEGIN PATCH for Guix */
> ++#if defined(GUIX_RKTIO_BIN_SH)
> ++# define GUIX_AS_a_STR_HELPER(x) #x
> ++# define GUIX_AS_a_STR(x) GUIX_AS_a_STR_HELPER(x)
> ++ /* A level of indirection makes `#` work as needed: */
> ++ struct stat guix_stat_buf;
> ++ char *guix_sh =
> ++ (0 == stat(GUIX_AS_a_STR(GUIX_RKTIO_BIN_SH), &guix_stat_buf))
> ++ ? GUIX_AS_a_STR(GUIX_RKTIO_BIN_SH)
> ++ : "/bin/sh";
> ++# undef GUIX_AS_a_STR
> ++# undef GUIX_AS_a_STR_HELPER
> ++#else /* GUIX_RKTIO_BIN_SH */
> ++ char *guix_sh = "/bin/sh";
> ++#endif
> ++ /* END PATCH for Guix */
/* BEGIN PATCH for Guix */ and /* END PATCH for Guix */ is in my humble
opinion superfluous (though apparently also present in the already
exsting patch, whose author might disagree).
Also, I think this could easily be submitted upstream if you named it
RKTIO_SHELL and rktio_shell respectively, with the default to
"/bin/sh". Then, we'd simply have to -DRKTIO_SHELL=/path/to/bin/sh in
our #:make-flags.
> ++ /* BEGIN PATCH for Guix */
> ++ execl(guix_sh, guix_sh, "-c", s, NULL);
> ++ /* END PATCH for Guix */
Likewise.
As for absorbing racket-specific patches into chez-scheme itself, I'm
not too sure if I agree with that approach. Maybe a different prefix
rather than RKTIO should be used here – one that fits chez.
Cheers
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.