| Message ID | 20200903013655.92224-3-polyzen@archlinux.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | Johannes Löthberg |
| Headers | show |
| Series | [1/3] paccache: Clarify sudo usage | expand |
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?
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.
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
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(-)