Re: [gentoo-dev] [PATCH] acct-user.eclass: improve error message when usermod fails

[Florian Schmaus]

On 24/06/2023 19.01, Mike Gilbert wrote:

usermod refuses to update the home directory for a user with running
processes. Output a more helpful message and avoid calling die for this.

For other usermod failures, output stderr as an eerror message and die.

Example output:

  * Failed to update user portage
  * This user currently has one or more running processes.
  * Please update this user manually with the following command:
  *   usermod '--comment' 'System user; portage' '--home' 
'/var/lib/portage/home' '--shell' '/bin/bash' '--gid' 'portage' '--groups' '' 
portage

Bug: https://bugs.gentoo.org/888189
Signed-off-by: Mike Gilbert <flop...@gentoo.org>
---
  eclass/acct-user.eclass | 28 +++++++++++++++++++++++-----
  1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/eclass/acct-user.eclass b/eclass/acct-user.eclass
index f8a51ebf9c6b..88d7d354c8e6 100644
--- a/eclass/acct-user.eclass
+++ b/eclass/acct-user.eclass
@@ -432,12 +432,30 @@ acct-user_pkg_postinst() {
        fi

-       if ! usermod "${opts[@]}" "${ACCT_USER_NAME}" 
2>"${T}/usermod-error.log"; then
-               # usermod outputs a warning if unlocking the account would 
result in an
-               # empty password. Hide stderr in a text file and display it if 
usermod
-               # fails.
+       # usermod outputs a warning if unlocking the account would result in an
+       # empty password. Hide stderr in a text file and display it if usermod 
fails.
+       usermod "${opts[@]}" "${ACCT_USER_NAME}" 2>"${T}/usermod-error.log"
+       local status=$?
+       if [[ ${status} -ne 0 ]]; then
                cat "${T}/usermod-error.log" >&2
-               die "usermod failed"
+               if [[ ${status} -eq 8 ]]; then
+                       # usermod refused to update the home directory
+                       # for a uid with active processes.
+                       eerror "Failed to update user ${ACCT_USER_NAME}"
+                       eerror "This user currently has one or more running 
processes."
+                       eerror "Please update this user manually with the following 
command:"
+
+                       # Surround opts with quotes.
+                       # With bash-5 (EAPI 8), we can use "${opts[@]@Q}" 
instead.
+                       local q="'"
+                       local optsq=( "${opts[@]/#/$q}" )
+                       optsq=( "${optsq[@]/%/$q}" )
+
+                       eerror "  usermod ${optsq[@]} ${ACCT_USER_NAME}"
+               else
+                       eerror "$(<"${T}/usermod-error.log")"
+                       die "usermod failed"

elog "Updating user ${ACCT_USER_NAME}"

                       die "usermod failed (${status})"

Otherwise, lgtm. Thanks!

- Flow

No strong opinion, but it is often a good idea to include the exit status. For example:

OpenPGP_0x8CAC2A9678548E35.asc
Description: OpenPGP public key

OpenPGP_signature
Description: OpenPGP digital signature