Subject: Re: [PATCH] guix-install.sh: Add message about Info reader.

From: Maxim Cournoyer <[email-masked]>
Date: Mon, 16 Dec 2024 11:53:18 +0900

Hi Simon,

Simon Tournier <[email-masked]> writes:

> * etc/guix-install.sh (_info): New procedure.
> (_chk_sys_nscd, main_install): Use it.

>
> Change-Id: I2cad8bc2554cd4ea88f30c8a104b7c62f2aa2e0e
> ---
>  etc/guix-install.sh | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/etc/guix-install.sh b/etc/guix-install.sh
> index f07b2741bb..08e25de238 100755
> --- a/etc/guix-install.sh
> +++ b/etc/guix-install.sh
> @@ -5,7 +5,7 @@
>  # Copyright © 2018 Efraim Flashner <[email-masked]>
>  # Copyright © 2019–2020, 2022 Tobias Geerinckx-Rice <[email-masked]>
>  # Copyright © 2020 Morgan Smith <[email-masked]>
> -# Copyright © 2020 Simon Tournier <[email-masked]>
> +# Copyright © 2020, 2024 Simon Tournier <[email-masked]>
>  # Copyright © 2020 Daniel Brooks <[email-masked]>
>  # Copyright © 2021 Jakub Kądziołka <[email-masked]>
>  # Copyright © 2021 Chris Marusich <[email-masked]>
> @@ -129,6 +129,16 @@ die()
>      exit 1
>  }
>  
> +_info()
> +{
> +    if [ "$(type -P info)" ]; then
> +        _msg "$1"
> +    else
> +        _msg "${WAR}Please install Info reader; see package 'info-reader'"
> +        _msg "$1"
> +    fi
> +}

It seems odd to me to "overload" _msg into _info that deals with some side
effect; I'd rather see this conditional explicit at the message printing
site.

Also, your test is testing for the empty string when info is not found,
not the exist status, which is wrong.  I think you meant something like:

--8<---------------cut here---------------start------------->8---
if type -P info >/dev/null then [...]; fi
--8<---------------cut here---------------end--------------->8---

But this got me curious again... could we instead automate the
installation of info post-installation?  If yes, we should also automate
the installation of glibc-locales, using prompts that the user can
accept or decline like for the other configuration choices.

That'd be more useful than asking the user to manually install things
itself.

-- 
Thanks,
Maxim