guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#56803] [PATCH 0/6] Add hydrus network and its dependencies


From: Tomasz Jeneralczyk
Subject: [bug#56803] [PATCH 0/6] Add hydrus network and its dependencies
Date: Sat, 13 Aug 2022 14:34:52 +0000
User-agent: Roundcube Webmail/1.4.11

On 2022-08-09 15:09, Ludovic Courtès wrote:
Perhaps these comments, or some of them, should go as comments in the
source?  That will prove helpful next time you or someone else tries to
work on the package.
They cluttered the code with meta-information that I didn't consider to be directly related to the package's source. Thought anyone searching would see the commit message and that would be enough. I'll add them back in if that's what you prefer.

In general, the solution here, rather than copy files like the ‘ffmpeg’
executable, would be to patch Hydrus so that it contains the absolute
file name of ‘ffmpeg’ as returned by (search-input-file inputs "/bin/ffmpeg").

How does that sound?
At the time I thought that directly patching the binary paths could break some logic, but I looked into it and it looks like I only had to patch one `if` statement. This could change in future releases, but it's not likely. It appears to work as well and maybe even better - Now miniupnpc isn't timing out for me, which might or might not be because of this change.

In addition, this substitution should be done in a phase rather than in
a snippet, because (1) the result of ‘guix build -S’ should be
platform-independent and thus not include the file name of ‘mpv’ for a
particular system, and (2) the reference to variable ‘mpv’ at the top
level can cause problems due to circular dependencies among modules.
I didn't know that, but it makes sense. I moved the code into its own phase. And if I understand the 2nd point correctly I should use something like `(assoc-ref inputs "mpv")` instead of `#$mpv`, right? I cannot use `(search-input-file ...)` because the name of the mpv's lib is determined during runtime by the python code and I didn't want to hardcode the mpv so file's version.

After I finish making the changes I'll send the whole patchset again, it will also include updated packages to their newest release and be rebased onto current master.

Thank you for taking the time to review my work.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]