[bug#49238] [PATCH v1 2/2] gnu: Add ytfzf.

[Raghav Gururajan via Guix-patches via <guix-patches@gnu.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2744 bytes --]

Hi Maxime!

> That's a neat trick to avoid fragile (substitute* ...).
> However, as I understand it, the origin is meant to be
> usable as source code (think install dependencies && guix build && tar xzf stuff.tgz
> && ./configure && make && try it out), such that
> "guix build --source=transitive stuff" would give all the
> source code for building stuff (in the sense of ‘Corresponding Source’
> of the GPL), and "guix-mpv", 'guix-jq" ... don't exist anywhere.
> 
> I don't know if that has been spelled out somewhere though.
> 
> I'd suggest adding "patch" to 'native-inputs', adding the patch
> to 'inputs' or 'native-inputs' (doesn't really matter which) and
> doing (invoke "patch" OPTIONS "blabla.patch") before the substitute*.
> 
> Actually, myself I'm not convinced because you could consider
> the package definition itself to be part of the ‘corresponding source’.

I get the idea, but I don't think we can call .patch files inside 
phases, the patches directory is not exposed/available inside 
build-environment.

> I'm wondering if (string-append (assoc-ref inputs "catimg") "/bin/catimg")
> would be better, as 'string-append' is less complex than 'format'
> and "format" doesn't seem to provide any additional value here.

Cool! I have made this change in v4.

> Now about the patch:
> 
> Maybe tell people to report issues at bug-guix@gnu.org or #guix? Dunno

Changed in v4.

> Don't forget to patch 'column'.
> 
> Don't forget to patch "mkfifo".
> 
> Don't forget patching 'sleep'. It is not shell a built-in
> (try "type sleep" and "type [" in a terminal").
> 
> Don't forget patching 'grep' and 'tr'.
> 
> Don't forget 'grep -> guix-grep' and 'cut -> guix-cut'
> 
> Don't forget 'awk -> guix-awk' and 'sort -> guix-sort' and 'uniq -> 'guix-uniq'
> 
> Don't forget 'sed -> guix-sed' and 'awk -> guix-awk'

Thanks, I have patched these in v4.

> FWIW, writing to "/tmp/ytfzf" seems to be a potential security problem
> and bad behaviour on multiple-user systems.
> 
> What would happen if /tmp/ytfzf is a symlink to /etc/passwd an ytfzf is run
> as root (yes, running as root is not recommended)? Would that brick the system?
> What if /tmp/ytfzf is a symlink to ~/.profile? Would that brick the login?
> 
> What if multiple users run ytfzf concurrently? Would they overwrite eaech
> other subtitles?  Would a different user be able to see what the other
> is downloading?
> 
> A relatively easy fix would be to write to, say, $HOME/.cache/ytzf-subs
> instead (not sure what the proper directory would be), which is completely
> under the user's control.

Good catch. I have changed this in v4.

Let me know if v4 looks good.

Regards,
RG.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]