[bug#59372] [PATCH] gnu: peek: Add missing input.

[bug#59372] [PATCH] gnu: peek: Add missing input.

[jgart via Guix-patches via]

* gnu/packages/video.scm (peek): Add missing input.
[inputs]: Add ffmpeg so that Peek will not fail to run on a foreign
distro.

hi, peek fails to run on a foreign distro without ffmpeg as an input.

this patch fixes that.

all best,

jgart
---
 gnu/packages/video.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index c394c65442..4f38d4432c 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -5369,6 +5369,8 @@ (define-public peek
     (arguments '(#:glib-or-gtk? #t))
     (inputs
      `(("gtk+" ,gtk+)
+       ;; Peek fails to run on a foreign distro without ffmpeg.
+       ("ffmpeg" ,ffmpeg)
        ("python" ,python-wrapper)))
     (native-inputs
      `(("desktop-file-utils" ,desktop-file-utils) ; for update-desktop-database
-- 
2.38.1

[bug#59372] [PATCH] gnu: peek: Add missing input.

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

Hi jgart,

jgart via Guix-patches via 写道：
> hi, peek fails to run on a foreign distro without ffmpeg as an 
> input.
>
> this patch fixes that.

Thanks!

As far as I can tell, this change doesn't affect the peek 
package's output at all.  Am I mistaken?  What should the 
observable difference be, beyond ‘working’?

How are you invoking both peeks, and testing the difference?  Are 
they both running in the exact same environment, back-to-back?

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#59372] [PATCH] gnu: peek: Add missing input.

[jgart via Guix-patches via]

On Sat, 19 Nov 2022 03:50:05 +0100 Tobias Geerinckx-Rice <me@tobias.gr> wrote:
> As far as I can tell, this change doesn't affect the peek 
> package's output at all.  Am I mistaken?  What should the 
> observable difference be, beyond ‘working’?

It was complaining on Ubuntu when I started it that either ffmpeg needs
to be installed or a gnome-shell needs to be running. I didn't log the
exact error message but that was more or less it.

> How are you invoking both peeks, and testing the difference?

When I add ffmpeg it worked without the warning and actually started
recording my screen as expected. After pressing the record button of
course ;()

I tested once without ffmpeg and once with ffmpeg. 

The former failed to record.

> Are they both running in the exact same environment, back-to-back?

Yup I tested both in Ubuntu.

Let me know if you'd like any more info or if I wasn't precise enough
in my description.

all best,

jgart

[bug#59372] [PATCH] gnu: peek: Add missing input.

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]

jgart 写道：
> It was complaining on Ubuntu when I started it that either 
> ffmpeg needs
> to be installed or a gnome-shell needs to be running. I didn't 
> log the
> exact error message but that was more or less it.

And how does this patch provide ffmpeg?  It adds it to the build 
environment, but what happens then?

We know it doesn't add a reference to the final ‘peek’ output (you 
can verify this yourself, assuming peek builds reproducibly).  It 
doesn't change any code in the peek binary.

So what's the mechanism by which it would work?  It will need to 
be documented.

>> How are you invoking both peeks, and testing the difference?
>
> When I add ffmpeg it worked without the warning and actually 
> started
> recording my screen as expected. After pressing the record 
> button of
> course ;()

But how exactly?  Please provide exact steps and commands.

( 写道：
> Could you also update the inputs/arguments style to gexp form?

…but in a separate patch, please.)

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#59372] [PATCH] gnu: peek: Add missing input.

[jgart via Guix-patches via]

On Sat, 19 Nov 2022 10:38:26 +0100 Tobias Geerinckx-Rice <me@tobias.gr> wrote:
> So what's the mechanism by which it would work?  It will need to 
> be documented.

Hi Tobias,

peek, needs either ffmpeg or gnome shell to be present.

See here for the exact reference:

https://github.com/phw/peek/blob/2daba7cf114e01cff870afd20b96b212a0252054/src/application.vala#L67

How should I handle that in Guix?

Should I patch ffmpeg? What if a user wants to use gnome-shell instead of ffmpeg?

Is there a switch I can provide in Guile code?

all best,

jgart

[bug#59372] [PATCH] gnu: peek: Add missing input.

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

Hi jgart,

The patch you sent does nothing and didn't fix your foreign 
distro.  The questions you won't answer were supposed to find out 
what might have, but let's move on.

So, please test ('s suggestion.  It's the right fix!  Propagation 
isn't.

You can offer two variants of peek (peek-ffmpeg and 
peek-gnome-shell, say) if you like.  I don't think that's 
necessary, though, unless ffmpeg support is incomplete?

Thanks,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#59372] [PATCH] gnu: peek: Add missing input.

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

jgart 写道：
> peek, needs either ffmpeg or gnome shell to be present.
> How should I handle that in Guix?

Ideally: let peek pop up the window asking the user to install 
ffmpeg.

Then the user installs ffmpeg only if it's actually needed, and 
isn't ‘forced’ to pay the cost if it's not.

Does it not show that window on a foreign distribution?

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#59372] [PATCH] gnu: peek: Add missing input.

[jgart via Guix-patches via]

On Mon, 21 Nov 2022 21:18:32 +0100 Tobias Geerinckx-Rice <me@tobias.gr> wrote:
> Does it not show that window on a foreign distribution?

Yup, it does show that window on a foreign distro.

So, we should just have the user install ffmpeg along with peek in their profile?

Should this ticket be closed then? https://issues.guix.gnu.org/issue/51202#3

bug#59372: [PATCH] gnu: peek: Add missing input.

[宋文武 via Guix-patches via]

jgart <jgart@dismail.de> writes:

> On Mon, 21 Nov 2022 21:18:32 +0100 Tobias Geerinckx-Rice <me@tobias.gr> wrote:
>> Does it not show that window on a foreign distribution?
>
> Yup, it does show that window on a foreign distro.
>
> So, we should just have the user install ffmpeg along with peek in their profile?
>
> Should this ticket be closed then? https://issues.guix.gnu.org/issue/51202#3

Yes, I think it's expected to install ffmpeg yourself for peek when not
in gnome shell.  Close now.

[bug#59372] [PATCH] gnu: peek: Add missing input.

[zimoun]

Hi,

Thanks for resuming #51202. :-)

On Sat, 19 Nov 2022 at 03:50, Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org> wrote:

> As far as I can tell, this change doesn't affect the peek 
> package's output at all.  Am I mistaken?  What should the 
> observable difference be, beyond ‘working’?

Peek needs to be propagated.  An issue is open,

    http://issues.guix.gnu.org/issue/51202
    
IIRC, peek needs to be patched to avoid the propagation and personally,
I am using something along,

    guix shell peek \
            gtk+ gsettings-desktop-schemas ffmpeg \
            -- peek


Cheers,
simon

[bug#59372] [PATCH] gnu: peek: Add missing input.

[jgart via Guix-patches via]

On Mon, 21 Nov 2022 18:13:26 +0100 zimoun <zimon.toutoune@gmail.com> wrote:
> Hi,
> 
> Thanks for resuming #51202. :-)
> 
> On Sat, 19 Nov 2022 at 03:50, Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org> wrote:
> 
> > As far as I can tell, this change doesn't affect the peek 
> > package's output at all.  Am I mistaken?  What should the 
> > observable difference be, beyond ‘working’?
> 
> Peek needs to be propagated.  An issue is open,

I read through the open issue. Should I send a patch implementing lilyp's suggestion?

That's what I was also thinking of doing but I also had the question
regarding choosing between ffmpeg or gnome-shell. Should we prefer
ffmpeg? Would be cool if we could support choosing between ffmpeg or
gnome-shell.

Is there currently a way to do that?

[bug#59372] [PATCH] gnu: peek: Add missing input.

[zimoun]

Hi,

On Mon, 21 Nov 2022 at 13:38, jgart via Guix-patches via <guix-patches@gnu.org> wrote:

> That's what I was also thinking of doing but I also had the question
> regarding choosing between ffmpeg or gnome-shell. Should we prefer
> ffmpeg? Would be cool if we could support choosing between ffmpeg or
> gnome-shell.

Well, from my point of view, let start to use ffmeg without the
propagation.  Then, incremental improvements. :-)

Cheers,
simon

[bug#59372] [PATCH] gnu: peek: Add missing input.

[jgart via Guix-patches via]

On Tue, 22 Nov 2022 15:39:31 +0100 zimoun <zimon.toutoune@gmail.com> wrote:
> Well, from my point of view, let start to use ffmeg without the
> propagation.  Then, incremental improvements. :-)

Ok, should I send a new patch patching the ffmpeg executable from the
inputs of peek?

[bug#59372] [PATCH] gnu: peek: Add missing input.

[( via Guix-patches via]

Heya,

On Sat Nov 19, 2022 at 1:27 AM GMT, jgart via Guix-patches via wrote:
> * gnu/packages/video.scm (peek): Add missing input.
> [inputs]: Add ffmpeg so that Peek will not fail to run on a foreign
> distro.

> diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
> index c394c65442..4f38d4432c 100644
> --- a/gnu/packages/video.scm
> +++ b/gnu/packages/video.scm
> @@ -5369,6 +5369,8 @@ (define-public peek
>      (arguments '(#:glib-or-gtk? #t))
>      (inputs
>       `(("gtk+" ,gtk+)
> +       ;; Peek fails to run on a foreign distro without ffmpeg.
> +       ("ffmpeg" ,ffmpeg)
>         ("python" ,python-wrapper)))
>      (native-inputs
>       `(("desktop-file-utils" ,desktop-file-utils) ; for update-desktop-database

I don't think this will be enough; does the program call the ``ffmpeg''
executable? If so, you'll need to patch the call to ``ffmpeg'' to point to
/gnu/store/.../bin/ffmpeg directly.

Could you also update the inputs/arguments style to gexp form?

    -- (