[pacman-dev,v2,2/2] repo-add: use more libmakepkg to handle common compression routines

Message ID 20190312172828.13249-2-eschwartz@archlinux.org
State Changes Requested
Headers show
Series [pacman-dev,v2,1/2] libmakepkg: fix reporting of invalid archive extensions in compress.sh | expand

Commit Message

Eli Schwartz March 12, 2019, 5:28 p.m. UTC
Currently the list of supported formats for an archive, is maintained in
two places. And repo-add does not actually get updated. :(

In the process, remove some of the logical duplication when calling
bsdtar/compress_as. Also, move to emulating makepkg by not returning a
hard error if given an invalid archive extension; compress_as will
accept it, throw a warning, and proceed using `cat` as the compression
filter, which pacman consumes just as well.

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---

v2: use a better argument to compress_as, expand the commit message a
bit to clarify the practical effects of the change.

 scripts/repo-add.sh.in | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

Comments

Allan McRae June 6, 2019, 2:02 a.m. UTC | #1
On 13/3/19 3:28 am, Eli Schwartz wrote:
> Currently the list of supported formats for an archive, is maintained in
> two places. And repo-add does not actually get updated. :(
> 
> In the process, remove some of the logical duplication when calling
> bsdtar/compress_as. Also, move to emulating makepkg by not returning a
> hard error if given an invalid archive extension; compress_as will
> accept it, throw a warning, and proceed using `cat` as the compression
> filter, which pacman consumes just as well.
> 
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
> 
> v2: use a better argument to compress_as, expand the commit message a
> bit to clarify the practical effects of the change.
> 
>  scripts/repo-add.sh.in | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> index 57413df5..ec2838a7 100644
> --- a/scripts/repo-add.sh.in
> +++ b/scripts/repo-add.sh.in
> @@ -44,6 +44,7 @@ CLEAN_LOCK=0
>  USE_COLOR='y'
>  
>  # Import libmakepkg
> +source "$LIBRARY"/util/compress.sh
>  source "$LIBRARY"/util/message.sh
>  
>  # ensure we have a sane umask set
> @@ -188,21 +189,12 @@ verify_signature() {
>  }
>  
>  verify_repo_extension() {
> -	local repofile=$1
> -
> -	case $repofile in
> -		*.db.tar.gz)  TAR_OPT="z" ;;
> -		*.db.tar.bz2) TAR_OPT="j" ;;
> -		*.db.tar.xz)  TAR_OPT="J" ;;
> -		*.db.tar.zst) TAR_OPT="--zstd" ;;
> -		*.db.tar.Z)   TAR_OPT="Z" ;;
> -		*.db.tar)     TAR_OPT="" ;;
> +	case $1 in
> +		*.db.tar.*|*.db.tar)   ;;

I'd like to see a "can_compress_as" function added so that we can
actually test if the extension is recognized.   We could also use this
to check PKGEXT/SRCEXT rather than falling back to plain tar.

Allan

Patch

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
index 57413df5..ec2838a7 100644
--- a/scripts/repo-add.sh.in
+++ b/scripts/repo-add.sh.in
@@ -44,6 +44,7 @@  CLEAN_LOCK=0
 USE_COLOR='y'
 
 # Import libmakepkg
+source "$LIBRARY"/util/compress.sh
 source "$LIBRARY"/util/message.sh
 
 # ensure we have a sane umask set
@@ -188,21 +189,12 @@  verify_signature() {
 }
 
 verify_repo_extension() {
-	local repofile=$1
-
-	case $repofile in
-		*.db.tar.gz)  TAR_OPT="z" ;;
-		*.db.tar.bz2) TAR_OPT="j" ;;
-		*.db.tar.xz)  TAR_OPT="J" ;;
-		*.db.tar.zst) TAR_OPT="--zstd" ;;
-		*.db.tar.Z)   TAR_OPT="Z" ;;
-		*.db.tar)     TAR_OPT="" ;;
+	case $1 in
+		*.db.tar.*|*.db.tar)   ;;
 		*) error "$(gettext "'%s' does not have a valid database archive extension.")" \
-				"$repofile"
+				"$1"
 			exit 1 ;;
 	esac
-
-	printf '%s' "$TAR_OPT"
 }
 
 # write an entry to the pacman database
@@ -513,7 +505,6 @@  rotate_db() {
 }
 
 create_db() {
-	TAR_OPT=$(verify_repo_extension "$REPO_DB_FILE")
 	# $LOCKFILE is already guaranteed to be absolute so this is safe
 	dirname=${LOCKFILE%/*}
 
@@ -523,13 +514,13 @@  create_db() {
 		tempname=$dirname/.tmp.$filename
 
 		pushd "$tmpdir/$repo" >/dev/null
-		if ( shopt -s nullglob; files=(*); (( ${#files[*]} )) ); then
-			bsdtar -c${TAR_OPT}f "$tempname" *
-		else
+		local files=(*)
+		if [[ ${files[*]} = '*' ]]; then
 			# we have no packages remaining? zip up some emptyness
 			warning "$(gettext "No packages remain, creating empty database.")"
-			bsdtar -c${TAR_OPT}f "$tempname" -T /dev/null
+			files=(-T /dev/null)
 		fi
+		bsdtar -cf - "${files[@]}" | compress_as "$filename" > "$tempname"
 		popd >/dev/null
 
 		create_signature "$tempname"
@@ -644,7 +635,7 @@  else
 	LOCKFILE=$PWD/$REPO_DB_FILE.lck
 fi
 
-verify_repo_extension "$REPO_DB_FILE" >/dev/null
+verify_repo_extension "$REPO_DB_FILE"
 
 REPO_DB_PREFIX=${REPO_DB_FILE##*/}
 REPO_DB_PREFIX=${REPO_DB_PREFIX%.db.*}