[pacman-dev] [PATCH 1/3] libmakepkg: use extraction commands instead of file to find archive type

[Eli Schwartz]

On 11/26/19 4:29 PM, Ethan Sommer wrote:
> Previously, to determine which command we should use to extract an
> archive, we would run file and match the output against our list of
> possible extraction commands
> 
> Instead, run the archive through each extraction command's -t (--test)
> flag, if this succeeds then we know that the command is able to extract
> the file and is the one to use

Missing rationale why we care.

>  scripts/libmakepkg/source/file.sh.in | 39 ++++++++--------------------
>  1 file changed, 11 insertions(+), 28 deletions(-)
> 
> diff --git a/scripts/libmakepkg/source/file.sh.in b/scripts/libmakepkg/source/file.sh.in
> index 7297a1c6..faace79b 100644
> --- a/scripts/libmakepkg/source/file.sh.in
> +++ b/scripts/libmakepkg/source/file.sh.in
> @@ -96,35 +96,18 @@ extract_file() {
>  	fi
>  
>  	# do not rely on extension for file type
> -	local file_type=$(@FILECMD@ -bizL -- "$file")
> -	local ext=${file##*.}
>  	local cmd=''
> -	case "$file_type" in
> -		*application/x-tar*|*application/zip*|*application/x-zip*|*application/x-cpio*)
> -			cmd="bsdtar" ;;
> -		*application/x-gzip*|*application/gzip*)
> -			case "$ext" in
> -				gz|z|Z) cmd="gzip" ;;
> -				*) return;;
> -			esac ;;
> -		*application/x-bzip*)
> -			case "$ext" in
> -				bz2|bz) cmd="bzip2" ;;
> -				*) return;;
> -			esac ;;
> -		*application/x-xz*)
> -			case "$ext" in
> -				xz) cmd="xz" ;;
> -				*) return;;
> -			esac ;;
> -		*)
> -			# See if bsdtar can recognize the file
> -			if bsdtar -tf "$file" -q '*' &>/dev/null; then
> -				cmd="bsdtar"
> -			else
> -				return 0
> -			fi ;;
> -	esac
> +	if bsdtar -tf "$file" -q '*'; then
> +		cmd='bsdtar'
> +	elif gzip -t "$file"; then
> +		cmd='gzip'
> +	elif bzip2 -t "$file"; then
> +		cmd='bzip2'
> +	elif xz -t "$file"; then
> +		cmd='xz'
> +	else
> +		return 0
> +	fi &>/dev/null

I had to look at this three times before I realized you were redirecting
both stdout and stderr of the entire if/elif block.

Because these commands check to see if the file is "uncorrupted", not to
see if they are "formatted in this compression format", I imagine this
would falsely fail files if they have some class of recoverable error,
but I'm not sure if we care.

What we do care about, I think, is that you're silencing stderr due to
the fact that it will noisily print diagnostic messages when the file is
not compressed or is in a different compression format, and bsdtar will
print listed filenames to stdout. So now, if these commands fail for
*any* reason, we treat this as a successful analysis that the file is in
a different format. One reason for it failing might be that xz is not
installed... this is hardly a successful analysis, and printing an
"error: command not found" message during the decompression, then dying
with "Failed to extract %s" is a desirable feature.

>  	local ret=0
>  	msg2 "$(gettext "Extracting %s with %s")" "$file" "$cmd"
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1601 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/pacman-dev/attachments/20191127/18ebbecb/attachment.sig>