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

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

[Mike Gilbert]

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 <floppym@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
 
 	elog "Updating user ${ACCT_USER_NAME}"
-	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"
+		fi
 	fi
 }
 
-- 
2.41.0

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

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 2607 bytes --]

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 <floppym@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
>   
>   	elog "Updating user ${ACCT_USER_NAME}"
> -	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"

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

                        die "usermod failed (${status})"

Otherwise, lgtm. Thanks!

- Flow

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17273 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

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

[Mike Gilbert]

On Sun, Jun 25, 2023 at 6:54 AM Florian Schmaus <flow@gentoo.org> wrote:
> No strong opinion, but it is often a good idea to include the exit
> status. For example:
>
>                         die "usermod failed (${status})"

That's a really good idea. It could also be applied to the
useradd/groupadd/groupmod commands.