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

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

[Maxim Cournoyer]

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@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

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

[zimoun]

Hi,

(Not related to the patch :-))

On Mon, 05 Jul 2021 at 19:49, Maxim Cournoyer <maxim.cournoyer@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@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

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

[Ludovic Courtès]

Hi,

zimoun <zimon.toutoune@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@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’.

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

[Ludovic Courtès]

Hi,

Maxim Cournoyer <maxim.cournoyer@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’.

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

[Maxim Cournoyer]

Hello,

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

> Hi,
>
> zimoun <zimon.toutoune@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@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

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

[Maxim Cournoyer]

Hi!

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

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@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