GNU bug report logs - #74962
[PATCH] etc/guix-install.sh: Explicit shebang to use /usr/bin/env.

Previous Next

Package: guix-patches;

Reported by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

Date: Thu, 19 Dec 2024 06:59:02 UTC

Severity: normal

Tags: patch

To reply to this bug, email your comments to 74962 AT debbugs.gnu.org.

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to ludo <at> gnu.org, guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 06:59:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Maxim Cournoyer <maxim.cournoyer <at> gmail.com>:
New bug report received and forwarded. Copy sent to ludo <at> gnu.org, guix-patches <at> gnu.org. (Thu, 19 Dec 2024 06:59:02 GMT) Full text and rfc822 format available.

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] etc/guix-install.sh: Explicit shebang to use /usr/bin/env.
Date: Thu, 19 Dec 2024 15:57:53 +0900
Having an explicit shebang tells something useful: we depend on Bash.  Tools
such as shellcheck make use of it.  The original technical reason for avoiding
/usr/bin/env is no more (Guix System lacking it).

* etc/guix-install.sh: Adjust shebang.  Remove conditional 'exec bash' further
block below.

Change-Id: I3c92a9e58fe11610dfbf74dbbd4b1ac8852abcf0
---
 etc/guix-install.sh | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index f07b2741bb..481eb6f12a 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/usr/bin/env bash
 # GNU Guix --- Functional package management for GNU
 # Copyright © 2017 sharlatan <sharlatanus <at> gmail.com>
 # Copyright © 2018 Ricardo Wurmus <rekado <at> elephly.net>
@@ -9,7 +9,7 @@
 # Copyright © 2020 Daniel Brooks <db48x <at> db48x.net>
 # Copyright © 2021 Jakub Kądziołka <kuba <at> kadziolka.net>
 # Copyright © 2021 Chris Marusich <cmmarusich <at> gmail.com>
-# Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+# Copyright © 2021, 2022, 2023, 2024 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 # Copyright © 2022 Prafulla Giri <prafulla.giri <at> protonmail.com>
 # Copyright © 2023 Andrew Tropin <andrew <at> trop.in>
 # Copyright © 2020 David A. Redick <david.a.redick <at> gmail.com>
@@ -31,10 +31,8 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
-
-# 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
@@ -50,11 +48,6 @@
 # installation required the user to extract Guix packs under /gnu to
 # satisfy its dependencies.
 
-if [ "x$BASH_VERSION" = "x" ]
-then
-    exec bash "$0" "$@"
-fi
-
 set -eo pipefail
 
 [ "$UID" -eq 0 ] || { echo "This script must be run as root."; exit 1; }

base-commit: 17c0aa6192f6a90c227e92720f2d63240996d0d4
-- 
2.46.0





Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 07:20:04 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74962 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 1/3] etc/teams.scm: Add etc/guix-install.sh to installer team
 scope.
Date: Thu, 19 Dec 2024 16:17:50 +0900
* etc/teams.scm (installer) <#:scope>: Add "etc/guix-install.sh".

Change-Id: I351476c3150c25c8d403e9a4ff0a05c98b9d1821
---
 etc/teams.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/etc/teams.scm b/etc/teams.scm
index 6b492f1d4b..fb6a30ed2f 100755
--- a/etc/teams.scm
+++ b/etc/teams.scm
@@ -10,7 +10,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2022-2024 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2022 Mathieu Othacehe <othacehe <at> gnu.org>
-;;; Copyright © 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2022, 2023, 2024 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2022 Simon Tournier <zimon.toutoune <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -523,7 +523,8 @@ (define-team translations
 (define-team installer
   (team 'installer
         #:name "Installer script and system installer"
-        #:scope (list (make-regexp* "^gnu/installer(\\.scm$|/)"))))
+        #:scope (list (make-regexp* "^gnu/installer(\\.scm$|/)")
+                      "etc/guix-install.sh")))
 
 (define-team home
   (team 'home

base-commit: 17c0aa6192f6a90c227e92720f2d63240996d0d4
-- 
2.46.0





Information forwarded to janneke <at> gnu.org, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 07:20:05 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74962 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 2/3] etc/guix-install.sh: Explicit shebang to use /usr/bin/env.
Date: Thu, 19 Dec 2024 16:17:51 +0900
Having an explicit shebang tells something useful: we depend on Bash.  Tools
such as shellcheck make use of it.  The original technical reason for avoiding
/usr/bin/env is no more (Guix System lacking it).

* etc/guix-install.sh: Adjust shebang.  Remove conditional 'exec bash' further
block below.

Change-Id: I3c92a9e58fe11610dfbf74dbbd4b1ac8852abcf0
---
 etc/guix-install.sh | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index f07b2741bb..481eb6f12a 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/usr/bin/env bash
 # GNU Guix --- Functional package management for GNU
 # Copyright © 2017 sharlatan <sharlatanus <at> gmail.com>
 # Copyright © 2018 Ricardo Wurmus <rekado <at> elephly.net>
@@ -9,7 +9,7 @@
 # Copyright © 2020 Daniel Brooks <db48x <at> db48x.net>
 # Copyright © 2021 Jakub Kądziołka <kuba <at> kadziolka.net>
 # Copyright © 2021 Chris Marusich <cmmarusich <at> gmail.com>
-# Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+# Copyright © 2021, 2022, 2023, 2024 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 # Copyright © 2022 Prafulla Giri <prafulla.giri <at> protonmail.com>
 # Copyright © 2023 Andrew Tropin <andrew <at> trop.in>
 # Copyright © 2020 David A. Redick <david.a.redick <at> gmail.com>
@@ -31,10 +31,8 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
-
-# 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
@@ -50,11 +48,6 @@
 # installation required the user to extract Guix packs under /gnu to
 # satisfy its dependencies.
 
-if [ "x$BASH_VERSION" = "x" ]
-then
-    exec bash "$0" "$@"
-fi
-
 set -eo pipefail
 
 [ "$UID" -eq 0 ] || { echo "This script must be run as root."; exit 1; }
-- 
2.46.0





Information forwarded to janneke <at> gnu.org, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 07:20:06 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74962 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 3/3] etc/guix-install.sh: Fix quoting and other issues.
Date: Thu, 19 Dec 2024 16:17:52 +0900
This change fixes most issues reported by the 'shellcheck' command.

* etc/guix-install.sh (add_init_sys_require): Use -n instead of ! -z.
(sys_create_build_user): Quote variable expansion.
(sys_delete_build_user): Likewise.
(sys_create_shell_completion): Likewise.
(sys_delete_user_profiles): Likewise.
(main): Replace $@ with $* inside string.

Change-Id: Ia88509b461b3844f2dd5abf9fb21a5b2bbb8a1e1
---
 etc/guix-install.sh | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index 481eb6f12a..2ab443d97b 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -151,7 +151,7 @@ chk_require()
 add_init_sys_require()
 { # Add the elements of FOO_INIT_SYS to REQUIRE
     local init_require="${INIT_SYS}_REQUIRE[@]"
-    if [[ ! -z "$init_require" ]]; then
+    if [[ -n "$init_require" ]]; then
         # Have to add piecemeal because ${!foo[@]} performs direct array key
         # expansion, not indirect plain array expansion.
         for r in "${!init_require}"; do
@@ -427,12 +427,12 @@ sys_create_build_user()
     for i in $(seq -w 1 10); do
         if id "guixbuilder${i}" &>/dev/null; then
             _msg "${INF}user is already in the system, reset"
-            usermod -g guixbuild -G guixbuild${KVMGROUP}     \
+            usermod -g guixbuild -G "guixbuild${KVMGROUP}"     \
                     -d /var/empty -s "$(which nologin)" \
                     -c "Guix build user $i"             \
                     "guixbuilder${i}";
         else
-            useradd -g guixbuild -G guixbuild${KVMGROUP}     \
+            useradd -g guixbuild -G "guixbuild${KVMGROUP}"     \
                     -d /var/empty -s "$(which nologin)" \
                     -c "Guix build user $i" --system    \
                     "guixbuilder${i}";
@@ -445,7 +445,7 @@ sys_delete_build_user()
 {
     for i in $(seq -w 1 10); do
         if id -u "guixbuilder${i}" &>/dev/null; then
-            userdel -f guixbuilder${i}
+            userdel -f "guixbuilder${i}"
         fi
     done
 
@@ -681,7 +681,7 @@ sys_create_shell_completion()
 
     { # Just in case
         for dir_shell in $bash_completion $zsh_completion $fish_completion; do
-            [ -d "$dir_shell" ] || mkdir -p $dir_shell
+            [ -d "$dir_shell" ] || mkdir -p "$dir_shell"
         done;
 
         ln -sf ${var_guix}/etc/bash_completion.d/* "$bash_completion";
@@ -747,10 +747,10 @@ sys_delete_user_profiles()
     rm -rf ~root/.cache/guix
 
     _msg "${INF}removing .guix-profile, .cache/guix and .config/guix of all /home users"
-    for user in `ls -1 /home`; do
-        rm -f /home/$user/.guix-profile
-        rm -rf /home/$user/.cache/guix
-        rm -rf /home/$user/.config/guix
+    for user in /home/*; do
+        rm -f "/home/$user/.guix-profile"
+        rm -rf "/home/$user/.cache/guix"
+        rm -rf "/home/$user/.config/guix"
     done
 }
 
@@ -898,7 +898,7 @@ main()
         if [ '--uninstall' = "${uninstall_flag}" ]; then
             main_uninstall
         else
-            echo "unsupported parameters: $@"
+            echo "unsupported parameters: $*"
             exit 1
         fi
     fi
-- 
2.46.0





Information forwarded to janneke <at> gnu.org, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 07:48:02 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74962 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3 2/5] etc/guix-install.sh: Explicit shebang to use
 /usr/bin/env.
Date: Thu, 19 Dec 2024 16:45:30 +0900
Having an explicit shebang tells something useful: we depend on Bash.  Tools
such as shellcheck make use of it.  The original technical reason for avoiding
/usr/bin/env is no more (Guix System lacking it).

* etc/guix-install.sh: Adjust shebang.  Remove conditional 'exec bash' further
block below.

Change-Id: I3c92a9e58fe11610dfbf74dbbd4b1ac8852abcf0
---

(no changes since v1)

 etc/guix-install.sh | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index f07b2741bb..481eb6f12a 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/usr/bin/env bash
 # GNU Guix --- Functional package management for GNU
 # Copyright © 2017 sharlatan <sharlatanus <at> gmail.com>
 # Copyright © 2018 Ricardo Wurmus <rekado <at> elephly.net>
@@ -9,7 +9,7 @@
 # Copyright © 2020 Daniel Brooks <db48x <at> db48x.net>
 # Copyright © 2021 Jakub Kądziołka <kuba <at> kadziolka.net>
 # Copyright © 2021 Chris Marusich <cmmarusich <at> gmail.com>
-# Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+# Copyright © 2021, 2022, 2023, 2024 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 # Copyright © 2022 Prafulla Giri <prafulla.giri <at> protonmail.com>
 # Copyright © 2023 Andrew Tropin <andrew <at> trop.in>
 # Copyright © 2020 David A. Redick <david.a.redick <at> gmail.com>
@@ -31,10 +31,8 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
-
-# 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
@@ -50,11 +48,6 @@
 # installation required the user to extract Guix packs under /gnu to
 # satisfy its dependencies.
 
-if [ "x$BASH_VERSION" = "x" ]
-then
-    exec bash "$0" "$@"
-fi
-
 set -eo pipefail
 
 [ "$UID" -eq 0 ] || { echo "This script must be run as root."; exit 1; }
-- 
2.46.0





Information forwarded to janneke <at> gnu.org, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 07:48:02 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74962 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3 3/5] etc/guix-install.sh: Fix quoting and other issues.
Date: Thu, 19 Dec 2024 16:45:31 +0900
This change fixes most issues reported by the 'shellcheck' command.

* etc/guix-install.sh (add_init_sys_require): Use -n instead of ! -z.
(sys_create_build_user): Quote variable expansion.
(sys_delete_build_user): Likewise.
(sys_create_shell_completion): Likewise.
(sys_delete_user_profiles): Likewise.
(sys_delete_guix_daemon): Explicitly access first array item.
(sys_create_store): Update shellcheck code to ignore.
(SYSV_INIT_REQUIRE): Ignore unused warning.
(sys_customize_bashrc): Ignore warnings due to using variables inside a
literal.
(main): Replace $@ with $* inside string.

Change-Id: Ia88509b461b3844f2dd5abf9fb21a5b2bbb8a1e1
---

Changes in v3:
 - Add comments to avoid remaining shellcheck warnings.

 etc/guix-install.sh | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index 481eb6f12a..8d3d9d224b 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -77,6 +77,7 @@ REQUIRE=(
 
 # Add variables using form FOO_INIT_REQUIRE when init system FOO dependencies
 # should be checked.
+# shellcheck disable=SC2034  # the variable name is computed
 SYSV_INIT_REQUIRE=(
     "daemonize"
 )
@@ -151,7 +152,7 @@ chk_require()
 add_init_sys_require()
 { # Add the elements of FOO_INIT_SYS to REQUIRE
     local init_require="${INIT_SYS}_REQUIRE[@]"
-    if [[ ! -z "$init_require" ]]; then
+    if [[ -n "$init_require" ]]; then
         # Have to add piecemeal because ${!foo[@]} performs direct array key
         # expansion, not indirect plain array expansion.
         for r in "${!init_require}"; do
@@ -390,7 +391,7 @@ sys_create_store()
        ~root/.config/guix/current
 
     GUIX_PROFILE=~root/.config/guix/current
-    # shellcheck disable=SC1090
+    # shellcheck disable=SC1091
     source "${GUIX_PROFILE}/etc/profile"
     _msg "${PAS}activated root profile at ${GUIX_PROFILE}"
 }
@@ -427,12 +428,12 @@ sys_create_build_user()
     for i in $(seq -w 1 10); do
         if id "guixbuilder${i}" &>/dev/null; then
             _msg "${INF}user is already in the system, reset"
-            usermod -g guixbuild -G guixbuild${KVMGROUP}     \
+            usermod -g guixbuild -G "guixbuild${KVMGROUP}"     \
                     -d /var/empty -s "$(which nologin)" \
                     -c "Guix build user $i"             \
                     "guixbuilder${i}";
         else
-            useradd -g guixbuild -G guixbuild${KVMGROUP}     \
+            useradd -g guixbuild -G "guixbuild${KVMGROUP}"     \
                     -d /var/empty -s "$(which nologin)" \
                     -c "Guix build user $i" --system    \
                     "guixbuilder${i}";
@@ -445,7 +446,7 @@ sys_delete_build_user()
 {
     for i in $(seq -w 1 10); do
         if id -u "guixbuilder${i}" &>/dev/null; then
-            userdel -f guixbuilder${i}
+            userdel -f "guixbuilder${i}"
         fi
     done
 
@@ -551,7 +552,7 @@ sys_delete_guix_daemon()
     local local_bin
     local var_guix
 
-    _debug "--- [ $FUNCNAME ] ---"
+    _debug "--- [ ${FUNCNAME[0]} ] ---"
 
     info_path="/usr/local/share/info"
     local_bin="/usr/local/bin"
@@ -681,7 +682,7 @@ sys_create_shell_completion()
 
     { # Just in case
         for dir_shell in $bash_completion $zsh_completion $fish_completion; do
-            [ -d "$dir_shell" ] || mkdir -p $dir_shell
+            [ -d "$dir_shell" ] || mkdir -p "$dir_shell"
         done;
 
         ln -sf ${var_guix}/etc/bash_completion.d/* "$bash_completion";
@@ -696,8 +697,10 @@ sys_customize_bashrc()
 
     for bashrc in /home/*/.bashrc /root/.bashrc; do
         test -f "$bashrc" || continue
+        # shellcheck disable=SC2016  # checking for literal $GUIX_ENVIRONMENT
         grep -Fq '$GUIX_ENVIRONMENT' "$bashrc" && continue
         cp "${bashrc}" "${bashrc}.bak"
+        # shellcheck disable=SC2016,SC2028  # that's also a string literal
         echo '
 # Automatically added by the Guix install script.
 if [ -n "$GUIX_ENVIRONMENT" ]; then
@@ -747,10 +750,10 @@ sys_delete_user_profiles()
     rm -rf ~root/.cache/guix
 
     _msg "${INF}removing .guix-profile, .cache/guix and .config/guix of all /home users"
-    for user in `ls -1 /home`; do
-        rm -f /home/$user/.guix-profile
-        rm -rf /home/$user/.cache/guix
-        rm -rf /home/$user/.config/guix
+    for user in /home/*; do
+        rm -f "/home/$user/.guix-profile"
+        rm -rf "/home/$user/.cache/guix"
+        rm -rf "/home/$user/.config/guix"
     done
 }
 
@@ -898,7 +901,7 @@ main()
         if [ '--uninstall' = "${uninstall_flag}" ]; then
             main_uninstall
         else
-            echo "unsupported parameters: $@"
+            echo "unsupported parameters: $*"
             exit 1
         fi
     fi
-- 
2.46.0





Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 07:48:03 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74962 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3 1/5] etc/teams.scm: Add etc/guix-install.sh to installer
 team scope.
Date: Thu, 19 Dec 2024 16:45:29 +0900
* etc/teams.scm (installer) <#:scope>: Add "etc/guix-install.sh".

Change-Id: I351476c3150c25c8d403e9a4ff0a05c98b9d1821
---

(no changes since v2)

Changes in v2:
 - New.

 etc/teams.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/etc/teams.scm b/etc/teams.scm
index 6b492f1d4b..fb6a30ed2f 100755
--- a/etc/teams.scm
+++ b/etc/teams.scm
@@ -10,7 +10,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2022-2024 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2022 Mathieu Othacehe <othacehe <at> gnu.org>
-;;; Copyright © 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2022, 2023, 2024 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2022 Simon Tournier <zimon.toutoune <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -523,7 +523,8 @@ (define-team translations
 (define-team installer
   (team 'installer
         #:name "Installer script and system installer"
-        #:scope (list (make-regexp* "^gnu/installer(\\.scm$|/)"))))
+        #:scope (list (make-regexp* "^gnu/installer(\\.scm$|/)")
+                      "etc/guix-install.sh")))
 
 (define-team home
   (team 'home

base-commit: 17c0aa6192f6a90c227e92720f2d63240996d0d4
-- 
2.46.0





Information forwarded to janneke <at> gnu.org, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 07:48:03 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74962 <at> debbugs.gnu.org
Cc: Simon Josefsson <simon <at> josefsson.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3 4/5] etc/guix-install.sh: Remove 'which' commands from
 requirements.
Date: Thu, 19 Dec 2024 16:45:32 +0900
* etc/guix-install.sh (REQUIRE): Remove "which".  Add "nologin".
(sys_create_build_user): Use 'type' instead of 'which'.

Fixes: <https://issues.guix.gnu.org/74952>
Reported-by: Simon Josefsson <simon <at> josefsson.org>
Change-Id: I0675716bab3fc22d3289ee7af2cb0ab33a1cee71
---

Changes in v3:
 - New.

 etc/guix-install.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index 8d3d9d224b..fb22287cf4 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -58,7 +58,7 @@ REQUIRE=(
     "wget"
     "gpg"
     "grep"
-    "which"
+    "nologin"
     "sed"
     "sort"
     "getent"
@@ -429,12 +429,12 @@ sys_create_build_user()
         if id "guixbuilder${i}" &>/dev/null; then
             _msg "${INF}user is already in the system, reset"
             usermod -g guixbuild -G "guixbuild${KVMGROUP}"     \
-                    -d /var/empty -s "$(which nologin)" \
+                    -d /var/empty -s "$(type -P nologin)" \
                     -c "Guix build user $i"             \
                     "guixbuilder${i}";
         else
             useradd -g guixbuild -G "guixbuild${KVMGROUP}"     \
-                    -d /var/empty -s "$(which nologin)" \
+                    -d /var/empty -s "$(type -P nologin)" \
                     -c "Guix build user $i" --system    \
                     "guixbuilder${i}";
             _msg "${PAS}user added <guixbuilder${i}>"
-- 
2.46.0





Information forwarded to janneke <at> gnu.org, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 07:48:04 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74962 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3 5/5] etc/guix-install.sh: Sort requirements.
Date: Thu, 19 Dec 2024 16:45:33 +0900
* etc/guix-install.sh (REQUIRE): Sort.

Change-Id: I59c57da31cd3846cf21810d5978d7d32516e0868
---

Changes in v3:
 - New

 etc/guix-install.sh | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index fb22287cf4..59d0d3820e 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -53,25 +53,25 @@ set -eo pipefail
 [ "$UID" -eq 0 ] || { echo "This script must be run as root."; exit 1; }
 
 REQUIRE=(
+    "chmod"
     "dirname"
-    "readlink"
-    "wget"
+    "getent"
     "gpg"
     "grep"
+    "groupadd"
+    "groupdel"
+    "mktemp"
     "nologin"
+    "readlink"
+    "rm"
     "sed"
     "sort"
-    "getent"
-    "mktemp"
-    "rm"
-    "chmod"
+    "tail"
+    "tr"
     "uname"
-    "groupadd"
-    "groupdel"
     "useradd"
     "userdel"
-    "tail"
-    "tr"
+    "wget"
     "xz"
 )
 
-- 
2.46.0





Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 08:19:02 GMT) Full text and rfc822 format available.

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

From: Janneke Nieuwenhuizen <janneke <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 Ludovic Courtès <ludo <at> gnu.org>, 74962 <at> debbugs.gnu.org
Subject: Re: [bug#74962] [PATCH 2/3] etc/guix-install.sh: Explicit shebang
 to use /usr/bin/env.
Date: Thu, 19 Dec 2024 09:18:15 +0100
Maxim Cournoyer writes:

Hi Maxim,

> * etc/guix-install.sh: Adjust shebang.  Remove conditional 'exec bash' further
> block below.
>
> Change-Id: I3c92a9e58fe11610dfbf74dbbd4b1ac8852abcf0
> ---
>  etc/guix-install.sh | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/etc/guix-install.sh b/etc/guix-install.sh
> index f07b2741bb..481eb6f12a 100755
[..]
> -if [ "x$BASH_VERSION" = "x" ]
> -then
> -    exec bash "$0" "$@"
> -fi
> -

+1 for the shebang but I think we want to keep this for people running:
sh install.sh, eg on Debian where sh is dash?

Greetings,
Janneke

-- 
Janneke Nieuwenhuizen <janneke <at> gnu.org>  | GNU LilyPond https://LilyPond.org
Freelance IT https://www.JoyOfSource.com | Avatar® https://AvatarAcademy.com




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 19 Dec 2024 12:50:02 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Janneke Nieuwenhuizen <janneke <at> gnu.org>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 Ludovic Courtès <ludo <at> gnu.org>, 74962 <at> debbugs.gnu.org
Subject: Re: [bug#74962] [PATCH 2/3] etc/guix-install.sh: Explicit shebang
 to use /usr/bin/env.
Date: Thu, 19 Dec 2024 21:48:14 +0900
Hi,

Janneke Nieuwenhuizen <janneke <at> gnu.org> writes:

> Maxim Cournoyer writes:
>
> Hi Maxim,
>
>> * etc/guix-install.sh: Adjust shebang.  Remove conditional 'exec bash' further
>> block below.
>>
>> Change-Id: I3c92a9e58fe11610dfbf74dbbd4b1ac8852abcf0
>> ---
>>  etc/guix-install.sh | 15 ++++-----------
>>  1 file changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/etc/guix-install.sh b/etc/guix-install.sh
>> index f07b2741bb..481eb6f12a 100755
> [..]
>> -if [ "x$BASH_VERSION" = "x" ]
>> -then
>> -    exec bash "$0" "$@"
>> -fi
>> -
>
> +1 for the shebang but I think we want to keep this for people running:
> sh install.sh, eg on Debian where sh is dash?

Hm.  Good point.  I'll restore it in a v4, but I'll give it some time
before I do, so as to avoid sending yet another series in a short time.

-- 
Thanks,
Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 26 Dec 2024 10:55:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 74962 <at> debbugs.gnu.org
Subject: Re: [bug#74962] [PATCH] etc/guix-install.sh: Explicit shebang to
 use /usr/bin/env.
Date: Thu, 26 Dec 2024 11:54:51 +0100
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> Having an explicit shebang tells something useful: we depend on Bash.  Tools
> such as shellcheck make use of it.  The original technical reason for avoiding
> /usr/bin/env is no more (Guix System lacking it).
>
> * etc/guix-install.sh: Adjust shebang.  Remove conditional 'exec bash' further
> block below.
>
> Change-Id: I3c92a9e58fe11610dfbf74dbbd4b1ac8852abcf0

LGTM!




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 26 Dec 2024 10:56:01 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 74962 <at> debbugs.gnu.org, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH 3/3] etc/guix-install.sh: Fix quoting and
 other issues.
Date: Thu, 26 Dec 2024 11:55:37 +0100
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> This change fixes most issues reported by the 'shellcheck' command.
>
> * etc/guix-install.sh (add_init_sys_require): Use -n instead of ! -z.
> (sys_create_build_user): Quote variable expansion.
> (sys_delete_build_user): Likewise.
> (sys_create_shell_completion): Likewise.
> (sys_delete_user_profiles): Likewise.
> (main): Replace $@ with $* inside string.
>
> Change-Id: Ia88509b461b3844f2dd5abf9fb21a5b2bbb8a1e1

LGTM.




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 26 Dec 2024 10:57:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Simon Josefsson <simon <at> josefsson.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 Mathieu Othacehe <othacehe <at> gnu.org>, 74962 <at> debbugs.gnu.org,
 Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH v3 4/5] etc/guix-install.sh: Remove 'which'
 commands from requirements.
Date: Thu, 26 Dec 2024 11:55:52 +0100
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> * etc/guix-install.sh (REQUIRE): Remove "which".  Add "nologin".
> (sys_create_build_user): Use 'type' instead of 'which'.
>
> Fixes: <https://issues.guix.gnu.org/74952>
> Reported-by: Simon Josefsson <simon <at> josefsson.org>
> Change-Id: I0675716bab3fc22d3289ee7af2cb0ab33a1cee71

LGTM.




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 26 Dec 2024 10:57:03 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 74962 <at> debbugs.gnu.org, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH v3 3/5] etc/guix-install.sh: Fix quoting and
 other issues.
Date: Thu, 26 Dec 2024 11:56:15 +0100
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> This change fixes most issues reported by the 'shellcheck' command.
>
> * etc/guix-install.sh (add_init_sys_require): Use -n instead of ! -z.
> (sys_create_build_user): Quote variable expansion.
> (sys_delete_build_user): Likewise.
> (sys_create_shell_completion): Likewise.
> (sys_delete_user_profiles): Likewise.
> (sys_delete_guix_daemon): Explicitly access first array item.
> (sys_create_store): Update shellcheck code to ignore.
> (SYSV_INIT_REQUIRE): Ignore unused warning.
> (sys_customize_bashrc): Ignore warnings due to using variables inside a
> literal.
> (main): Replace $@ with $* inside string.
>
> Change-Id: Ia88509b461b3844f2dd5abf9fb21a5b2bbb8a1e1

LGTM.




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 26 Dec 2024 11:01:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 74962 <at> debbugs.gnu.org, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH v3 5/5] etc/guix-install.sh: Sort requirements.
Date: Thu, 26 Dec 2024 12:00:15 +0100
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> * etc/guix-install.sh (REQUIRE): Sort.
>
> Change-Id: I59c57da31cd3846cf21810d5978d7d32516e0868

LGTM.




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Thu, 26 Dec 2024 12:37:02 GMT) Full text and rfc822 format available.

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

From: Simon Josefsson <simon <at> josefsson.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 74962 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>,
 Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH v3 4/5] etc/guix-install.sh: Remove 'which'
 commands from requirements.
Date: Thu, 26 Dec 2024 13:34:21 +0100
[Message part 1 (text/plain, inline)]
Ludovic Courtès <ludo <at> gnu.org> writes:

> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> * etc/guix-install.sh (REQUIRE): Remove "which".  Add "nologin".
>> (sys_create_build_user): Use 'type' instead of 'which'.
>>
>> Fixes: <https://issues.guix.gnu.org/74952>
>> Reported-by: Simon Josefsson <simon <at> josefsson.org>
>> Change-Id: I0675716bab3fc22d3289ee7af2cb0ab33a1cee71
>
> LGTM.

Using 'type -P' is not POSIX and neither /bin/dash nor /bin/gash
supports it.  It seems like a GNU bash extension.  Is that okay?

The snippet ends up in the manual as recommendations for users to run on
different operating systems.  We may want to assume GNU bash to favor
it, but I'm not sure if that is really helping users.

If 'type -P' is used, shouldn't that really be 'type -fP' to avoid shell
function expansion?  It isn't all that clear from the man page if -f is
still needed for -P or not:

https://manpages.debian.org/bookworm/bash/bash.1.en.html#type

Even so 'type' uses hashed names, do they survive sub-shell $()
execution?  If type is to be used, maybe this should be:

  $(hash -r nologin && type -Pf nologin)

My suggestion was to use 'command -v nologin' which behaviour is
standard POSIX /bin/sh.  I acknowledge that it has the trouble of
expanding to an alias if the shell had 'nologin' aliases somehow
(unlikely but not impossible).

  $(unalias nologin; command -v nologin)

It seems all of the options (which, type -P, command -v) has another
unwanted property: if 'nologin' is not available in the path, these
commands expand to the empty string, and that empty string gets passed
to 'useradd -s STR -c ...' and the user gets an ugly error message about
'-c' not being a proper shell.

I wonder what all this solves compared to hard-coding "/" as the login
shell for the guixbuild user?

Here is source code for nologin, which we seem to make some effort to
use - is this better than 'false'?

https://github.com/shadow-maint/shadow/blob/master/src/nologin.c

At least I'm happy nobody wants to keep using 'which'.

I am sorry for the rabbit hole :)

/Simon
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Sat, 28 Dec 2024 16:54:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Simon Josefsson <simon <at> josefsson.org>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 74962 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>,
 Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH v3 4/5] etc/guix-install.sh: Remove 'which'
 commands from requirements.
Date: Sat, 28 Dec 2024 17:53:44 +0100
Simon Josefsson <simon <at> josefsson.org> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>>
>>> * etc/guix-install.sh (REQUIRE): Remove "which".  Add "nologin".
>>> (sys_create_build_user): Use 'type' instead of 'which'.
>>>
>>> Fixes: <https://issues.guix.gnu.org/74952>
>>> Reported-by: Simon Josefsson <simon <at> josefsson.org>
>>> Change-Id: I0675716bab3fc22d3289ee7af2cb0ab33a1cee71
>>
>> LGTM.
>
> Using 'type -P' is not POSIX and neither /bin/dash nor /bin/gash
> supports it.  It seems like a GNU bash extension.  Is that okay?

Oh, not great.  From what you write, I’m not sure what to conclude;
just skip this patch and be done with it?

Thanks!

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Sun, 29 Dec 2024 02:29:02 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Simon Josefsson <simon <at> josefsson.org>, Josselin Poiret <dev <at> jpoiret.xyz>,
 Mathieu Othacehe <othacehe <at> gnu.org>, 74962 <at> debbugs.gnu.org,
 Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH v3 4/5] etc/guix-install.sh: Remove 'which'
 commands from requirements.
Date: Sun, 29 Dec 2024 11:26:57 +0900
Hi,

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

> Simon Josefsson <simon <at> josefsson.org> skribis:
>
>> Ludovic Courtès <ludo <at> gnu.org> writes:
>>
>>> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>>>
>>>> * etc/guix-install.sh (REQUIRE): Remove "which".  Add "nologin".
>>>> (sys_create_build_user): Use 'type' instead of 'which'.
>>>>
>>>> Fixes: <https://issues.guix.gnu.org/74952>
>>>> Reported-by: Simon Josefsson <simon <at> josefsson.org>
>>>> Change-Id: I0675716bab3fc22d3289ee7af2cb0ab33a1cee71
>>>
>>> LGTM.
>>
>> Using 'type -P' is not POSIX and neither /bin/dash nor /bin/gash
>> supports it.  It seems like a GNU bash extension.  Is that okay?
>
> Oh, not great.  From what you write, I’m not sure what to conclude;
> just skip this patch and be done with it?

We currently use other Bash-specific features, so I think it's fine to
embrace the Bash requirement instead of shying away from it.

If we decide that we don't want Bash as a requirement at some point,
we'll have to change a bunch of things; one of them would be to no
longer make use of arrays since POSIX shells don't have them, for
example.

-- 
Thanks,
Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Sun, 29 Dec 2024 02:36:01 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Simon Josefsson <simon <at> josefsson.org>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>,
 Ludovic Courtès <ludo <at> gnu.org>, 74962 <at> debbugs.gnu.org,
 Josselin Poiret <dev <at> jpoiret.xyz>, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH v3 4/5] etc/guix-install.sh: Remove 'which'
 commands from requirements.
Date: Sun, 29 Dec 2024 11:34:17 +0900
Hi Simon,

Simon Josefsson <simon <at> josefsson.org> writes:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>>
>>> * etc/guix-install.sh (REQUIRE): Remove "which".  Add "nologin".
>>> (sys_create_build_user): Use 'type' instead of 'which'.
>>>
>>> Fixes: <https://issues.guix.gnu.org/74952>
>>> Reported-by: Simon Josefsson <simon <at> josefsson.org>
>>> Change-Id: I0675716bab3fc22d3289ee7af2cb0ab33a1cee71
>>
>> LGTM.
>
> Using 'type -P' is not POSIX and neither /bin/dash nor /bin/gash
> supports it.  It seems like a GNU bash extension.  Is that okay?

I think it's OK, since we currently mandate Bash, but we could use
'command -v the-command > /dev/null' for the same result, which *is*
POSIX, so perhaps we should use that instead.

> The snippet ends up in the manual as recommendations for users to run on
> different operating systems.  We may want to assume GNU bash to favor
> it, but I'm not sure if that is really helping users.
>
> If 'type -P' is used, shouldn't that really be 'type -fP' to avoid shell
> function expansion?  It isn't all that clear from the man page if -f is
> still needed for -P or not:
>
> https://manpages.debian.org/bookworm/bash/bash.1.en.html#type

I think we'd have to use -f if we want to guard against shell functions
being found instead; but maybe then it's simpler and clearer to just use
'command -v' as I mentioned above.

> Even so 'type' uses hashed names, do they survive sub-shell $()
> execution?  If type is to be used, maybe this should be:
>
>   $(hash -r nologin && type -Pf nologin)
>
> My suggestion was to use 'command -v nologin' which behaviour is
> standard POSIX /bin/sh.  I acknowledge that it has the trouble of
> expanding to an alias if the shell had 'nologin' aliases somehow
> (unlikely but not impossible).

I agree; I'll make the change.  Perhaps adjust the other 'type' usages
also (there was only 2).

>   $(unalias nologin; command -v nologin)
>
> It seems all of the options (which, type -P, command -v) has another
> unwanted property: if 'nologin' is not available in the path, these
> commands expand to the empty string, and that empty string gets passed
> to 'useradd -s STR -c ...' and the user gets an ugly error message about
> '-c' not being a proper shell.

Yuck.

> I wonder what all this solves compared to hard-coding "/" as the login
> shell for the guixbuild user?
>
> Here is source code for nologin, which we seem to make some effort to
> use - is this better than 'false'?
>
> https://github.com/shadow-maint/shadow/blob/master/src/nologin.c

It seems marginally better than using 'false' in that it logs something
to syslog when a login is attempted and fail :-).

> At least I'm happy nobody wants to keep using 'which'.
>
> I am sorry for the rabbit hole :)

Thanks for the comments.  I'll send a reworked version.

-- 
Thanks,
Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#74962; Package guix-patches. (Sun, 29 Dec 2024 02:36:02 GMT) Full text and rfc822 format available.

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

From: Simon Josefsson <simon <at> josefsson.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>,
 Ludovic Courtès <ludo <at> gnu.org>, 74962 <at> debbugs.gnu.org,
 Josselin Poiret <dev <at> jpoiret.xyz>, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: [bug#74962] [PATCH v3 4/5] etc/guix-install.sh: Remove 'which'
 commands from requirements.
Date: Sun, 29 Dec 2024 03:33:46 +0100
[Message part 1 (text/plain, inline)]
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Hi,
>
> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Simon Josefsson <simon <at> josefsson.org> skribis:
>>
>>> Ludovic Courtès <ludo <at> gnu.org> writes:
>>>
>>>> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>>>>
>>>>> * etc/guix-install.sh (REQUIRE): Remove "which".  Add "nologin".
>>>>> (sys_create_build_user): Use 'type' instead of 'which'.
>>>>>
>>>>> Fixes: <https://issues.guix.gnu.org/74952>
>>>>> Reported-by: Simon Josefsson <simon <at> josefsson.org>
>>>>> Change-Id: I0675716bab3fc22d3289ee7af2cb0ab33a1cee71
>>>>
>>>> LGTM.
>>>
>>> Using 'type -P' is not POSIX and neither /bin/dash nor /bin/gash
>>> supports it.  It seems like a GNU bash extension.  Is that okay?
>>
>> Oh, not great.  From what you write, I’m not sure what to conclude;
>> just skip this patch and be done with it?
>
> We currently use other Bash-specific features, so I think it's fine to
> embrace the Bash requirement instead of shying away from it.
>
> If we decide that we don't want Bash as a requirement at some point,
> we'll have to change a bunch of things; one of them would be to no
> longer make use of arrays since POSIX shells don't have them, for
> example.

There is a difference to use bashisms in code in Guix intended to be run
on bash, and code we have in the manual that is suggested to be used on
other operating system as part of the Guix installation process.

There appears to be no perfect solution here.  I think 'command -v
nologin' is the closest.  Or just keep the code as-is and use 'which',
but that caused my initial problem (lack of 'which').

I'd like to second-guess why we even bother with using "nologin" instead
of simply hard-coding "/bin/false" or why not "/" which I suppose is not
a executable shell on any system.

/Simon
[signature.asc (application/pgp-signature, inline)]

This bug report was last modified 165 days ago.

Previous Next


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