Subject: Re: [PATCH v2] gnu: Add grimshot.

From: Maxime Devos <[email-masked]>
Date: Tue, 26 Jul 2022 20:29:53 +0200

On 26-07-2022 19:58, Antero Mejr wrote:
> * gnu/packages/wm.scm (grimshot): New variable.
> ---
> changes for v2:
> 1. using copy-build-system instead of trivial-build-system because it is
> simpler, copy-build-system handles unpacking the source
Now that you are using copy-build-system, using phases, the 
'this-package-input' can be changed to something more robust ...
> +(define-public grimshot
> +  (package
> +    (inherit sway)
> +    (name "grimshot")
> +    (source (origin
> +              (inherit (package-source sway))
> +              (snippet #~(begin
> +                           (delete-file "contrib/grimshot.1")))))
Some people have a preference for writing #~(begin [a single 
invocation]) anyway, but I'd like to note that wrapping the delete-file 
in a (begin ...) is not technically required.
> +    (build-system copy-build-system)
> +    (arguments
> +     (list #:install-plan #~`(("grimshot" "bin/")
> +                              ("grimshot.1" "usr/share/man/man1/"))
> +           #:phases #~(modify-phases %standard-phases
> +                        (add-after 'unpack 'chdir
> +                          (lambda _
> +                            (chdir "contrib")))
> +                        (add-after 'chdir 'patch-script-deps
No need to abbreviate dependencies -> deps, not that it matters much I 
suppose.
> +                          (lambda _
> +                            (substitute* "grimshot"
> +                              (("date ")
> +                               (string-append #$(this-package-input "coreutils")
> +                                              "/bin/date "))

Now that this is written as a phase, it becomes possible to use 
'search-input-file', like this:

> (lambda* (#:key inputs #:allow-other-keys)
>   (substitute*
>     (("\\b(date|jq|swaymsg|...)\\b" _ binary)
>      (search-input-file inputs (string-append "bin/" binary)))))
>
This way, you are not referring to input labels anymore, which has as 
benefit that package transformations become more robust. E.g., if you do 
it this way instead of using input labels (which this-package-input) 
does, it becomes possible to do things like 
--with-input=coreutils=busybox or the Scheme equivalent.
> +                        (add-after 'patch-script-deps 'build-man-page
> +                          (lambda _
> +                            (with-input-from-file "grimshot.1.scd"
> +                              (lambda _
> +                                (with-output-to-file "grimshot.1"
> +                                  (lambda _
> +                                    (invoke #+(file-append
> +                                               (this-package-native-input
> +                                                "scdoc")
> +                                               "/bin/scdoc")))))))))))

'invoke' does not need the absolute file name, so you the #+(file-append 
...) can be simplified to just "scdoc": [all the surrounding 
with-input/output-...+lambda (invoke "scdoc")], 'invoke' will 
automatically figure out the absolute file name.

Greetings,
Maxime.