[arch-projects] [dbscripts] [PATCH v2 3/3] Update messages to make fuller use of printf formatters

[Eli Schwartz]

On 02/22/2018 09:15 PM, Luke Shumaker wrote:
> From: Luke Shumaker <lukeshu at parabola.nu>
> 
> These are things that were (IMO) missed in 5afac1e.  I found them using:
> 
>     git grep -E '(plain|msg|msg2|warning|error|die) "[^"]*\$'
> 
> I went a little above-and-beyond for escaping strings for the error
> messages in db-functions' arch_repo_add and arch_repo_remove.  The
> code should explain itself, but I wanted to point it out, as it's more than
> the usual "slap %s in there, and move the ${...} to the right".
> 
> v2:
>  - arch_repo_add, arch_repo_remove: Use Bash 4.4 parameter
>    transformation to avoid having to introduce temporary variables.

> --- a/db-functions
> +++ b/db-functions
> @@ -450,7 +450,7 @@ arch_repo_add() {
>  	# package files might be relative to repo dir
>  	pushd "${FTP_BASE}/${repo}/os/${arch}" >/dev/null
>  	/usr/bin/repo-add -q "${repo}${DBEXT}" ${pkgs[@]} \
> -		|| error "repo-add ${repo}${DBEXT} ${pkgs[@]}"
> +		|| error 'repo-add %q %s' "${repo}${DBEXT}" "${pkgs[*]@Q}"
>  	popd >/dev/null
>  	set_repo_permission "${repo}" "${arch}"
>  
> @@ -468,7 +468,7 @@ arch_repo_remove() {
>  		return 1
>  	fi
>  	/usr/bin/repo-remove -q "${dbfile}" ${pkgs[@]} \
> -		|| error "repo-remove ${dbfile} ${pkgs[@]}"
> +		|| error 'repo-remove %q %s' "$dbfile" "${pkgs[*]@Q}"
>  	set_repo_permission "${repo}" "${arch}"

I think for consistency we should use the same style which means using
"${dbfile at Q}"

Currently we'll get the dbfile backslash-escaped, followed by the
package names unequivocally quoted on the same line of error output. It
makes more sense to ensure that they are all quoted.

While in theory one could bikeshed over which is the better way to
consistently print them, there is only one option for us since we're
trying to interject all the ${pkgs[*]} as one argument...

>  
>  	REPO_MODIFIED=1
> diff --git a/db-move b/db-move
> index fb7ebac..806ad9a 100755
> --- a/db-move
> +++ b/db-move
> @@ -4,7 +4,7 @@
>  . "$(dirname $0)/db-functions"
>  
>  if (( $# < 3 )); then
> -	msg "usage: ${0##*/} <repo-from> <repo-to> <pkgname|pkgbase> ..."
> +	msg "usage: %s <repo-from> <repo-to> <pkgname|pkgbase> ..." "${0##*/}"
>  	exit 1
>  fi
>  
> @@ -74,7 +74,7 @@ for pkgbase in ${args[@]:2}; do
>  			else
>  				tarches=("${pkgarch}")
>  			fi
> -			msg2 "${pkgbase} ($(echo ${tarches[@]}))"
> +			msg2 "%s (%s)" "$pkgbase" "${tarches[*]}"
>  			pkgnames=($(. "${svnrepo_from}/PKGBUILD"; echo ${pkgname[@]}))
>  			pkgver=$(. "${svnrepo_from}/PKGBUILD"; get_full_version)
>  
> diff --git a/db-remove b/db-remove
> index 70502bc..e7aed31 100755
> --- a/db-remove
> +++ b/db-remove
> @@ -4,7 +4,7 @@
>  . "$(dirname $0)/db-functions"
>  
>  if (( $# < 3 )); then
> -	msg "usage: ${0##*/} <repo> <arch> <pkgname|pkgbase> ..."
> +	msg "usage: %s <repo> <arch> <pkgname|pkgbase> ..." "${0##*/}"
>  	exit 1
>  fi
>  
> diff --git a/db-repo-add b/db-repo-add
> index d7be302..4ea758f 100755
> --- a/db-repo-add
> +++ b/db-repo-add
> @@ -4,7 +4,7 @@
>  . "$(dirname $0)/db-functions"
>  
>  if (( $# < 3 )); then
> -	msg "usage: ${0##*/} <repo> <arch> <pkgfile> ..."
> +	msg "usage: %s <repo> <arch> <pkgfile> ..." "${0##*/}"
>  	exit 1
>  fi
>  
> diff --git a/db-repo-remove b/db-repo-remove
> index 32d167e..2f24edd 100755
> --- a/db-repo-remove
> +++ b/db-repo-remove
> @@ -4,7 +4,7 @@
>  . "$(dirname $0)/db-functions"
>  
>  if (( $# < 3 )); then
> -	msg "usage: ${0##*/} <repo> <arch> <pkgname> ..."
> +	msg "usage: %s <repo> <arch> <pkgname> ..." "${0##*/}"
>  	exit 1
>  fi
>  
> diff --git a/db-update b/db-update
> index 4e17184..5077389 100755
> --- a/db-update
> +++ b/db-update
> @@ -78,7 +78,7 @@ for repo in ${repos[@]}; do
>  		arch_pkgs=($(getpkgfiles "${STAGING}/${repo}/"*-${pkgarch}${PKGEXTS} 2>/dev/null))
>  		for pkg in ${arch_pkgs[@]} ${any_pkgs[@]}; do
>  			pkgfile="${pkg##*/}"
> -			msg2 "${pkgfile} (${pkgarch})"
> +			msg2 '%s (%s)' "$pkgfile" "$pkgarch"
>  			# any packages might have been moved by the previous run
>  			if [[ -f ${pkg} ]]; then
>  				mv "${pkg}" "$FTP_BASE/${PKGPOOL}"
> diff --git a/testing2x b/testing2x
> index f0d77cb..d68e405 100755
> --- a/testing2x
> +++ b/testing2x
> @@ -4,7 +4,7 @@
>  . "$(dirname $0)/db-functions"
>  
>  if (( $# < 1 )); then
> -	msg "usage: ${0##*/} <pkgname|pkgbase> ..."
> +	msg "usage: %s <pkgname|pkgbase> ..." "${0##*/}"
>  	exit 1
>  fi
>  
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/arch-projects/attachments/20180226/c259ca09/attachment.asc>