Re: [PATCH] mps-youtube

[ng0]

Marius Bakke <address@hidden> writes:

> ng0 <address@hidden> writes:
>
>> I am not sure about how the non-python runtime dependencies are supposed to 
>> be handled in python packages.
>>
>> python-pafy needs youtube-dl to function.
>> mps-youtube needs at least mpv and optionally ffmpeg to deal with conversion 
>> of formats.
>
> Have you checked whether the inputs are referenced in `guix gc -R
> /gnu/store/...item`?

Well, youtube-dl is in there, ffmpeg and mpv are not.

> If not, we'll need to make it record the absolute
> paths of the dependencies somehow. Often this can be done with
> '--with-foo=(string-append (assoc-ref inputs "foo") "/bin/foo"' in the
> configure step. Another approach is to substitute those commands
> with the full path directly in the code.

You have the choice between mpv and mplayer for mps-youtube, if I
remember correctly you get an informative error message if none
of the binaries is found in your $PATH.
The same applies for ffmpeg, but I never used the ffmpeg feature
so this is just asuming they are consistent.

part of mps_youtube/config.py:

def check_player(player):
    """ Check player exefile exists and get mpv version. """
    if util.has_exefile(player):
        util.load_player_info(player)

        if "mpv" in player:
            version = "%s.%s.%s" % g.mpv_version
            fmt = c.g, c.w, c.g, c.w, version
            msg = "%splayer%s set to %smpv%s (version %s)" % fmt
            return dict(valid=True, message=msg, value=player)

        else:
            msg = "%splayer%s set to %s%s%s" % (c.g, c.w, c.g, player, c.w)
            return dict(valid=True, message=msg, value=player)

    else:
        if mswin and not (player.endswith(".exe") or player.endswith(".com")):
            # Using mpv.exe has issues; use mpv.com
            if "mpv" in player:
                retval = check_player(player + ".com")
                if retval["valid"]:
                    return retval
            return check_player(player + ".exe")

        else:
            msg = "Player application %s%s%s not found" % (c.r, player, c.w)
            return dict(valid=False, message=msg)


>> So far they are in (inputs). Are those supposed to be installed by the users 
>> themselves?
>
> For optional dependencies (i.e. if mps-youtube gracefully handles
> missing ffmpeg), leaving it up to the user is good (but then it should
> not be an input). For "hard" runtime dependencies, the absolute paths
> must be recorded so that they won't get garbage collected.
>

With the info I gave above, I'd say I remove the dependencies
which are not found and add information to the description.
For example: … it can be used with either mpv or mplayer, for
optional format conversion you need to use ffmpeg.

What do you think?