[pacman-dev] [PATCH] repo-add: use more libmakepkg to handle common compression routines
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. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- 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..0f9628ad 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 "$REPO_DB_SUFFIX" > "$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.*} -- 2.21.0
On 3/8/19 2:32 PM, 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.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- 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..0f9628ad 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) ;;
repo-add mydb.tar.badext ... This does no checking at all.
*) 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 "$REPO_DB_SUFFIX" > "$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.*}
On Tue, 12 Mar 2019 16:32:34 +1000 Allan McRae <allan@archlinux.org> wrote:
On 3/8/19 2:32 PM, 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.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- 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..0f9628ad 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) ;;
repo-add mydb.tar.badext ...
This does no checking at all.
You could use the mime type instead, which also removes the need to hardcode .tar.foo: local mimetype=$(file --mime-type -b "$1") case $mimetype in application/gzip) TAR_OPT="z" ;; application/x-bzip2) TAR_OPT="j" ;; application/x-xz) TAR_OPT="J" ;; application/x-zstd) TAR_OPT="--zstd" ;; application/x-compress) TAR_OPT="Z" ;; application/x-tar) TAR_OPT="" ;; esac Ideally this would be done in libmakepkg's compress.sh, and then re-used here. Thoughts? Alad
*) 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 "$REPO_DB_SUFFIX" > "$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.*}
-- Alad Wenter <alad@archlinux.org>
On 3/12/19 8:51 AM, Alad Wenter wrote:
On Tue, 12 Mar 2019 16:32:34 +1000 Allan McRae <allan@archlinux.org> wrote:
On 3/8/19 2:32 PM, 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.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- 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..0f9628ad 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) ;;
repo-add mydb.tar.badext ...
This does no checking at all.
You could use the mime type instead, which also removes the need to hardcode .tar.foo:
local mimetype=$(file --mime-type -b "$1")
case $mimetype in application/gzip) TAR_OPT="z" ;; application/x-bzip2) TAR_OPT="j" ;; application/x-xz) TAR_OPT="J" ;; application/x-zstd) TAR_OPT="--zstd" ;; application/x-compress) TAR_OPT="Z" ;; application/x-tar) TAR_OPT="" ;; esac
Ideally this would be done in libmakepkg's compress.sh, and then re-used here. Thoughts?
Alad
The file only exists yet, when updating an existing database, and you cannot get a mimetype from a nonexistent file. I see no reason why or how the use of mimetypes vs. file extension matches would help in this specific case though -- we already have a list of compression mappings using $implementation and we just want to figure out under which situations to use it. I remember in relation to aurutils you have some use case where it is problematic for you that the .db file itself is not autodetected *on updates* using mimetypes, but this is not really the same issue at all and implementing extensionless databases that are just .db would mean adding a new option to repo-add to express the compression type for the database. -- Eli Schwartz Bug Wrangler and Trusted User
On 3/12/19 2:32 AM, Allan McRae wrote:
On 3/8/19 2:32 PM, 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.
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- 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..0f9628ad 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) ;;
repo-add mydb.tar.badext ...
This does no checking at all.
It does. It continues to fail hard if the extension is not .db.tar with an optional additional extension, and instead of failing hard when specifying an unknown compression type, it reuses the existing libmakepkg infrastructure to use "cat" as the compressor, but emit a warning when an invalid archive extension is shown. IMHO it is more reasonable to allow it but warn about it, like makepkg itself does. I can send in a v2 with a modified commit message specifying that (but also I need to fix compress_as slightly).
*) 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 "$REPO_DB_SUFFIX" > "$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.*}
-- Eli Schwartz Bug Wrangler and Trusted User
participants (3)
-
Alad Wenter
-
Allan McRae
-
Eli Schwartz