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.
next prev parent 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).