[PATCH 1/2] guix-install.sh: Add GUIX_ALLOW_OVERWRITE environment variable.

  • Done
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Maxim Cournoyer
  • Tobias Geerinckx-Rice
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
Merged with
M
M
Maxim Cournoyer wrote on 14 Dec 2022 16:56
(address . guix-patches@gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221214155603.29381-1-maxim.cournoyer@gmail.com
The need for this use case appeared when attempting to install Guix on a truly
minimal image made with Buildroot, which lacked enough GNU components that I
had to extract a guix pack to /gnu before attempting installation, which would
then refuse to proceed because of the existing /gnu.

* etc/guix-install.sh: Document environment variables.
(REQUIRE): Add missing "useradd" command.
(sys_create_store) [GUIX_ALLOW_OVERWRITE]: Skip pre-existing installation
checks and output a warning. Extract the tarball directly to /.
---
etc/guix-install.sh | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)

Toggle diff (60 lines)
diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index fb9006b3e2..06730f7e3f 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -29,6 +29,22 @@
# We require Bash but for portability we'd rather not use /bin/bash or
# /usr/bin/env in the shebang, hence this hack.
+
+# Environment variables
+#
+# GUIX_BINARY_FILE_NAME
+#
+# Can be used to override the automatic download mechanism and point
+# to a local Guix binary archive filename like
+# "/tmp/guix-binary-1.4.0rc2.armhf-linux.tar.xz"
+#
+# GUIX_ALLOW_OVERWRITE
+#
+# Instead of aborting to avoid overwriting a previous installations,
+# allow copying over /var/guix or /gnu. This can be useful when the
+# installation required the user to extract Guix packs under /gnu to
+# satisfy its dependencies.
+
if [ "x$BASH_VERSION" = "x" ]
then
exec bash "$0" "$@"
@@ -53,6 +69,7 @@ REQUIRE=(
"chmod"
"uname"
"groupadd"
+ "useradd"
"tail"
"tr"
"xz"
@@ -337,16 +354,15 @@ sys_create_store()
_debug "--- [ ${FUNCNAME[0]} ] ---"
- if [[ -e "/var/guix" || -e "/gnu" ]]; then
+ if [[ -z $GUIX_ALLOW_OVERWRITE && (-e /var/guix || -e /gnu) ]]; then
die "A previous Guix installation was found. Refusing to overwrite."
+ else
+ _msg "${WAR}Overwriting existing installation!"
fi
cd "$tmp_path"
- tar --extract --file "$pkg" && _msg "${PAS}unpacked archive"
-
_msg "${INF}Installing /var/guix and /gnu..."
- mv "${tmp_path}/var/guix" /var/
- mv "${tmp_path}/gnu" /
+ tar --extract --file "$pkg" -C /
_msg "${INF}Linking the root user's profile"
mkdir -p ~root/.config/guix

base-commit: 1b6e251ed1bae7aa2f544e8ccb6b4aaf872e77e6
--
2.38.1
T
T
Tobias Geerinckx-Rice wrote on 14 Dec 2022 17:16
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
875yeec57z.fsf@nckx
Hi Maxim,

Nice! More steps towards world domination.

Maxim Cournoyer ???
Toggle quote (2 lines)
> +# Environment variables

These sound like they should be command-line arguments.

[…]

Actually, I'm not totally sold on GUIX_ALLOW_OVERWRITE. It's not
solving much a problem.

Instead, the error message could be specific about what it
considers a ‘previous Guix installation’ — which would be a good
idea regardless — and tell the admin exactly what needs to be
removed to continue.

Toggle quote (3 lines)
> "groupadd"
> + "useradd"

Good catch, but separate patch. (?)

Toggle quote (18 lines)
> - if [[ -e "/var/guix" || -e "/gnu" ]]; then
> + if [[ -z $GUIX_ALLOW_OVERWRITE && (-e /var/guix || -e /gnu)
> ]]; then
> die "A previous Guix installation was found. Refusing
> to overwrite."
> + else
> + _msg "${WAR}Overwriting existing installation!"
> fi
>
> cd "$tmp_path"
> - tar --extract --file "$pkg" && _msg "${PAS}unpacked
> archive"
> -
> _msg "${INF}Installing /var/guix and /gnu..."
> - mv "${tmp_path}/var/guix" /var/
> - mv "${tmp_path}/gnu" /
> + tar --extract --file "$pkg" -C /

I'm still in favour of using something like ‘mktemp -d
/gnu.XXXXXX’ here if there's no security flaw I missed. WDYT?

If the overwrite functionality is kept, we should remove the old
directories before re-populating them.

Kind regards,

T G-R
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCY5n7kA0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW15JYQA/0MC+9Nq+MATmo1+I6w6hw1wcCsFaUR5bzjr3DyO
vmKWAP9ZPyqhLEYgIENx0DPBEz0p8zLD2G/yx5gCCHWgFyEmAA==
=++lV
-----END PGP SIGNATURE-----

M
M
Maxim Cournoyer wrote on 14 Dec 2022 19:25
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
87zgbp3ksd.fsf@gmail.com
Hi Tobias,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

Toggle quote (4 lines)
> Hi Maxim,
>
> Nice! More steps towards world domination.

Eh :-)

Toggle quote (5 lines)
> Maxim Cournoyer ???
>> +# Environment variables
>
> These sound like they should be command-line arguments.

I agree, but that'd require a loop, or GNU getopt, and I'm not motivated
enough in the moment to re-design it :-). When we get there, we could
add an --uninstall option too.

Toggle quote (5 lines)
> […]
>
> Actually, I'm not totally sold on GUIX_ALLOW_OVERWRITE. It's not
> solving much a problem.

The problem it solves for me was that I needed to use 'guix pack'd
dependencies such as gpg, glibc for getent, and shadow's
groupadd/useradd, etc. to satisfy the install script dependencies on my
weird target OS (minimal busybox embedded OS); which are needed to be
unpacked under /gnu, thus conflicting with the requirement that /gnu
doesn't exist.

I tried a relocatable pack, but it didn't work, at least for gpg (file
not found error).

Toggle quote (9 lines)
> Instead, the error message could be specific about what it considers a
> ‘previous Guix installation’ — which would be a good idea regardless —
> and tell the admin exactly what needs to be removed to continue.
>
>> "groupadd"
>> + "useradd"
>
> Good catch, but separate patch. (?)

OK!

Toggle quote (22 lines)
>> - if [[ -e "/var/guix" || -e "/gnu" ]]; then
>> + if [[ -z $GUIX_ALLOW_OVERWRITE && (-e /var/guix || -e /gnu) ]];
>> then
>> die "A previous Guix installation was found. Refusing
>> to overwrite."
>> + else
>> + _msg "${WAR}Overwriting existing installation!"
>> fi
>> cd "$tmp_path"
>> - tar --extract --file "$pkg" && _msg "${PAS}unpacked archive"
>> -
>> _msg "${INF}Installing /var/guix and /gnu..."
>> - mv "${tmp_path}/var/guix" /var/
>> - mv "${tmp_path}/gnu" /
>> + tar --extract --file "$pkg" -C /
>
> I'm still in favour of using something like ‘mktemp -d /gnu.XXXXXX’
> here if there's no security flaw I missed. WDYT?
>
> If the overwrite functionality is kept, we should remove the old
> directories before re-populating them.

Hopefully the reason the above makes more sense is also covered by my
use case explanation above.

Is the use case/change motivation a bit clearer now?

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 14 Dec 2022 21:46
[PATCH v2 1/3] guix-install.sh: Add missing "useradd" command.
(address . 60068@debbugs.gnu.org)
20221214204640.16879-1-maxim.cournoyer@gmail.com
* etc/guix-install.sh: (REQUIRE): Add missing "useradd" command.
---
etc/guix-install.sh | 1 +
1 file changed, 1 insertion(+)

Toggle diff (16 lines)
diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index f008593d84..b8ea9e54c3 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -53,6 +53,7 @@ REQUIRE=(
"chmod"
"uname"
"groupadd"
+ "useradd"
"tail"
"tr"
"xz"

base-commit: fa23fb86f7741570d194bba1f227016d9aa25881
--
2.38.1
M
M
Maxim Cournoyer wrote on 14 Dec 2022 21:46
[PATCH v2 2/3] guix-install.sh: Add GUIX_ALLOW_OVERWRITE environment variable.
(address . 60068@debbugs.gnu.org)
20221214204640.16879-2-maxim.cournoyer@gmail.com
The need for this use case appeared when attempting to install Guix on a truly
minimal image made with Buildroot, which lacked enough GNU components that I
had to extract a guix pack to /gnu before attempting installation, which would
then refuse to proceed because of the existing /gnu.

* etc/guix-install.sh: Document environment variables.
(sys_create_store) [GUIX_ALLOW_OVERWRITE]: Skip pre-existing installation
checks and output a warning. Extract the tarball directly to /.
---
etc/guix-install.sh | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)

Toggle diff (50 lines)
diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index b8ea9e54c3..62d85e765a 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -29,6 +29,22 @@
# We require Bash but for portability we'd rather not use /bin/bash or
# /usr/bin/env in the shebang, hence this hack.
+
+# Environment variables
+#
+# GUIX_BINARY_FILE_NAME
+#
+# Can be used to override the automatic download mechanism and point
+# to a local Guix binary archive filename like
+# "/tmp/guix-binary-1.4.0rc2.armhf-linux.tar.xz"
+#
+# GUIX_ALLOW_OVERWRITE
+#
+# Instead of aborting to avoid overwriting a previous installations,
+# allow copying over /var/guix or /gnu. This can be useful when the
+# installation required the user to extract Guix packs under /gnu to
+# satisfy its dependencies.
+
if [ "x$BASH_VERSION" = "x" ]
then
exec bash "$0" "$@"
@@ -338,16 +354,15 @@ sys_create_store()
_debug "--- [ ${FUNCNAME[0]} ] ---"
- if [[ -e "/var/guix" || -e "/gnu" ]]; then
+ if [[ -z $GUIX_ALLOW_OVERWRITE && (-e /var/guix || -e /gnu) ]]; then
die "A previous Guix installation was found. Refusing to overwrite."
+ else
+ _msg "${WAR}Overwriting existing installation!"
fi
cd "$tmp_path"
- tar --extract --file "$pkg" && _msg "${PAS}unpacked archive"
-
_msg "${INF}Installing /var/guix and /gnu..."
- mv "${tmp_path}/var/guix" /var/
- mv "${tmp_path}/gnu" /
+ tar --extract --file "$pkg" -C /
_msg "${INF}Linking the root user's profile"
mkdir -p ~root/.config/guix
--
2.38.1
M
M
Maxim Cournoyer wrote on 14 Dec 2022 21:46
[PATCH v2 3/3] guix-install.sh: Directly exit in case of errors in chk_require.
(address . 60068@debbugs.gnu.org)
20221214204640.16879-3-maxim.cournoyer@gmail.com
* etc/guix-install.sh (chk_require): Directly exit in case of errors in
chk_require, instead of relying on 'set -e'.
---
etc/guix-install.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

Toggle diff (19 lines)
diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index 62d85e765a..ea10f35250 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -137,10 +137,8 @@ chk_require()
command -v "$c" &>/dev/null || warn+=("$c")
done
- [ "${#warn}" -ne 0 ] &&
- { _err "${ERR}Missing commands: ${warn[*]}.";
- return 1; }
-
+ [ "${#warn}" -ne 0 ] && die "Missing commands: ${warn[*]}."
+
_msg "${PAS}verification of required commands completed"
}
--
2.38.1
M
M
Maxim Cournoyer wrote on 14 Dec 2022 21:47
control message for bug #60068
(address . control@debbugs.gnu.org)
87cz8l3e7l.fsf@gmail.com
forcemerge 60068 60069
quit
L
L
Ludovic Courtès wrote on 15 Dec 2022 15:41
Re: bug#60069: [PATCH 2/2] guix-install.sh: Directly exit in case of errors in chk_require.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
871qp0pw4y.fsf_-_@gnu.org
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (2 lines)
> * etc/guix-install.sh: (REQUIRE): Add missing "useradd" command.

[...]

Toggle quote (3 lines)
> * etc/guix-install.sh (chk_require): Directly exit in case of errors in
> chk_require, instead of relying on 'set -e'.

These two patches LGTM; you can add them to ‘master’ and that way people
will benefit from it when installing 1.4.0.

Thanks!

Ludo’.
L
L
Ludovic Courtès wrote on 15 Dec 2022 15:43
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87wn6sohh7.fsf_-_@gnu.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (9 lines)
> The need for this use case appeared when attempting to install Guix on a truly
> minimal image made with Buildroot, which lacked enough GNU components that I
> had to extract a guix pack to /gnu before attempting installation, which would
> then refuse to proceed because of the existing /gnu.
>
> * etc/guix-install.sh: Document environment variables.
> (sys_create_store) [GUIX_ALLOW_OVERWRITE]: Skip pre-existing installation
> checks and output a warning. Extract the tarball directly to /.

Like Tobias, I’m reluctant to adding environment variables; I’m also
skeptical about the use case (I think it’s fine to let users remove
their previous installation if that’s what they want).

I also think we’d rather minimize changes to the script since we’re a
couple of days before the release.

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 15 Dec 2022 15:44
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87v8mcoheh.fsf_-_@gnu.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (9 lines)
> The need for this use case appeared when attempting to install Guix on a truly
> minimal image made with Buildroot, which lacked enough GNU components that I
> had to extract a guix pack to /gnu before attempting installation, which would
> then refuse to proceed because of the existing /gnu.
>
> * etc/guix-install.sh: Document environment variables.
> (sys_create_store) [GUIX_ALLOW_OVERWRITE]: Skip pre-existing installation
> checks and output a warning. Extract the tarball directly to /.

Like Tobias, I’m reluctant to adding environment variables; I’m also
skeptical about the use case (I think it’s fine to let users remove
their previous installation if that’s what they want).

I also think we’d rather minimize changes to the script since we’re a
couple of days before the release.

Thanks,
Ludo’.
M
M
Maxim Cournoyer wrote on 16 Dec 2022 06:07
(name . Ludovic Courtès)(address . ludo@gnu.org)
87wn6saqcz.fsf@gmail.com
Hi Ludovic,

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

Toggle quote (15 lines)
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> The need for this use case appeared when attempting to install Guix on a truly
>> minimal image made with Buildroot, which lacked enough GNU components that I
>> had to extract a guix pack to /gnu before attempting installation, which would
>> then refuse to proceed because of the existing /gnu.
>>
>> * etc/guix-install.sh: Document environment variables.
>> (sys_create_store) [GUIX_ALLOW_OVERWRITE]: Skip pre-existing installation
>> checks and output a warning. Extract the tarball directly to /.
>
> Like Tobias, I’m reluctant to adding environment variables; I’m also
> skeptical about the use case (I think it’s fine to let users remove
> their previous installation if that’s what they want).

Removing my previous installation wouldn't have helped (it would have
cleared the guix packs I needed to be able to run the installer).
Without this change, I wouldn't have been able to install guix using
guix-install.sh. It's niche, but I bet it'd help folks trying to
install Guix on Alpine and similar minimal OSes.

Toggle quote (3 lines)
> I also think we’d rather minimize changes to the script since we’re a
> couple of days before the release.

The change seems fairly small to me and would be easy to revert if it
causes a problem. But I'll let you do the call, given you're the one
pushing the release forward (thank you!).

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 16 Dec 2022 06:24
(name . Ludovic Courtès)(address . ludo@gnu.org)
87k02rc45b.fsf@gmail.com
Hi Ludo,

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

Toggle quote (14 lines)
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * etc/guix-install.sh: (REQUIRE): Add missing "useradd" command.
>
> [...]
>
>> * etc/guix-install.sh (chk_require): Directly exit in case of errors in
>> chk_require, instead of relying on 'set -e'.
>
> These two patches LGTM; you can add them to ‘master’ and that way people
> will benefit from it when installing 1.4.0.

Done, for the two patches mentioned above, leaving the
GUIX_ALLOW_OVERWRITE one open for further discussions.

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 4 Feb 2023 05:36
Re: [bug#60068] [PATCH 1/2] guix-install.sh: Add GUIX_ALLOW_OVERWRITE environment variable.
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
87mt5unj10.fsf@gmail.com
Hi Tobias and Ludovic,

Had you seen my reply below? It seems you had perhaps misunderstood the
use case at hand?

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (34 lines)
> Hi Tobias,
>
> Tobias Geerinckx-Rice <me@tobias.gr> writes:
>
>> Hi Maxim,
>>
>> Nice! More steps towards world domination.
>
> Eh :-)
>
>> Maxim Cournoyer ???
>>> +# Environment variables
>>
>> These sound like they should be command-line arguments.
>
> I agree, but that'd require a loop, or GNU getopt, and I'm not motivated
> enough in the moment to re-design it :-). When we get there, we could
> add an --uninstall option too.
>
>> […]
>>
>> Actually, I'm not totally sold on GUIX_ALLOW_OVERWRITE. It's not
>> solving much a problem.
>
> The problem it solves for me was that I needed to use 'guix pack'd
> dependencies such as gpg, glibc for getent, and shadow's
> groupadd/useradd, etc. to satisfy the install script dependencies on my
> weird target OS (minimal busybox embedded OS); which are needed to be
> unpacked under /gnu, thus conflicting with the requirement that /gnu
> doesn't exist.
>
> I tried a relocatable pack, but it didn't work, at least for gpg (file
> not found error).

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 17 Feb 2023 05:24
control message for bug #60069
(address . control@debbugs.gnu.org)
874jrkudez.fsf@gmail.com
close 60069
quit
?