Subject: Re: bug#65924: [PATCH core-updates 3/3] gnu: git-minimal: Add coreutils and sed to PATH.

From: Ludovic Courtès <[email-masked]>
Date: Sat, 14 Oct 2023 19:01:02 +0200

Maxim Cournoyer <[email-masked]> skribis:

> Fixes <https://issues.guix.gnu.org/65924>.
>
> * gnu/packages/version-control.scm (git-minimal)
> [arguments] <imported-modules>: New field.
> <modules>: Augment with (ice-9 match), (ice-9 textual-ports) and (guix
> search-paths).
> <phases>: Add patch-commands phase.
> [inputs]: Add coreutils-minimal and sed.

[...]

> +      #:imported-modules `(,@%gnu-build-system-modules
> +                           ,@(source-module-closure '((guix search-paths))))

I think we should avoid the dependency on (guix search-paths) here, to
avoid situation such as that described in
<https://issues.guix.gnu.org/66525>.

> +          (add-after 'unpack 'patch-commands
> +            (lambda* (#:key inputs #:allow-other-keys)
> +              (define (prepend-string-to-file text file)
> +                "Prepend TEXT to FILE."

Nitpick: no need to add a docstring to internal defines because it’s
optimized out and inaccessible (you can use a comment instead).

> +                (let ((content (call-with-input-file file
> +                                 (cut get-string-all <>))))
> +                  (call-with-output-file file
> +                    (lambda (port)
> +                      (display text port)
> +                      (display content port)))))
> +
> +              (define PATH-variable-definition
> +                (let ((value
> +                       (match (evaluate-search-paths
> +                               (list $PATH)
> +                               (list #$(this-package-input "coreutils-minimal")
> +                                     #$(this-package-input "sed")))
> +                         (((spec . value))
> +                          value))))
> +                  (string-append
> +                   (search-path-definition $PATH value
> +                                           #:kind 'prefix) "\n\n")))
> +
> +              ;; Ensure that coreutils (for basename) and sed are on PATH
> +              ;; for any script that sources the 'git-sh-setup.sh' file.
> +              (prepend-string-to-file PATH-variable-definition
> +                                      "git-sh-setup.sh")

How about something along these lines instead:

  ;; Instead PATH definition at the top of the file.
  (substitute* "git-sh-setup.sh"
    (("^unset CDPATH" all)
     (string-append "PATH=" (dirname (search-input-file inputs "bin/basename"))
                    ":$PATH\nexport PATH\n" all)))

?

Thanks,
Ludo’.