Compare commits

..

3 Commits

Author SHA1 Message Date
Josephine Pfeiffer
939790367a Merge branch 'gl-261-confirm-move' into 'master'
[GL#261] add db move confirmation prompt

Closes #261

See merge request archlinux/devtools!304
2025-07-28 08:32:34 +00:00
Josephine Pfeiffer
f5c8258784 Merge branch 'master' of gitlab.archlinux.org:pfeifferj/devtools into gl-261-confirm-move 2025-03-18 22:26:52 +01:00
Josephine Pfeiffer
c169f5c7ab feat(pgkctl_db): require confirmation for package repository moves
The pkgctl db move command previously executed without confirmation,
which could lead to accidental package moves between repositories.
This change adds a confirmation prompt similar to deletion operations,
while preserving the ability to bypass confirmation with a new --noconfirm flag.

This approach prevents unintended package moves while maintaining efficiency:
- Confirmation prompt provides a safety check before executing moves
- The --noconfirm flag allows for scripting and automation when needed
- Implementation follows consistent patterns used in other pkgctl commands

Package move operations can have significant impact on repository management,
and this small usability improvement helps prevent mistakes that could
affect multiple users or require additional corrective actions.

Fixes #261

Component: pkgctl db move
Signed-off-by: Josephine Pfeiffer <hi@josie.lol>
2025-03-18 22:07:30 +01:00
9 changed files with 63 additions and 51 deletions

View File

@@ -150,7 +150,6 @@ _pkgctl_cmds=(
db db
diff diff
issue issue
license
release release
repo repo
search search

View File

@@ -14,7 +14,7 @@ Description
Build a PKGBUILD on a remote server using makechrootpkg. Requires a remote user Build a PKGBUILD on a remote server using makechrootpkg. Requires a remote user
that can run archbuild in a non-interactive manner, e.g. must be able to that can run archbuild in a non-interactive manner, e.g. must be able to
elevate permissions using passwordless run0. elevate permissions using passwordless sudo.
Options Options
------- -------

View File

@@ -3,7 +3,7 @@ pkgctl-auth(1)
Name Name
---- ----
pkgctl-auth - Authenticate with services like GitLab. pkgctl-auth - Authenticate with serivces like GitLab.
Synopsis Synopsis
-------- --------

View File

@@ -155,7 +155,7 @@ if (( ${#needsversioning[*]} )); then
if [[ ! -f "${file}" ]]; then if [[ ! -f "${file}" ]]; then
continue continue
fi fi
if ! git ls-files --error-unmatch "$file" >/dev/null; then if ! git ls-files --error-unmatch "$file"; then
die "%s is not under version control" "$file" die "%s is not under version control" "$file"
fi fi
done done

View File

@@ -15,11 +15,7 @@ check_root() {
local orig_argv=("$@") local orig_argv=("$@")
(( EUID == 0 )) && return (( EUID == 0 )) && return
if type -P run0 >/dev/null; then if type -P sudo >/dev/null; then
keepenv=",$keepenv"
command="run0 ${keepenv//,/ --setenv=}"
exec ${command} -- "${orig_argv[@]}"
elif type -P sudo >/dev/null; then
exec sudo --preserve-env="${keepenv}" -- "${orig_argv[@]}" exec sudo --preserve-env="${keepenv}" -- "${orig_argv[@]}"
else else
exec su root -c "$(printf ' %q' "${orig_argv[@]}")" exec su root -c "$(printf ' %q' "${orig_argv[@]}")"

View File

@@ -8,6 +8,8 @@ DEVTOOLS_INCLUDE_DB_MOVE_SH=1
_DEVTOOLS_LIBRARY_DIR=${_DEVTOOLS_LIBRARY_DIR:-@pkgdatadir@} _DEVTOOLS_LIBRARY_DIR=${_DEVTOOLS_LIBRARY_DIR:-@pkgdatadir@}
# shellcheck source=src/lib/common.sh # shellcheck source=src/lib/common.sh
source "${_DEVTOOLS_LIBRARY_DIR}"/lib/common.sh source "${_DEVTOOLS_LIBRARY_DIR}"/lib/common.sh
# shellcheck source=src/lib/valid-repos.sh
source "${_DEVTOOLS_LIBRARY_DIR}"/lib/valid-repos.sh
set -e set -e
@@ -20,6 +22,7 @@ pkgctl_db_move_usage() {
Move packages between binary repositories. Move packages between binary repositories.
OPTIONS OPTIONS
--noconfirm Bypass any confirmation messages, should only be used with caution
-h, --help Show this help text -h, --help Show this help text
EXAMPLES EXAMPLES
@@ -29,9 +32,12 @@ _EOF_
} }
pkgctl_db_move() { pkgctl_db_move() {
local SOURCE_REPO="" if (( $# < 3 )); then
local TARGET_REPO="" pkgctl_db_move_usage
local PKGBASES=() exit 1
fi
local CONFIRM=1
# option checking # option checking
while (( $# )); do while (( $# )); do
@@ -40,6 +46,10 @@ pkgctl_db_move() {
pkgctl_db_move_usage pkgctl_db_move_usage
exit 0 exit 0
;; ;;
--noconfirm)
CONFIRM=0
shift
;;
-*) -*)
die "invalid argument: %s" "$1" die "invalid argument: %s" "$1"
;; ;;
@@ -49,16 +59,35 @@ pkgctl_db_move() {
esac esac
done done
if (( $# < 3 )); then local source_repo=$1
pkgctl_db_move_usage local target_repo=$2
exit 1 shift 2
if ! in_array "${source_repo}" "${DEVTOOLS_VALID_REPOS[@]}"; then
die "Invalid source repository: %s" "${source_repo}"
fi
if ! in_array "${target_repo}" "${DEVTOOLS_VALID_REPOS[@]}"; then
die "Invalid target repository: %s" "${target_repo}"
fi fi
SOURCE_REPO=$1 if (( CONFIRM )); then
TARGET_REPO=$2 local pkglist
shift 2 pkglist=$(printf '%s\n' "$@" | paste -sd ' ')
PKGBASES+=("$@") read -r -p "Move packages from [${source_repo}] to [${target_repo}]: ${pkglist}? [Y/n] " response
case ${response} in
[Yy][Ee][Ss]|[Yy]|'')
: # continue
;;
*)
exit 0
;;
esac
fi
# shellcheck disable=SC2029 local pkgbase
ssh "${PACKAGING_REPO_RELEASE_HOST}" db-move "${SOURCE_REPO}" "${TARGET_REPO}" "${PKGBASES[@]}" for pkgbase in "$@"; do
msg "Moving [%s] from [%s] to [%s]" "${pkgbase}" "${source_repo}" "${target_repo}"
# shellcheck disable=SC2046
db-move "${source_repo}" "${target_repo}" $(pacman -Sql "${source_repo}" | grep "^${pkgbase}" || echo "${pkgbase}")
done
} }

View File

@@ -188,13 +188,10 @@ path = [
"README.md", "README.md",
"keys/**", "keys/**",
".SRCINFO", ".SRCINFO",
".gitignore",
".nvchecker.toml", ".nvchecker.toml",
"*.install", "*.install",
"*.sysusers", "*.sysusers",
"*sysusers.conf",
"*.tmpfiles", "*.tmpfiles",
"*tmpfiles.conf",
"*.logrotate", "*.logrotate",
"*.pam", "*.pam",
"*.service", "*.service",

View File

@@ -185,18 +185,10 @@ prepare_chroot() {
echo "$x" >>"$copydir/etc/makepkg.conf" echo "$x" >>"$copydir/etc/makepkg.conf"
done done
# TODO(gromit): check if this rule is sane cat > "$copydir/etc/sudoers.d/builduser-pacman" <<EOF
# TODO(gromit): this will require a full container builduser ALL = NOPASSWD: /usr/bin/pacman
cat > "$copydir/etc/polkit-1/rules.d/10-systemd-nopasswd.rules" <<EOF
polkit.addRule(function(action, subject) {
if (action.id == "org.freedesktop.systemd1.manage-units") {
if (subject.isInGroup("wheel")) {
return polkit.Result.YES;
}
}
});
EOF EOF
chmod 440 "$copydir/etc/polkit-1/rules.d/10-systemd-nopasswd.rules" chmod 440 "$copydir/etc/sudoers.d/builduser-pacman"
cat > "$copydir/etc/gitconfig" <<EOF cat > "$copydir/etc/gitconfig" <<EOF
[safe] [safe]
@@ -230,14 +222,17 @@ _chrootbuild() {
# shellcheck source=/dev/null # shellcheck source=/dev/null
. /etc/profile . /etc/profile
run0 --setenv=SOURCE_DATE_EPOCH \ # Beware, there are some stupid arbitrary rules on how you can
--setenv=BUILDTOOL \ # use "$" in arguments to commands with "sudo -i". ${foo} or
--setenv=BUILDTOOLVER \ # ${1} is OK, but $foo or $1 isn't.
--via-shell --chdir='~' \ # https://bugzilla.sudo.ws/show_bug.cgi?id=765
--user=builduser -- bash -c 'cd /startdir; makepkg "$@"' -bash "$@" sudo --preserve-env=SOURCE_DATE_EPOCH \
--preserve-env=BUILDTOOL \
--preserve-env=BUILDTOOLVER \
-iu builduser bash -c 'cd /startdir; makepkg "$@"' -bash "$@"
ret=$? ret=$?
case $ret in case $ret in
0) 0|14)
return 0;; return 0;;
*) *)
return $ret;; return $ret;;
@@ -248,7 +243,7 @@ _chrootnamcap() {
pacman -S --needed --noconfirm namcap pacman -S --needed --noconfirm namcap
for pkgfile in /startdir/PKGBUILD /pkgdest/*; do for pkgfile in /startdir/PKGBUILD /pkgdest/*; do
echo "Checking ${pkgfile##*/}" echo "Checking ${pkgfile##*/}"
run0 --user=builduser -- namcap "$pkgfile" 2>&1 | tee "/logdest/${pkgfile##*/}-namcap.log" sudo -u builduser namcap "$pkgfile" 2>&1 | tee "/logdest/${pkgfile##*/}-namcap.log"
done done
} }
@@ -257,12 +252,8 @@ download_sources() {
chown "$makepkg_user:" "$WORKDIR" chown "$makepkg_user:" "$WORKDIR"
# Ensure sources are downloaded # Ensure sources are downloaded
run0 --user="$makepkg_user" \ sudo -u "$makepkg_user" --preserve-env=GNUPGHOME,SSH_AUTH_SOCK \
--setenv=GNUPGHOME \ env SRCDEST="$SRCDEST" BUILDDIR="$WORKDIR" \
--setenv=SSH_AUTH_SOCK \
--setenv=SRCDEST="$SRCDEST" \
--setenv=BUILDDIR="$WORKDIR" \
--chdir=. -- \
makepkg --config="$copydir/etc/makepkg.conf" --verifysource -o "${verifysource_args[@]}" || makepkg --config="$copydir/etc/makepkg.conf" --verifysource -o "${verifysource_args[@]}" ||
die "Could not download sources." die "Could not download sources."
} }
@@ -409,7 +400,7 @@ if arch-nspawn "$copydir" \
"${nspawn_build_args[@]}" \ "${nspawn_build_args[@]}" \
/chrootbuild "${makepkg_args[@]}" /chrootbuild "${makepkg_args[@]}"
then then
mapfile -t pkgnames < <(run0 --user="$makepkg_user" -- bash -c 'source PKGBUILD; printf "%s\n" "${pkgname[@]}"') mapfile -t pkgnames < <(sudo -u "$makepkg_user" bash -c 'source PKGBUILD; printf "%s\n" "${pkgname[@]}"')
move_products move_products
else else
(( ret += 1 )) (( ret += 1 ))
@@ -462,7 +453,7 @@ else
done done
msg2 "Checking packages" msg2 "Checking packages"
run0 --user="$makepkg_user" -- checkpkg --rmdir --warn --makepkg-config "$copydir/etc/makepkg.conf" "${remotepkgs[@]/#file:\/\//}" sudo -u "$makepkg_user" checkpkg --rmdir --warn --makepkg-config "$copydir/etc/makepkg.conf" "${remotepkgs[@]/#file:\/\//}"
fi fi
true true
fi fi

View File

@@ -192,7 +192,7 @@ for p in "$@"; do
pkgfile=${pkgfile_remote#file://} pkgfile=${pkgfile_remote#file://}
if [[ ! -f ${pkgfile} ]]; then if [[ ! -f ${pkgfile} ]]; then
msg "Downloading package '%s' into pacman's cache" "${pkgfile}" msg "Downloading package '%s' into pacman's cache" "${pkgfile}"
run0 -- pacman -Swdd --noconfirm --logfile /dev/null "${p}" || exit 1 sudo pacman -Swdd --noconfirm --logfile /dev/null "${p}" || exit 1
pkgfile_remote=$(pacman -Sddp "${p}" 2>/dev/null) pkgfile_remote=$(pacman -Sddp "${p}" 2>/dev/null)
pkgfile="${pkgfile_remote#file://}" pkgfile="${pkgfile_remote#file://}"
fi fi