Subject: Re: [bug#76572] [PATCH 3/4] gnu: gnome-shell-extension-gsconnect: Patch shebangs with gjs.

From: Maxim Cournoyer <[email-masked]>
Date: Fri, 28 Feb 2025 09:53:30 +0900

Hi!

aurtzy <[email-masked]> writes:

> The 'patch-shebangs phase does not handle shebangs of the form
> "/usr/bin/env -S", so we manually patch them to fix a not-found error.

Oh!  Could you please open an issue about this open so we remember to
fix the problem at its root?

> * gnu/packages/gnome-xyz.scm (gnome-shell-extension-gsconnect):
> [arguments]<#:phases>: Add 'patch-gjs-shebangs phase to patch shebangs using
> "/usr/bin/env -S" to execute gjs.
>
> Change-Id: I6b84474e4976484f1203b7cf78fe5e882694cd22
> ---
>  gnu/packages/gnome-xyz.scm | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/gnu/packages/gnome-xyz.scm b/gnu/packages/gnome-xyz.scm
> index 0a785193c4..abcbe2c9f9 100644
> --- a/gnu/packages/gnome-xyz.scm
> +++ b/gnu/packages/gnome-xyz.scm
> @@ -765,6 +765,18 @@ (define-public gnome-shell-extension-gsconnect
>                   "gtk_update_icon_cache: false")
>                  (("update_desktop_database: true")
>                   "update_desktop_database: false"))))
> +          (add-after 'unpack 'patch-gjs-shebangs

I'd also mention and cross-reference the newly opened issue about
/usr/bin/env -S not being patched here, with e.g.

TODO: Remove after patch-shebangs is fixed to handle '/usr/bin/env -S'
shebangs (see bug#NNNNN).

> +            (lambda* (#:key inputs #:allow-other-keys)
> +              (for-each
> +               (lambda (file)
> +                 (substitute* file
> +                   (("^#!.*$")

That pattern seems a bit too wide?  Can'twe match at least gjs in it, to
avoid breaking non-gjs shebangs which may be introduced in the future
and forgotten?

> +                    (string-append "#!" (which "gjs") " -m"))))
> +               '("installed-tests/minijasmine"
> +                 "src/gsconnect-preferences"
> +                 "src/service/nativeMessagingHost.js"
> +                 "src/service/daemon.js"
> +                 "webextension/gettext.js"))))

Otherwise, LGTM.  Could you please submit a v2 with my above
suggestions?

-- 
Thanks,
Maxim