Message ID | 20221119012744.30470-1-jgart@dismail.de |
---|---|
State | New |
Headers | show |
Series | [bug#59372] gnu: peek: Add missing input. | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git-branch | success | View Git branch |
cbaines/applying patch | success | |
cbaines/issue | success | View issue |
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
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
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? -- (
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
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
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
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
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?
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
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
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
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?
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.
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