Subject: Re: [bug#69743] [PATCH] update grimshot package definition

From: Ignas Lapėnas <[email-masked]>
Date: Tue, 12 Mar 2024 15:33:36 +0200

Hi, this is a first patch for me. Just to be clear, I'm still a newb
with Guix and Guile. All feedback is greatly appreciated. Thank You :)

> I'm happy to see this improvement of building the manpage from source.
This part was not written by me. Last package definition version had a
chdir to the contrib directory step, to correctly copy over required
files. Since now the contrib directory is directly downloaded from
github it was no longer necessary. (that was the part removed)

> For my curiosity, what's the benefit of propagating grim? It looks
> like the full bath is also being substituted in the script?
While running the ./pre-inst-env to check if the package still works
correctly, I had an error that grim was not found. After a quick read on
the manual, noticed that propogated inputs actually copy it over.
It seems that my mistake is probably due to not updating this part.

#+BEGIN_EXAMPLE
(add-after 'chdir 'patch-script-dependencies
#+END_EXAMPLE

since chdir stage was removed it might have possibly skipped other steps
altogether. I'll look into it some more how it works exactly and try to
provide a new patch.

Jack Hill <[email-masked]> writes:

> [1. text/plain]
> On Tue, 12 Mar 2024, Ignas Lapėnas wrote:
>
>> Issue: https://issues.guix.gnu.org/69667
>>
>> Grimshot was dropped from sway.
>> https://github.com/swaywm/sway/issues/7405
>>
>> The contrib was moved to:
>> https://github.com/OctopusET/sway-contrib
>>
>> Package definition updated accordingly.
>>
>> Change-Id: I9ed9cdf7ce10fe878b4f2b931791ed22a63ff99a
>> ---
>> gnu/packages/wm.scm | 60 +++++++++++++++++++++++++--------------------
>> 1 file changed, 33 insertions(+), 27 deletions(-)
>>
>> diff --git a/gnu/packages/wm.scm b/gnu/packages/wm.scm
>> index edbb3096b8..eeaee0cb68 100644
>> --- a/gnu/packages/wm.scm
>> +++ b/gnu/packages/wm.scm
>> @@ -3524,46 +3524,52 @@ (define-public avizo
>
> I haven't had a chance to test on a system yet, but thanks for
> preparing this, reading through it it looks good.
>
>> +                   (add-after 'patch-script-dependencies 'build-man-page
>> +                     (lambda _
>> +                       (with-input-from-file "grimshot.1.scd"
>> +                         (lambda _
>> +                           (with-output-to-file "grimshot.1"
>> +                             (lambda _
>> +                               (invoke "scdoc"))))))))))
>
> I'm happy to see this improvement of building the manpage from source.
>
>>     (native-inputs (list scdoc))
>>     (inputs (list coreutils
>> -                  grim
>>                   jq
>>                   libnotify
>>                   slurp
>>                   sway
>>                   wl-clipboard))
>> +    (propagated-inputs (list grim))
>
> For my curiosity, what's the benefit of propagating grim? It looks
> like the full bath is also being substituted in the script?
>
> Thanks!
> Jack

-- 
Pagarbiai,
Ignas Lapėnas