GNU bug report logs - #49425
[PATCH] guix-install.sh: Prompt for configuring substitutes discovery.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49425 in the body.
You can then email your comments to 49425 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH] guix-install.sh: Prompt for configuring substitutes discovery.
Date: Mon,  5 Jul 2021 19:49:03 -0400

Also ensure prompt_yes_no always print the message with a trailing space,
which is more pleasing to the eye.

* etc/guix-daemon.conf.in <--discovery=no>: New guix-daemon option.
* etc/guix-daemon.service.in: Likewise.
* etc/init.d/guix-daemon.in: Likewise.
* etc/openrc/guix-daemon.in: Likewise.
* etc/guix-install.sh (configure_substitute_discovery): New procedure.
(sys_enable_guix_daemon): Ask the user whether automatic substitute discovery
should be enabled.  Set the '--discovery' argument accordingly.
(prompt_yes_no): Add a trailing space to the message.
(sys_authorize_build_farms): Remove trailing space from the message argument.
* NEWS (Distribution): Add news.
---
 NEWS                       |  4 +++-
 etc/guix-daemon.conf.in    |  2 +-
 etc/guix-daemon.service.in |  3 ++-
 etc/guix-install.sh        | 22 ++++++++++++++++++++--
 etc/init.d/guix-daemon.in  |  2 +-
 etc/openrc/guix-daemon.in  |  2 +-
 6 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index b0647b3700..ad9433a921 100644
--- a/NEWS
+++ b/NEWS
@@ -14,7 +14,9 @@ Please send Guix bug reports to bug-guix <at> gnu.org.
 
 * Changes in 1.4.0 (since 1.3.0)
 ** Package management
-   * New 'deb' format for the 'guix pack' command
+*** New 'deb' format for the 'guix pack' command
+** Distribution
+*** The installation script can now configure substitute discovery
 
 * Changes in 1.3.0 (since 1.2.0)
 ** Package management
diff --git a/etc/guix-daemon.conf.in b/etc/guix-daemon.conf.in
index 755192d555..b122ca8c7a 100644
--- a/etc/guix-daemon.conf.in
+++ b/etc/guix-daemon.conf.in
@@ -7,4 +7,4 @@ start on runlevel [2345]
 
 stop on runlevel [016]
 
-exec @localstatedir@/guix/profiles/per-user/root/current-guix/bin/guix-daemon --build-users-group=guixbuild
+exec @localstatedir@/guix/profiles/per-user/root/current-guix/bin/guix-daemon --build-users-group=guixbuild --discovery=no
diff --git a/etc/guix-daemon.service.in b/etc/guix-daemon.service.in
index 407cdd199c..17b54eaeb0 100644
--- a/etc/guix-daemon.service.in
+++ b/etc/guix-daemon.service.in
@@ -6,7 +6,8 @@
 Description=Build daemon for GNU Guix
 
 [Service]
-ExecStart=@localstatedir@/guix/profiles/per-user/root/current-guix/bin/guix-daemon --build-users-group=guixbuild
+ExecStart=@localstatedir@/guix/profiles/per-user/root/current-guix/bin/guix-daemon \
+    --build-users-group=guixbuild --discover=no
 Environment='GUIX_LOCPATH=@localstatedir@/guix/profiles/per-user/root/guix-profile/lib/locale' LC_ALL=en_US.utf8
 RemainAfterExit=yes
 StandardOutput=syslog
diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index 6a799fa823..1e7702d4c5 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -96,7 +96,7 @@ _debug()
 # $1: The prompt question.
 prompt_yes_no() {
     while true; do
-        read -rp "$1" yn
+        read -rp "$1 " yn
         case $yn in
             [Yy]*) return 0;;
             [Nn]*) return 1;;
@@ -249,6 +249,16 @@ chk_sys_nscd()
     fi
 }
 
+# Configure substitute discovery according to user's preferences.
+# $1 is the installed service file to edit.
+configure_substitute_discovery() {
+if grep -q -- '--discover=no' "$1" && \
+        prompt_yes_no "Would you like to enable automatic substitute \
+discovery? (yes/no)"; then
+    sed -i 's/--discover=no/--discover=yes/' "$1"
+fi
+}
+
 # ------------------------------------------------------------------------------
 #+MAIN
 
@@ -397,6 +407,7 @@ sys_enable_guix_daemon()
             { initctl reload-configuration;
               cp "~root/.config/guix/current/lib/upstart/system/guix-daemon.conf" \
                  /etc/init/ &&
+                  configure_substitute_discovery /etc/init/guix-daemon.conf &&
                   start guix-daemon; } &&
                 _msg "${PAS}enabled Guix daemon via upstart"
             ;;
@@ -426,6 +437,9 @@ sys_enable_guix_daemon()
                        -e 's/^Environment=\(.*\)$/Environment=\1 LC_ALL=en_US.UTF-8';
               fi;
 
+              configure_substitute_discovery \
+                  /etc/systemd/system/guix-daemon.service
+
               systemctl daemon-reload &&
                   systemctl enable guix-daemon &&
                   systemctl start  guix-daemon; } &&
@@ -437,6 +451,8 @@ sys_enable_guix_daemon()
                  /etc/init.d/guix-daemon;
               chmod 775 /etc/init.d/guix-daemon;
 
+              configure_substitute_discovery /etc/init.d/guix-daemon
+
               update-rc.d guix-daemon defaults &&
                   update-rc.d guix-daemon enable &&
                   service guix-daemon start; } &&
@@ -448,6 +464,8 @@ sys_enable_guix_daemon()
                  /etc/init.d/guix-daemon;
               chmod 775 /etc/init.d/guix-daemon;
 
+              configure_substitute_discovery /etc/init.d/guix-daemon
+
               rc-update add guix-daemon default &&
                   rc-service guix-daemon start; } &&
                 _msg "${PAS}enabled Guix daemon via OpenRC"
@@ -472,7 +490,7 @@ sys_enable_guix_daemon()
 sys_authorize_build_farms()
 { # authorize the public key of the build farm
     if prompt_yes_no "Permit downloading pre-built package binaries from the \
-project's build farm? (yes/no) "; then
+project's build farm? (yes/no)"; then
         guix archive --authorize \
              < "~root/.config/guix/current/share/guix/ci.guix.gnu.org.pub" \
             && _msg "${PAS}Authorized public key for ci.guix.gnu.org"
diff --git a/etc/init.d/guix-daemon.in b/etc/init.d/guix-daemon.in
index 1cc49fed89..b7d4bb72bb 100644
--- a/etc/init.d/guix-daemon.in
+++ b/etc/init.d/guix-daemon.in
@@ -36,7 +36,7 @@ start)
       -E LC_ALL=en_US.utf8 \
       -p "/var/run/guix-daemon.pid" \
       @localstatedir@/guix/profiles/per-user/root/current-guix/bin/guix-daemon \
-      --build-users-group=guixbuild
+      --build-users-group=guixbuild --discover=no
   fi
   ;;
 stop)
diff --git a/etc/openrc/guix-daemon.in b/etc/openrc/guix-daemon.in
index 110a58b88d..eeedc2a840 100644
--- a/etc/openrc/guix-daemon.in
+++ b/etc/openrc/guix-daemon.in
@@ -20,7 +20,7 @@
 export GUIX_LOCPATH=@localstatedir@/guix/profiles/per-user/root/guix-profile/lib/locale
 export LC_ALL=en_US.utf8
 command="@localstatedir@/guix/profiles/per-user/root/current-guix/bin/guix-daemon"
-command_args="--build-users-group=guixbuild"
+command_args="--build-users-group=guixbuild --discover=no"
 command_background="yes"
 pidfile="/var/run/guix-daemon.pid"
 
-- 
2.32.0

Message #8 received at 49425 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 49425 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#49425] [PATCH] guix-install.sh: Prompt for configuring
 substitutes discovery.
Date: Tue, 06 Jul 2021 10:38:01 +0200

Hi,

(Not related to the patch :-))

On Mon, 05 Jul 2021 at 19:49, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:

> * NEWS (Distribution): Add news.

[...]

> diff --git a/NEWS b/NEWS
> index b0647b3700..ad9433a921 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -14,7 +14,9 @@ Please send Guix bug reports to bug-guix <at> gnu.org.
>  
>  * Changes in 1.4.0 (since 1.3.0)
>  ** Package management
> -   * New 'deb' format for the 'guix pack' command
> +*** New 'deb' format for the 'guix pack' command
> +** Distribution
> +*** The installation script can now configure substitute discovery
>  
>  * Changes in 1.3.0 (since 1.2.0)
>  ** Package management

From my point of view, this is a good practise. :-)  Especially when
preparing a new release. ;-)

Now, is it a standard practise?  I mean, is it asked to committers to do
it more systematically for the changes that require it?

Cheers,
simon

Message #11 received at 49425 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 49425 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#49425: [PATCH] guix-install.sh: Prompt for configuring
 substitutes discovery.
Date: Tue, 06 Jul 2021 16:40:06 +0200

Hi,

zimoun <zimon.toutoune <at> gmail.com> skribis:

>> diff --git a/NEWS b/NEWS
>> index b0647b3700..ad9433a921 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -14,7 +14,9 @@ Please send Guix bug reports to bug-guix <at> gnu.org.
>>  
>>  * Changes in 1.4.0 (since 1.3.0)
>>  ** Package management
>> -   * New 'deb' format for the 'guix pack' command
>> +*** New 'deb' format for the 'guix pack' command
>> +** Distribution
>> +*** The installation script can now configure substitute discovery
>>  
>>  * Changes in 1.3.0 (since 1.2.0)
>>  ** Package management
>
>>From my point of view, this is a good practise. :-)  Especially when
> preparing a new release. ;-)
>
> Now, is it a standard practise?  I mean, is it asked to committers to do
> it more systematically for the changes that require it?

I think we should aim to make it standard practice!

The worst that could happen is that we’d end up with an overly detailed
‘NEWS’, but that’s probably better than the other way around.

Ludo’.

Message #14 received at 49425 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 49425 <at> debbugs.gnu.org
Subject: Re: bug#49425: [PATCH] guix-install.sh: Prompt for configuring
 substitutes discovery.
Date: Tue, 06 Jul 2021 16:44:48 +0200

Hi,

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

> +# Configure substitute discovery according to user's preferences.
> +# $1 is the installed service file to edit.
> +configure_substitute_discovery() {
> +if grep -q -- '--discover=no' "$1" && \
> +        prompt_yes_no "Would you like to enable automatic substitute \
> +discovery? (yes/no)"; then
> +    sed -i 's/--discover=no/--discover=yes/' "$1"
> +fi
> +}

Indentation is off, compared to the other functions.

Should the message say a bit more, similar to that in (gnu installer
newt substitutes)?

Otherwise LGTM!

Note that someone running “yes | ./guix-install.sh” will now have
substitutes enabled (whereas it’s off by default in
<guix-configuration>).  I suppose that’s fine though.

Thanks,
Ludo’.

Message #17 received at 49425 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 49425 <at> debbugs.gnu.org, zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#49425: [PATCH] guix-install.sh: Prompt for configuring
 substitutes discovery.
Date: Tue, 06 Jul 2021 12:16:08 -0400

Hello,

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

> Hi,
>
> zimoun <zimon.toutoune <at> gmail.com> skribis:
>
>>> diff --git a/NEWS b/NEWS
>>> index b0647b3700..ad9433a921 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -14,7 +14,9 @@ Please send Guix bug reports to bug-guix <at> gnu.org.
>>>  
>>>  * Changes in 1.4.0 (since 1.3.0)
>>>  ** Package management
>>> -   * New 'deb' format for the 'guix pack' command
>>> +*** New 'deb' format for the 'guix pack' command
>>> +** Distribution
>>> +*** The installation script can now configure substitute discovery
>>>  
>>>  * Changes in 1.3.0 (since 1.2.0)
>>>  ** Package management
>>
>>>From my point of view, this is a good practise. :-)  Especially when
>> preparing a new release. ;-)
>>
>> Now, is it a standard practise?  I mean, is it asked to committers to do
>> it more systematically for the changes that require it?
>
> I think we should aim to make it standard practice!
>
> The worst that could happen is that we’d end up with an overly detailed
> ‘NEWS’, but that’s probably better than the other way around.

It'd be useful indeed to have the NEWS file populated by the very people
who authored the features.  We could at least mention that doing so is
useful and appreciated in our contributing section.

Maxim

Message #22 received at 49425-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 49425-done <at> debbugs.gnu.org
Subject: Re: bug#49425: [PATCH] guix-install.sh: Prompt for configuring
 substitutes discovery.
Date: Wed, 07 Jul 2021 23:58:43 -0400

Hi!

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

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> +# Configure substitute discovery according to user's preferences.
>> +# $1 is the installed service file to edit.
>> +configure_substitute_discovery() {
>> +if grep -q -- '--discover=no' "$1" && \
>> +        prompt_yes_no "Would you like to enable automatic substitute \
>> +discovery? (yes/no)"; then
>> +    sed -i 's/--discover=no/--discover=yes/' "$1"
>> +fi
>> +}
>
> Indentation is off, compared to the other functions.

Good catch.

> Should the message say a bit more, similar to that in (gnu installer
> newt substitutes)?
>
> Otherwise LGTM!

I went with:

--8<---------------cut here---------------start------------->8---
-if grep -q -- '--discover=no' "$1" && \
-        prompt_yes_no "Would you like to enable automatic substitute \
-discovery? (yes/no)"; then
-    sed -i 's/--discover=no/--discover=yes/' "$1"
-fi
+    if grep -q -- '--discover=no' "$1" && \
+            prompt_yes_no "Would you like the Guix daemon to automatically \
+discover substitute servers on the local network? (yes/no)"; then
+        sed -i 's/--discover=no/--discover=yes/' "$1"
+    fi
 }
--8<---------------cut here---------------end--------------->8---

and pushed as commit 4cbe0127a6.

> Note that someone running “yes | ./guix-install.sh” will now have
> substitutes enabled (whereas it’s off by default in
> <guix-configuration>).  I suppose that’s fine though.

I think it's OK too.  I like to think that someone typing yes |
./guix-install.sh wants the most complete setup, with all the bells and
whistles ("yes, all that" :-)).

Thanks!

Maxim

Previous Next

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