unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Tomasz Jeneralczyk <tj@schwi.pl>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: Maxime Devos <maximedevos@telenet.be>, 56803@debbugs.gnu.org
Subject: [bug#56803] [PATCH 0/6] Add hydrus network and its dependencies
Date: Sat, 13 Aug 2022 14:34:52 +0000	[thread overview]
Message-ID: <6db66d068f014489f5202d825e1985af@schwi.pl> (raw)
In-Reply-To: <87lerx5u0q.fsf_-_@gnu.org>

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	other threads:[~2022-08-13 14:35 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27 18:57 [bug#56803] [PATCH 0/6] Add hydrus network and its dependencies Tomasz Jeneralczyk
2022-07-27 19:00 ` [bug#56803] [PATCH 1/6] gnu: Add swftools Tomasz Jeneralczyk
2022-07-27 19:00 ` [bug#56803] [PATCH 2/6] gnu: Add python-xvfbwrapper Tomasz Jeneralczyk
2022-07-27 19:00 ` [bug#56803] [PATCH 3/6] gnu: Add python-mpv Tomasz Jeneralczyk
2022-07-27 22:05   ` Maxime Devos
2022-07-28 14:58     ` Tomasz Jeneralczyk
2022-08-09 15:14       ` [bug#56803] [PATCH 0/6] Add hydrus network and its dependencies Ludovic Courtès
2022-08-13 14:34         ` Tomasz Jeneralczyk [this message]
2022-08-13 21:26           ` ( via Guix-patches via
2022-08-14 10:10             ` Tomasz Jeneralczyk
2022-08-14 10:11               ` ( via Guix-patches via
2022-08-09 16:59       ` [bug#56803] [PATCH 3/6] gnu: Add python-mpv Maxime Devos
2022-07-27 22:09   ` Maxime Devos
2022-07-27 19:00 ` [bug#56803] [PATCH 4/6] gnu: Add opencv-with-python Tomasz Jeneralczyk
2022-07-27 19:00 ` [bug#56803] [PATCH 5/6] gnu: Update python-cloudscraper Tomasz Jeneralczyk
2022-07-27 19:00 ` [bug#56803] [PATCH 6/6] gnu: Add hydrus-network Tomasz Jeneralczyk
2022-08-09 15:09   ` [bug#56803] [PATCH 0/6] Add hydrus network and its dependencies Ludovic Courtès
2022-08-14 12:46 ` [bug#56803] [PATCH v2 1/6] gnu: Add swftools Tomasz Jeneralczyk
2022-08-14 12:46   ` [bug#56803] [PATCH v2 2/6] gnu: Add python-xvfbwrapper Tomasz Jeneralczyk
2022-08-14 12:46   ` [bug#56803] [PATCH v2 3/6] gnu: Add python-mpv Tomasz Jeneralczyk
2022-08-14 12:46   ` [bug#56803] [PATCH v2 4/6] gnu: Add support for python in opencv Tomasz Jeneralczyk
2022-08-14 12:46   ` [bug#56803] [PATCH v2 5/6] gnu: Update python-cloudscraper Tomasz Jeneralczyk
2022-08-14 12:46   ` [bug#56803] [PATCH v2 6/6] gnu: Add hydrus-network Tomasz Jeneralczyk
2022-08-29 22:46     ` bug#56803: [PATCH 0/6] Add hydrus network and its dependencies Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6db66d068f014489f5202d825e1985af@schwi.pl \
    --to=tj@schwi.pl \
    --cc=56803@debbugs.gnu.org \
    --cc=ludo@gnu.org \
    --cc=maximedevos@telenet.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).