From 57d53000aaff7ad3826e79ef43d29eb1571df53f Mon Sep 17 00:00:00 2001 From: Levente Polyak <anthraxx@archlinux.org> Date: Thu, 12 Oct 2023 20:33:24 +0200 Subject: [PATCH] feat(db-remove): revamp arguments to handle all arches in one batch This effectively also fixes multiple issues with combinations of any packages and their native subset removal. The sacrifice is a different CLI interface that caller need to adapt to. For specific use cases a new --arch options has been introduced to just remove a specific architecture. For removing only partial packages from a split pkgbase, the option --partial must be passed. By default only allow removing complete pkgbase. This makes both operation mutual and explicit, either remove partial packages or remove complete pkgbases. Fixes #21 Fixes #39 --- db-functions | 6 +- db-remove | 144 +++++++++++++++++++++++++++------ test/cases/common.bats | 11 ++- test/cases/db-remove.bats | 48 ++++++++--- test/cases/ftpdir-cleanup.bats | 46 +++++------ test/cases/sourceballs.bats | 4 +- test/lib/common.bash | 2 +- 7 files changed, 196 insertions(+), 65 deletions(-) diff --git a/db-functions b/db-functions index afc7175..32efbb6 100644 --- a/db-functions +++ b/db-functions @@ -428,13 +428,13 @@ maybe_getpkgfile() { getpkgfile() { if (( $# != 1 )); then error 'No canonical package found!' - exit 1 + return 1 elif [[ ! -f ${1} ]]; then error "Package %s not found!" "$1" - exit 1 + return 1 elif [[ ! -f ${1}.sig ]]; then error "Package signature %s not found!" "$1.sig" - exit 1 + return 1 fi echo "${1}" diff --git a/db-remove b/db-remove index 475bd68..f418eae 100755 --- a/db-remove +++ b/db-remove @@ -3,15 +3,79 @@ . "$(dirname "$(readlink -e "$0")")/config" . "$(dirname "$(readlink -e "$0")")/db-functions" -if (( $# < 3 )); then - msg "usage: %s <repo> <arch> <pkgname|pkgbase> ..." "${0##*/}" +set -eo pipefail +shopt -s nullglob + +colorize + +usage() { + cat <<- _EOF_ + Usage: ${BASH_SOURCE[0]##*/} [OPTIONS] REPO PKGBASE... + + OPTIONS: + -a, --arch ARCH Remove packages exclusively from ARCH instead of everywhere + --partial Remove only specific partial pkgnames from a split package + This leaves debug packages and pkgbase entries in the state repo + -h, --help Show this help text +_EOF_ +} + +pkgbase_mode=1 +arch=any + +# option checking +while (( $# )); do + case $1 in + -h|--help) + usage + exit 0 + ;; + -a|--arch) + (( $# <= 1 )) && die "missing argument for %s" "$1" + arch=$2 + shift 2 + if [[ "${arch}" != any ]] && ! in_array "${arch}" "${ARCHES[@]}"; then + die "invalid architecture: %s" "${arch}" + fi + ;; + --partial) + pkgbase_mode=0 + warning "Removing only partial pkgnames from a split package" + warning "This leaves debug packages and pkgbase entries in the state repo" + shift + ;; + -*) + die "invalid argument: %s" "$1" + ;; + *) + break + ;; + esac +done + +if (( $# < 2 )); then + usage exit 1 fi +if [[ $arch = any ]]; then + tarches=("${ARCHES[@]}") + vcsarches=("${ARCHES[@]}") + vcsarches+=(any) +else + tarches=("$arch") + vcsarches=("$arch") +fi + repo="$1" -arch="$2" -pkgbases=("${@:3}") +pkgbases=("${@:2}") + +# filter out duplicates +mapfile -t pkgbases < <(printf '%s\n' "${pkgbases[@]}" | sort --unique) +if ! in_array "${repo}" "${PKGREPOS[@]}"; then + die "Unknown target repository %s" "${repo}" +fi if ! check_repo_permission "$repo"; then die "You don't have permission to remove packages from %s" "$repo" fi @@ -19,12 +83,16 @@ if ! check_author; then die "You don't have a matching author mapping" fi -if [[ $arch = any ]]; then - tarches=("${ARCHES[@]}") -else - tarches=("$arch") +# Fetch all pkgbase repositories once +if (( pkgbase_mode )); then + for pkgbase in "${pkgbases[@]}"; do + if ! fetch_pkgbuild "${pkgbase}"; then + die "Couldn't find package %s in git!" "${pkgbase}" + fi + done fi +# lock repos for tarch in "${tarches[@]}"; do repo_lock "$repo" "$tarch" || exit 1 done @@ -32,33 +100,61 @@ done declare -A remove_pkgbases declare -A remove_pkgs declare -A remove_debug_pkgs +declare -A handled_pkgbaes for pkgbase in "${pkgbases[@]}"; do - msg "Removing %s from [%s]..." "$pkgbase" "$repo" - - for tarch in "${tarches[@]}" any; do + for tarch in "${vcsarches[@]}"; do vcsrepo="$repo-$tarch" # check to remove whole pkgbase or parts of a split package - if fetch_pkgbuild "${pkgbase}" && \ - _pkgver=$(pkgver_from_state_repo "${pkgbase}" "${vcsrepo}") && \ - mapfile -t pkgnames < <(source_pkgbuild "${pkgbase}" "${_pkgver}" && printf "%s\n" "${pkgname[@]}"); then - array_append remove_pkgbases "${vcsrepo}" "${pkgbase}" - array_append remove_pkgs "${tarch}" "${pkgnames[*]}" - elif is_globfile "${FTP_BASE}/${repo}/os/${tarch}/${pkgbase}"-*-*-*.pkg.tar*; then - warning "pkgbase %s not found in %s - removing only pkgname" "$pkgbase" "$vcsrepo" - warning "If %s was a split package you have to pass its pkgbase to remove it completely!" "$pkgbase" - array_append remove_pkgs "${tarch}" "${pkgbase}" + if (( ! pkgbase_mode )); then + # check to remove split packages + if is_globfile "${FTP_BASE}/${repo}/os/${tarch}/${pkgbase}"-*-*-*.pkg.tar*; then + msg "Removing %s from %s [%s]..." "${pkgbase}" "${repo}" "${tarch}" + array_append remove_pkgs "${tarch}" "${pkgbase}" + handled_pkgbaes["${pkgbase}"]=1 + fi + continue fi - # check to remove debug packages - if is_globfile "${FTP_BASE}/${repo}-debug/os/${tarch}/${pkgbase}-debug"-*-*-*.pkg.tar*; then - msg "Found debug package. Removing %s from [%s]..." "${pkgbase}-debug" "${repo}-debug" - array_append remove_debug_pkgs "${tarch}" "${pkgbase}-debug" + # try to get pkgbase metadata from state repo + if ! _pkgver=$(pkgver_from_state_repo "${pkgbase}" "${vcsrepo}"); then + continue + fi + + array_append remove_pkgbases "${vcsrepo}" "${pkgbase}" + handled_pkgbaes["${pkgbase}"]=1 + + # loop through all arches for any packages + remove_arches=("${tarch}") + if [[ ${tarch} == any ]]; then + remove_arches=("${tarches[@]}") fi + + # remove pkgnames + mapfile -t pkgnames < <(source_pkgbuild "${pkgbase}" "${_pkgver}" && printf "%s\n" "${pkgname[@]}") + for remove_arch in "${remove_arches[@]}"; do + msg "Removing %s from %s [%s]..." "${pkgbase}" "${repo}" "${remove_arch}" + array_append remove_pkgs "${remove_arch}" "${pkgnames[*]}" + done + + # check to remove debug packages + for remove_arch in any "${remove_arches[@]}"; do + if is_globfile "${FTP_BASE}/${repo}-debug/os/${remove_arch}/${pkgbase}-debug"-*-*-*.pkg.tar*; then + msg "Found debug package. Removing %s from %s [%s]..." "${pkgbase}-debug" "${repo}-debug" "${remove_arch}" + array_append remove_debug_pkgs "${remove_arch}" "${pkgbase}-debug" + fi + done done done +# check for unhandled arguments before executing +for pkgbase in "${pkgbases[@]}"; do + if [[ ! "${handled_pkgbaes["${pkgbase}"]+exists}" ]]; then + die "Package not found in %s: %s" "${repo}" "${pkgbase}" + fi +done + # remove binary repo packages for key in "${!remove_pkgs[@]}"; do tarch=${key} diff --git a/test/cases/common.bats b/test/cases/common.bats index 2239476..610b9a5 100644 --- a/test/cases/common.bats +++ b/test/cases/common.bats @@ -1,9 +1,18 @@ load ../lib/common @test "commands display usage message by default" { - for cmd in db-move db-remove db-repo-add db-repo-remove testing2x; do + for cmd in db-move db-repo-add db-repo-remove testing2x; do echo Testing $cmd run ! $cmd [[ $output == *'usage: '* ]] done + for cmd in db-remove; do + echo Testing $cmd + run ! $cmd + [[ $output == *'Usage: '* ]] + + echo Testing $cmd --help + run -0 $cmd --help + [[ $output == *'Usage: '* ]] + done } diff --git a/test/cases/db-remove.bats b/test/cases/db-remove.bats index 5575e73..d850fd9 100644 --- a/test/cases/db-remove.bats +++ b/test/cases/db-remove.bats @@ -14,7 +14,7 @@ load ../lib/common for pkgbase in ${pkgs[@]}; do for arch in ${arches[@]}; do - db-remove extra ${arch} ${pkgbase} + db-remove --arch ${arch} extra ${pkgbase} run ! checkStateRepoContains extra ${arch} ${pkgbase} done done @@ -39,7 +39,7 @@ load ../lib/common for pkgbase in ${pkgs[@]}; do for arch in ${arches[@]}; do - db-remove extra ${arch} ${pkgbase} + db-remove --arch ${arch} extra ${pkgbase} run ! checkStateRepoContains extra ${arch} ${pkgbase} done done @@ -51,6 +51,8 @@ load ../lib/common } @test "remove specific debug package" { + skip "removing only debug packages is currently unsupported" + local arches=('i686' 'x86_64') local pkgs=('pkg-split-debuginfo') local debug_pkgs=('pkg-split-debuginfo') @@ -67,7 +69,7 @@ load ../lib/common # without removing the repo packages for pkgbase in ${debug_pkgs[@]}; do for arch in ${arches[@]}; do - db-remove extra-debug ${arch} ${pkgbase}-debug + db-remove --arch ${arch} extra-debug ${pkgbase}-debug checkStateRepoContains extra ${arch} ${pkgbase} done done @@ -90,7 +92,7 @@ load ../lib/common db-update for arch in ${arches[@]}; do - db-remove extra ${arch} ${pkgs[@]} + db-remove --arch ${arch} extra ${pkgs[@]} done for pkgbase in ${pkgs[@]}; do @@ -109,7 +111,7 @@ load ../lib/common db-update for arch in ${arches[@]}; do - db-remove extra "${arch}" pkg-split-a1 + db-remove --arch "${arch}" --partial extra pkg-split-a1 checkStateRepoContains extra ${arch} pkg-split-a for db in db files; do @@ -132,7 +134,7 @@ load ../lib/common db-update for pkgbase in ${pkgs[@]}; do - db-remove extra any ${pkgbase} + run -0 db-remove extra ${pkgbase} done for pkgbase in ${pkgs[@]}; do @@ -150,7 +152,7 @@ load ../lib/common db-update disablePermissionOverride - run ! db-remove noperm any ${pkgbase} + run ! db-remove noperm ${pkgbase} checkPackage noperm ${pkgbase} 1-1 } @@ -159,7 +161,7 @@ load ../lib/common releasePackage testing pkg-any-a db-update - db-remove testing any pkg-any-a + db-remove testing pkg-any-a checkRemovedPackage testing pkg-any-a checkStateRepoAutoredBy "Bob Tester <tester@localhost>" @@ -170,7 +172,7 @@ load ../lib/common db-update emptyAuthorsFile - run ! db-remove testing any pkg-any-a + run ! db-remove testing pkg-any-a checkPackage testing pkg-any-a 1-1 } @@ -183,10 +185,36 @@ load ../lib/common releasePackage extra ${pkgbase} db-update - db-remove extra any ${pkgbase} + db-remove extra ${pkgbase} + + checkRemovedPackage extra ${pkgbase} + for arch in ${arches[@]}; do + run ! checkStateRepoContains extra ${arch} ${pkgbase} + done +} + +@test "remove duplicate packages in command" { + local pkgbase=pkg-simple-a + local arches=('i686' 'x86_64') + local arch + + releasePackage extra ${pkgbase} + + db-update + db-remove extra ${pkgbase} ${pkgbase} checkRemovedPackage extra ${pkgbase} for arch in ${arches[@]}; do run ! checkStateRepoContains extra ${arch} ${pkgbase} done } + +@test "remove none existing pkgbase fails" { + releasePackage extra pkg-any-a + db-update + + run ! db-remove extra pkg-any-a zdoesnotexist + [[ $output == *"Couldn't find package zdoesnotexist"* ]] + + checkPackage extra pkg-any-a 1-1 +} diff --git a/test/cases/ftpdir-cleanup.bats b/test/cases/ftpdir-cleanup.bats index e06b474..fb30a69 100644 --- a/test/cases/ftpdir-cleanup.bats +++ b/test/cases/ftpdir-cleanup.bats @@ -40,9 +40,7 @@ __checkRepoRemovedPackage() { db-update - for arch in ${arches[@]}; do - db-remove extra ${arch} pkg-simple-a - done + db-remove extra pkg-simple-a ftpdir-cleanup @@ -65,9 +63,7 @@ __checkRepoRemovedPackage() { done db-update - for arch in ${arches[@]}; do - db-remove extra ${arch} "${pkgs[0]}" - done + db-remove extra "${pkgs[0]}" ftpdir-cleanup @@ -145,9 +141,7 @@ __checkRepoRemovedPackage() { done db-update - for arch in ${arches[@]}; do - db-remove extra ${arch} "${pkgs[0]}" - done + db-remove extra "${pkgs[0]}" ftpdir-cleanup @@ -173,9 +167,7 @@ __checkRepoRemovedPackage() { db-update - for arch in ${arches[@]}; do - db-remove extra ${arch} pkg-simple-epoch - done + db-remove extra pkg-simple-epoch ftpdir-cleanup @@ -196,7 +188,7 @@ __checkRepoRemovedPackage() { done db-update - db-remove extra any pkg-any-a + db-remove extra pkg-any-a ftpdir-cleanup local pkg1="pkg-any-a-1-1-any${PKGEXT}" @@ -221,9 +213,7 @@ __checkRepoRemovedPackage() { db-update - for arch in ${arches[@]}; do - db-remove extra ${arch} ${pkgs[0]} - done + db-remove extra ${pkgs[0]} ftpdir-cleanup @@ -248,21 +238,31 @@ __checkRepoRemovedPackage() { db-update for pkgbase in ${pkgs[@]}; do - for arch in ${arches[@]}; do - db-remove extra ${arch} ${pkgbase} - done + db-remove extra ${pkgbase} done ftpdir-cleanup - local pkgfilea="pkg-simple-a-1-1-${arch}${PKGEXT}" - local pkgfileb="pkg-simple-b-1-1-${arch}${PKGEXT}" for arch in ${arches[@]}; do + local pkgfilea="pkg-simple-a-1-1-${arch}${PKGEXT}" + local pkgfileb="pkg-simple-b-1-1-${arch}${PKGEXT}" + + [ -f ${CLEANUP_DESTDIR}/${pkgfilea} ] + [ -f ${CLEANUP_DESTDIR}/${pkgfileb} ] + done + + for arch in ${arches[@]}; do + local pkgfilea="pkg-simple-a-1-1-${arch}${PKGEXT}" touch -d "-$(expr ${CLEANUP_KEEP} + 1)days" ${CLEANUP_DESTDIR}/${pkgfilea}{,.sig} done ftpdir-cleanup - [ ! -f ${CLEANUP_DESTDIR}/${pkgfilea} ] - [ -f ${CLEANUP_DESTDIR}/${pkgfileb} ] + for arch in ${arches[@]}; do + local pkgfilea="pkg-simple-a-1-1-${arch}${PKGEXT}" + local pkgfileb="pkg-simple-b-1-1-${arch}${PKGEXT}" + + [ ! -f ${CLEANUP_DESTDIR}/${pkgfilea} ] + [ -f ${CLEANUP_DESTDIR}/${pkgfileb} ] + done } diff --git a/test/cases/sourceballs.bats b/test/cases/sourceballs.bats index df7ddd4..aed26ee 100644 --- a/test/cases/sourceballs.bats +++ b/test/cases/sourceballs.bats @@ -73,9 +73,7 @@ __checkRemovedSourcePackage() { db-update sourceballs - for arch in ${arches[@]}; do - db-remove extra ${arch} pkg-simple-a - done + db-remove extra pkg-simple-a sourceballs __checkRemovedSourcePackage pkg-simple-a diff --git a/test/lib/common.bash b/test/lib/common.bash index 3f5967c..57e0bd3 100644 --- a/test/lib/common.bash +++ b/test/lib/common.bash @@ -163,7 +163,7 @@ setup() { UNSTABLE_REPOS=(unstable) STAGING_REPOS=('staging') TESTING_REPOS=('testing') - STABLE_REPOS=('core' 'extra') + STABLE_REPOS=('core' 'extra' 'noperm') CLEANUP_DESTDIR="${TMP}/package-cleanup" SOURCE_CLEANUP_DESTDIR="${TMP}/source-cleanup" STAGING="${TMP}/staging" -- GitLab