diff mbox series

[bug#35033] gnu: mpv: Set absolute path to 'youtube-dl'.

Message ID 20190328200054.8306-1-eisenmann@fn.de
State Accepted
Headers show
Series [bug#35033] gnu: mpv: Set absolute path to 'youtube-dl'. | expand

Commit Message

Timo Eisenmann March 28, 2019, 8 p.m. UTC 
* gnu/packages/video.scm (mpv)[arguments]: Add phase 'patch-paths' to
substitute in the absolute path to 'youtube-dl'.
---
 gnu/packages/video.scm | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Ludovic Courtès April 3, 2019, 8:24 p.m. UTC | #1 
Hi Timo,

Timo Eisenmann <eisenmann@fn.de> skribis:

> * gnu/packages/video.scm (mpv)[arguments]: Add phase 'patch-paths' to
> substitute in the absolute path to 'youtube-dl'.

I think it’s a case where it may be more appropriate to look for
‘youtube-dl’ in $PATH because ‘youtube-dl’ is updated frequently.

WDYT?

Thanks,
Ludo’.
Arun Isaac April 3, 2019, 8:36 p.m. UTC | #2 
We normally don't require the user to install dependencies such as
youtube-dl into the profile. I think we should retain that behaviour
even in this case with mpv. It would mean fewer surprises for the
user. If I'm guessing correctly, mpv doesn't require a lot of effort to
rebuild.
Timo Eisenmann April 3, 2019, 10:48 p.m. UTC | #3 
On Wed, 03 Apr 2019 22:24:12 +0200
Ludovic Courtès <ludo@gnu.org> wrote:

> Hi Timo,
> 
> Timo Eisenmann <eisenmann@fn.de> skribis:
> 
> > * gnu/packages/video.scm (mpv)[arguments]: Add phase 'patch-paths'
> > to substitute in the absolute path to 'youtube-dl'.
> 
> I think it’s a case where it may be more appropriate to look for
> ‘youtube-dl’ in $PATH because ‘youtube-dl’ is updated frequently.
> 
> WDYT?

I think we should make mpv's youtube-dl feature work by default instead
of requiring the user to install youtube-dl into their profile.
That being said if, for some reason, someone does not want to update mpv
and still use a newer version of youtube-dl, they can do so by linking
the youtube-dl binary into mpv's config dir (by default: ~/.config/mpv).

> 
> Thanks,
> Ludo’.
Ludovic Courtès April 8, 2019, 8:29 a.m. UTC | #4 
Hello,

Timo Eisenmann <eisenmann@fn.de> skribis:

> On Wed, 03 Apr 2019 22:24:12 +0200
> Ludovic Courtès <ludo@gnu.org> wrote:

[...]

>> I think it’s a case where it may be more appropriate to look for
>> ‘youtube-dl’ in $PATH because ‘youtube-dl’ is updated frequently.
>> 
>> WDYT?
>
> I think we should make mpv's youtube-dl feature work by default instead
> of requiring the user to install youtube-dl into their profile.

Arun Isaac <arunisaac@systemreboot.net> skribis:

> We normally don't require the user to install dependencies such as
> youtube-dl into the profile. I think we should retain that behaviour
> even in this case with mpv. It would mean fewer surprises for the
> user. If I'm guessing correctly, mpv doesn't require a lot of effort to
> rebuild.

Alright, that makes sense (and indeed, mpv builds very quickly.)

I added a #t at the end of the phase and committed.

Thank you!

Ludo’.
diff mbox series

Patch

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 55a1931530..3c5ade55a3 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -1326,6 +1326,13 @@  SVCD, DVD, 3ivx, DivX 3/4/5, WMV and H.264 movies.")
     (arguments
      '(#:phases
        (modify-phases %standard-phases
+         (add-after
+          'unpack 'patch-paths
+          (lambda* (#:key inputs #:allow-other-keys)
+            (let ((ytdl (assoc-ref inputs "youtube-dl")))
+              (substitute* "player/lua/ytdl_hook.lua"
+                (("\"youtube-dl\",")
+                 (string-append "\"" ytdl "/bin/youtube-dl\","))))))
          (add-before
           'configure 'setup-waf
           (lambda* (#:key inputs #:allow-other-keys)