[PATCH 1/3] paccache: Clarify sudo usage
"Privilege escalation required" sounds more like an error. Signed-off-by: Daniel M. Capella <polyzen@archlinux.org> --- src/paccache.sh.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/paccache.sh.in b/src/paccache.sh.in index 1311fd7..fa53d58 100644 --- a/src/paccache.sh.in +++ b/src/paccache.sh.in @@ -138,11 +138,11 @@ m4_include(../lib/size_to_human.sh) runcmd() { if (( needsroot && EUID != 0 )); then - msg "Privilege escalation required" + msg 'Escalating privileges using sudo' if sudo -v &>/dev/null && sudo -l &>/dev/null; then sudo "$@" else - die 'Unable to escalate privileges using sudo' + die 'Failed to escalate' fi else "$@" -- 2.28.0
Signed-off-by: Daniel M. Capella <polyzen@archlinux.org> --- src/paccache.sh.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/paccache.sh.in b/src/paccache.sh.in index fa53d58..5ab467e 100644 --- a/src/paccache.sh.in +++ b/src/paccache.sh.in @@ -162,7 +162,7 @@ summarize() { printf -v output "finished: %d packages moved to '%s'" "$filecount" "$movedir" elif (( dryrun )); then if (( verbose )); then - msg "Candidate packages:" + msg 'Candidate packages:' while read -r pkg; do if (( verbose >= 3 )); then [[ $pkg =~ $pkg_re ]] && name=${BASH_REMATCH[1]} arch=${BASH_REMATCH[2]} @@ -330,8 +330,8 @@ whitelist=("$@") # sanity checks case $(( dryrun+delete+move )) in - 0) die "no operation specified (use -h for help)" ;; - [^1]) die "only one operation may be used at a time" ;; + 0) die 'no operation specified (use -h for help)' ;; + [^1]) die 'only one operation may be used at a time' ;; esac [[ $movedir && ! -d $movedir ]] && -- 2.28.0
runcmd() taken from paccache. Fixes FS#64328 Signed-off-by: Daniel M. Capella <polyzen@archlinux.org> --- src/checkupdates.sh.in | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/checkupdates.sh.in b/src/checkupdates.sh.in index ba9b960..67ff144 100644 --- a/src/checkupdates.sh.in +++ b/src/checkupdates.sh.in @@ -30,6 +30,24 @@ USE_COLOR=0 source "$LIBRARY"/util/message.sh source "$LIBRARY"/util/parseopts.sh +die() { + error "$@" + exit 1 +} + +runcmd() { + if (( EUID != 0 )); then + msg 'Escalating privileges using sudo' + if sudo -v &>/dev/null && sudo -l &>/dev/null; then + sudo "$@" + else + die 'Failed to escalate' + fi + else + "$@" + fi +} + usage() { cat << __EOF__ ${myname} v${myver} @@ -80,8 +98,7 @@ else fi if ! type -P fakeroot >/dev/null; then - error 'Cannot find the fakeroot binary.' - exit 1 + die 'Cannot find the fakeroot binary' fi if [[ -z $CHECKUPDATES_DB ]]; then @@ -98,15 +115,14 @@ fi mkdir -p "$CHECKUPDATES_DB" ln -s "${DBPath}/local" "$CHECKUPDATES_DB" &> /dev/null if ! fakeroot -- pacman -Sy --dbpath "$CHECKUPDATES_DB" --logfile /dev/null &> /dev/null; then - error 'Cannot fetch updates' - exit 1 + die 'Cannot fetch updates' fi mapfile -t updates < <(pacman -Qu --dbpath "$CHECKUPDATES_DB" 2> /dev/null | grep -v '\[.*\]') if (( ${#updates[@]} )); then printf '%s\n' "${updates[@]}" if (( DOWNLOAD_CACHE )); then - sudo pacman -Sw --noconfirm "${updates[@]%% *}" --dbpath "$CHECKUPDATES_DB" --logfile /dev/null + runcmd pacman -Sw --noconfirm "${updates[@]%% *}" --dbpath "$CHECKUPDATES_DB" --logfile /dev/null fi else exit 2 -- 2.28.0
Excerpts from Daniel M. Capella's message of September 3, 2020 3:36:
+die() { + error "$@" + exit 1 +} + +runcmd() { + if (( EUID != 0 )); then + msg 'Escalating privileges using sudo' + if sudo -v &>/dev/null && sudo -l &>/dev/null; then + sudo "$@" + else + die 'Failed to escalate' + fi + else + "$@" + fi +} +
Might make more sense to move it to a lib/ script and m4_include it in both places rather than copy-pasting it around? -- Sincerely, Johannes Löthberg :: SA0DEM
On October 23, 2020 10:53:54 AM EDT, "Johannes Löthberg" <johannes@kyriasis.com> wrote:
Excerpts from Daniel M. Capella's message of September 3, 2020 3:36:
+die() { + error "$@" + exit 1 +} + +runcmd() { + if (( EUID != 0 )); then + msg 'Escalating privileges using sudo' + if sudo -v &>/dev/null && sudo -l &>/dev/null; then + sudo "$@" + else + die 'Failed to escalate' + fi + else + "$@" + fi +} +
Might make more sense to move it to a lib/ script and m4_include it in
both places rather than copy-pasting it around?
`runcmd()` here doesn't use a `needsroot` variable as in paccache. `pacman -Sw` always needs root afaik. -- Best, Daniel <https://danielcapella.com>
Excerpts from Daniel M. Capella's message of November 1, 2020 21:04:
On October 23, 2020 10:53:54 AM EDT, "Johannes Löthberg" <johannes@kyriasis.com> wrote:
Excerpts from Daniel M. Capella's message of September 3, 2020 3:36:
+die() { + error "$@" + exit 1 +} + +runcmd() { + if (( EUID != 0 )); then + msg 'Escalating privileges using sudo' + if sudo -v &>/dev/null && sudo -l &>/dev/null; then + sudo "$@" + else + die 'Failed to escalate' + fi + else + "$@" + fi +} +
Might make more sense to move it to a lib/ script and m4_include it in
both places rather than copy-pasting it around?
`runcmd()` here doesn't use a `needsroot` variable as in paccache. `pacman -Sw` always needs root afaik.
I still think that they're similar enough that it would make sense to just have one function that takes an argument saying which user the command should be run as. However, since it's just used in two places for now we could leave it as is, and just merge it if we get a third use. `die` should be merged however, but that's a separate thing, since it's already duplicated in multiple places. Applied. -- Sincerely, Johannes Löthberg :: SA0DEM
Patch 1 and 2 applied. -- Sincerely, Johannes Löthberg :: SA0DEM
participants (2)
-
Daniel M. Capella
-
Johannes Löthberg