[bug#57050] [PATCH 3/6] gnu: chez-scheme: Fix use of "/bin/sh".

[Liliana Marie Prikler]

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@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