[PATCH] Fix Disarchive fallback on Guix System

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Timothy Sample
Owner
unassigned
Submitted by
Timothy Sample
Severity
normal
T
T
Timothy Sample wrote on 27 Dec 2021 19:39
(address . guix-patches@gnu.org)
877dbpuc2n.fsf@ngyro.com
Hi everyone,

I noticed recently that Disarchive fallback does not work on Guix
System. This is because the ‘(guix swh)’ module shells out to ‘tar’ to
extract the tarball that it downloads from SWH. However, when used as
part of ‘guix perform-download’ via the daemon, ‘tar’ is not available.
AFAICS, that the daemon is run with no ‘PATH’ at all.

You can confirm this by running (on Guix System):

$ GUIX_DOWNLOAD_FALLBACK_TEST=disarchive-mirrors \
guix build --check -S python-flask

You should see:

[...]
Downloading [...] from Software Heritage...
In procedure fport_write: Broken pipe
[...]

This patch adds ‘tar’ and ‘gzip’ to the daemon’s ‘PATH’. To me, this is
the most straight-forward way to fix the issue, but there are others.
Any opinions?


-- Tim
From 2893252c16f3e447eccd0f8d216bfb44b1965c43 Mon Sep 17 00:00:00 2001
From: Timothy Sample <samplet@ngyro.com>
Date: Thu, 23 Dec 2021 22:32:07 -0500
Subject: [PATCH] services: guix: Add tar and gzip to PATH.

* gnu/services/base.scm (guix-shepherd-service): Add the PATH
environment-variable and populate it with tar and gzip.
---
gnu/services/base.scm | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

Toggle diff (32 lines)
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 88869e40d2..2fad07097b 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -55,7 +55,8 @@ (define-module (gnu services base)
#:select (alsa-utils crda eudev e2fsprogs fuse gpm kbd lvm2 rng-tools))
#:use-module (gnu packages bash)
#:use-module ((gnu packages base)
- #:select (coreutils glibc glibc-utf8-locales))
+ #:select (coreutils glibc glibc-utf8-locales tar))
+ #:use-module ((gnu packages compression) #:select (gzip))
#:autoload (gnu packages guile-xyz) (guile-netlink)
#:autoload (gnu packages hurd) (hurd)
#:use-module (gnu packages package-management)
@@ -1709,7 +1710,14 @@ (define (guix-shepherd-service config)
(string-append "GUIX_LOCPATH="
#$glibc-utf8-locales
"/lib/locale")
- "LC_ALL=en_US.utf8")
+ "LC_ALL=en_US.utf8"
+ ;; Make 'tar' and 'gzip' available so
+ ;; that 'guix perform-download' can use
+ ;; them when downloading from Software
+ ;; Heritage via '(guix swh)'.
+ (string-append "PATH="
+ #$(file-append tar "/bin") ":"
+ #$(file-append gzip "/bin")))
(if proxy
(list (string-append "http_proxy=" proxy)
(string-append "https_proxy=" proxy))
--
2.34.0
L
L
Ludovic Courtès wrote on 5 Jan 2022 21:54
(name . Timothy Sample)(address . samplet@ngyro.com)(address . 52828@debbugs.gnu.org)
87mtk9kiol.fsf@gnu.org
Hi Timothy,

Timothy Sample <samplet@ngyro.com> skribis:

Toggle quote (30 lines)
> I noticed recently that Disarchive fallback does not work on Guix
> System. This is because the ‘(guix swh)’ module shells out to ‘tar’ to
> extract the tarball that it downloads from SWH. However, when used as
> part of ‘guix perform-download’ via the daemon, ‘tar’ is not available.
> AFAICS, that the daemon is run with no ‘PATH’ at all.
>
> You can confirm this by running (on Guix System):
>
> $ GUIX_DOWNLOAD_FALLBACK_TEST=disarchive-mirrors \
> guix build --check -S python-flask
>
> You should see:
>
> [...]
> Downloading [...] from Software Heritage...
> In procedure fport_write: Broken pipe
> [...]
>
> This patch adds ‘tar’ and ‘gzip’ to the daemon’s ‘PATH’. To me, this is
> the most straight-forward way to fix the issue, but there are others.
> Any opinions?
>
>>From 2893252c16f3e447eccd0f8d216bfb44b1965c43 Mon Sep 17 00:00:00 2001
> From: Timothy Sample <samplet@ngyro.com>
> Date: Thu, 23 Dec 2021 22:32:07 -0500
> Subject: [PATCH] services: guix: Add tar and gzip to PATH.
>
> * gnu/services/base.scm (guix-shepherd-service): Add the PATH
> environment-variable and populate it with tar and gzip.

What about uses of guix-daemon on other distros? Perhaps we assume tar
and gzip are already on PATH?

I’d feel more comfortable with a solution at the package level. In the
‘guix’ package, or perhaps in a Makefile.am, we could hard-code absolute
file names of tar and gzip in (guix scripts perform-download) and set
PATH from there. We’d need to do the same in (guix self) though.

Alternatively, we could change (guix swh) to use Guile-Zlib and the tar
implementation of Gash-Utils or that of Disarchive.

WDYT?

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 13 Jan 2022 11:55
control message for bug #53214
(address . control@debbugs.gnu.org)
87ilun9a6i.fsf@gnu.org
block 53214 by 52828
quit
T
T
Timothy Sample wrote on 15 Jan 2022 16:33
Re: bug#52828: [PATCH v2] swh: Do not rely on $PATH for tar and gzip.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 52828@debbugs.gnu.org)
8735lpxbd1.fsf_-_@ngyro.com
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (3 lines)
> What about uses of guix-daemon on other distros? Perhaps we assume tar
> and gzip are already on PATH?

That’s my guess. And if those other weird distros don’t have ‘tar’ and
‘gzip’ in “/usr/bin”, I’m sure they’re plenty accustomed to liberally
patching their packages. ;)

Toggle quote (10 lines)
> I’d feel more comfortable with a solution at the package level. In the
> ‘guix’ package, or perhaps in a Makefile.am, we could hard-code absolute
> file names of tar and gzip in (guix scripts perform-download) and set
> PATH from there. We’d need to do the same in (guix self) though.
>
> Alternatively, we could change (guix swh) to use Guile-Zlib and the tar
> implementation of Gash-Utils or that of Disarchive.
>
> WDYT?

I’ve attached a new patch that mixes those two suggestions but gets the
first one a little wrong. It uses the absolute path for ‘tar’, but uses
Guile-zlib for decompression.

I honestly don’t have a strong opinion about when and where to set
‘$PATH’ vs. using a configured, absolute path. My original patch
assumed that it’s the user’s job to make sure that ‘tar’ and ‘gzip’ are
available to Guix at runtime. This patch assumes that that linkage
happens at configure time. The main benefit I could see to setting
‘$PATH’ in ‘(guix scripts perform-download)’ is that we could add our
‘tar’ and ‘gzip’ as a suffix. This makes it work while allowing users
to choose whatever ‘tar’ and ‘gzip’ they prefer. The downside is that
whenever we use ‘(guix swh)’ have to remember to make sure that ‘tar’
and ‘gzip’ are available.

Basically, I can to change this to do the setup in ‘perform-download’,
but I really want to understand why.

Thanks!
Attachment: v2-0001-swh-Do-not-rely-on-PATH-for-tar-and-gzip.patch
L
L
Ludovic Courtès wrote on 15 Jan 2022 22:33
Re: bug#52828: [PATCH] Fix Disarchive fallback on Guix System
(name . Timothy Sample)(address . samplet@ngyro.com)(address . 52828@debbugs.gnu.org)
87a6fw3cri.fsf_-_@gnu.org
Hi,

Timothy Sample <samplet@ngyro.com> skribis:

Toggle quote (9 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> What about uses of guix-daemon on other distros? Perhaps we assume tar
>> and gzip are already on PATH?
>
> That’s my guess. And if those other weird distros don’t have ‘tar’ and
> ‘gzip’ in “/usr/bin”, I’m sure they’re plenty accustomed to liberally
> patching their packages. ;)

True.

Toggle quote (28 lines)
>> I’d feel more comfortable with a solution at the package level. In the
>> ‘guix’ package, or perhaps in a Makefile.am, we could hard-code absolute
>> file names of tar and gzip in (guix scripts perform-download) and set
>> PATH from there. We’d need to do the same in (guix self) though.
>>
>> Alternatively, we could change (guix swh) to use Guile-Zlib and the tar
>> implementation of Gash-Utils or that of Disarchive.
>>
>> WDYT?
>
> I’ve attached a new patch that mixes those two suggestions but gets the
> first one a little wrong. It uses the absolute path for ‘tar’, but uses
> Guile-zlib for decompression.
>
> I honestly don’t have a strong opinion about when and where to set
> ‘$PATH’ vs. using a configured, absolute path. My original patch
> assumed that it’s the user’s job to make sure that ‘tar’ and ‘gzip’ are
> available to Guix at runtime. This patch assumes that that linkage
> happens at configure time. The main benefit I could see to setting
> ‘$PATH’ in ‘(guix scripts perform-download)’ is that we could add our
> ‘tar’ and ‘gzip’ as a suffix. This makes it work while allowing users
> to choose whatever ‘tar’ and ‘gzip’ they prefer. The downside is that
> whenever we use ‘(guix swh)’ have to remember to make sure that ‘tar’
> and ‘gzip’ are available.
>
> Basically, I can to change this to do the setup in ‘perform-download’,
> but I really want to understand why.

Hmm yes, I guess you’re right, I prefer the initial patch after all.
(In particular I’m not keen on adding things to (guix config).)

Go for it?

Longer-term, I think it would still be interesting to migrate to
Guile-Zlib + Disarchive/Gash-Utils, but we can check that later—better
fix the Disarchive fallback first.

Thanks and sorry for the hesitations!

Ludo’.
T
T
Timothy Sample wrote on 17 Jan 2022 02:00
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 52828-done@debbugs.gnu.org)
87iluj89cn.fsf@ngyro.com
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (5 lines)
> Hmm yes, I guess you’re right, I prefer the initial patch after all.
> (In particular I’m not keen on adding things to (guix config).)
>
> Go for it?

Pushed as 3b6755defe44c4795e134a46a7ef7b6009146872.

Toggle quote (4 lines)
> Longer-term, I think it would still be interesting to migrate to
> Guile-Zlib + Disarchive/Gash-Utils, but we can check that later—better
> fix the Disarchive fallback first.

For sure. Changing it to use zlib was pretty easy. Sadly, neither of
those tarball libraries are slam dunks for Guix as of today.

Toggle quote (2 lines)
> Thanks and sorry for the hesitations!

No worries. Thanks very much for the review! :)


-- Tim
Closed
L
L
Ludovic Courtès wrote on 17 Jan 2022 14:11
(name . Timothy Sample)(address . samplet@ngyro.com)(address . 52828-done@debbugs.gnu.org)
87sftmpkvv.fsf@gnu.org
Howdy!

Timothy Sample <samplet@ngyro.com> skribis:

Toggle quote (9 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hmm yes, I guess you’re right, I prefer the initial patch after all.
>> (In particular I’m not keen on adding things to (guix config).)
>>
>> Go for it?
>
> Pushed as 3b6755defe44c4795e134a46a7ef7b6009146872.

Thanks!

Toggle quote (7 lines)
>> Longer-term, I think it would still be interesting to migrate to
>> Guile-Zlib + Disarchive/Gash-Utils, but we can check that later—better
>> fix the Disarchive fallback first.
>
> For sure. Changing it to use zlib was pretty easy. Sadly, neither of
> those tarball libraries are slam dunks for Guix as of today.

I suppose they’re good enough for this use case, aren’t they?

Here we’re only dealing with one tarball producer (the software behind
SWH), so the scope is much narrower compared to what Gash might need to
support (tarballs of a variety of flavors.)

Ludo’.
Closed
?