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
diff
issue
license
release
repo
search

View File

@@ -14,7 +14,7 @@ Description
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
elevate permissions using passwordless run0.
elevate permissions using passwordless sudo.
Options
-------

View File

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

View File

@@ -155,7 +155,7 @@ if (( ${#needsversioning[*]} )); then
if [[ ! -f "${file}" ]]; then
continue
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"
fi
done

View File

@@ -15,11 +15,7 @@ check_root() {
local orig_argv=("$@")
(( EUID == 0 )) && return
if type -P run0 >/dev/null; then
keepenv=",$keepenv"
command="run0 ${keepenv//,/ --setenv=}"
exec ${command} -- "${orig_argv[@]}"
elif type -P sudo >/dev/null; then
if type -P sudo >/dev/null; then
exec sudo --preserve-env="${keepenv}" -- "${orig_argv[@]}"
else
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@}
# shellcheck source=src/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
@@ -20,6 +22,7 @@ pkgctl_db_move_usage() {
Move packages between binary repositories.
OPTIONS
--noconfirm Bypass any confirmation messages, should only be used with caution
-h, --help Show this help text
EXAMPLES
@@ -29,9 +32,12 @@ _EOF_
}
pkgctl_db_move() {
local SOURCE_REPO=""
local TARGET_REPO=""
local PKGBASES=()
if (( $# < 3 )); then
pkgctl_db_move_usage
exit 1
fi
local CONFIRM=1
# option checking
while (( $# )); do
@@ -40,6 +46,10 @@ pkgctl_db_move() {
pkgctl_db_move_usage
exit 0
;;
--noconfirm)
CONFIRM=0
shift
;;
-*)
die "invalid argument: %s" "$1"
;;
@@ -49,16 +59,35 @@ pkgctl_db_move() {
esac
done
if (( $# < 3 )); then
pkgctl_db_move_usage
exit 1
local source_repo=$1
local target_repo=$2
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
SOURCE_REPO=$1
TARGET_REPO=$2
shift 2
PKGBASES+=("$@")
if (( CONFIRM )); then
local pkglist
pkglist=$(printf '%s\n' "$@" | paste -sd ' ')
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
ssh "${PACKAGING_REPO_RELEASE_HOST}" db-move "${SOURCE_REPO}" "${TARGET_REPO}" "${PKGBASES[@]}"
local pkgbase
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",
"keys/**",
".SRCINFO",
".gitignore",
".nvchecker.toml",
"*.install",
"*.sysusers",
"*sysusers.conf",
"*.tmpfiles",
"*tmpfiles.conf",
"*.logrotate",
"*.pam",
"*.service",

View File

@@ -185,18 +185,10 @@ prepare_chroot() {
echo "$x" >>"$copydir/etc/makepkg.conf"
done
# TODO(gromit): check if this rule is sane
# TODO(gromit): this will require a full container
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;
}
}
});
cat > "$copydir/etc/sudoers.d/builduser-pacman" <<EOF
builduser ALL = NOPASSWD: /usr/bin/pacman
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
[safe]
@@ -230,14 +222,17 @@ _chrootbuild() {
# shellcheck source=/dev/null
. /etc/profile
run0 --setenv=SOURCE_DATE_EPOCH \
--setenv=BUILDTOOL \
--setenv=BUILDTOOLVER \
--via-shell --chdir='~' \
--user=builduser -- bash -c 'cd /startdir; makepkg "$@"' -bash "$@"
# Beware, there are some stupid arbitrary rules on how you can
# use "$" in arguments to commands with "sudo -i". ${foo} or
# ${1} is OK, but $foo or $1 isn't.
# https://bugzilla.sudo.ws/show_bug.cgi?id=765
sudo --preserve-env=SOURCE_DATE_EPOCH \
--preserve-env=BUILDTOOL \
--preserve-env=BUILDTOOLVER \
-iu builduser bash -c 'cd /startdir; makepkg "$@"' -bash "$@"
ret=$?
case $ret in
0)
0|14)
return 0;;
*)
return $ret;;
@@ -248,7 +243,7 @@ _chrootnamcap() {
pacman -S --needed --noconfirm namcap
for pkgfile in /startdir/PKGBUILD /pkgdest/*; do
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
}
@@ -257,12 +252,8 @@ download_sources() {
chown "$makepkg_user:" "$WORKDIR"
# Ensure sources are downloaded
run0 --user="$makepkg_user" \
--setenv=GNUPGHOME \
--setenv=SSH_AUTH_SOCK \
--setenv=SRCDEST="$SRCDEST" \
--setenv=BUILDDIR="$WORKDIR" \
--chdir=. -- \
sudo -u "$makepkg_user" --preserve-env=GNUPGHOME,SSH_AUTH_SOCK \
env SRCDEST="$SRCDEST" BUILDDIR="$WORKDIR" \
makepkg --config="$copydir/etc/makepkg.conf" --verifysource -o "${verifysource_args[@]}" ||
die "Could not download sources."
}
@@ -409,7 +400,7 @@ if arch-nspawn "$copydir" \
"${nspawn_build_args[@]}" \
/chrootbuild "${makepkg_args[@]}"
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
else
(( ret += 1 ))
@@ -462,7 +453,7 @@ else
done
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
true
fi

View File

@@ -192,7 +192,7 @@ for p in "$@"; do
pkgfile=${pkgfile_remote#file://}
if [[ ! -f ${pkgfile} ]]; then
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="${pkgfile_remote#file://}"
fi