GNU bug report logs - #56803
[PATCH 0/6] Add hydrus network and its dependencies

Previous Next

Full log

View this message in rfc822 format

From: Tomasz Jeneralczyk <tj <at> schwi.pl>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Maxime Devos <maximedevos <at> telenet.be>, 56803 <at> debbugs.gnu.org
Subject: [bug#56803] [PATCH 0/6] Add hydrus network and its dependencies
Date: Sat, 13 Aug 2022 14:34:52 +0000

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.