[pacman-dev] Adds a --nowait option

Message ID CABLa6CQakdxLQt0Y8d_Ub+JgNwJivHc62fn1vdx9Az5QbVJKOw@mail.gmail.com
State Under Review
Headers show
Series [pacman-dev] Adds a --nowait option | expand

Commit Message

Daniel Radetsky Feb. 12, 2021, 5:21 p.m. UTC
Apologies if gmail ruins this. I'm used to github prs & don't have time to
setup mutt at the moment.

If pacman is in use, makepkg -si will wait until it is available. This
can be undesirable if you aren't running makepkg -si in the normal
interactive way. For example, if you're running it from ansible (so
shell output is supressed), and you also have a pacman -Syuw running in
another terminal which you forgot to confirm, it will hang forever and
you won't know why.

More generally, if you're running it in any kind of non-interactive
environment, you may want it to fail fast instead of waiting for some
other condition to resolve.

This is kind of a work-in-progress. I'm not totally sure what to do with
the .po files (do I manually fix all the line number comments? God, I
hope not). And maybe --nowait is a bad name; I'd have gone with a longer
name normally like --no-wait-for-pacman, since I don't expect this to be
something a user is typing a lot in a terminal, but writing and
copypasta-ing in a script. But I also didn't want to buck the
conventions. Whatever, I can change it.

Signed-off-by: Daniel Radetsky <dradetsky@gmail.com>
---
 scripts/makepkg.sh.in | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

use")\n"
  printf -- "$(gettext "  --packagelist    Only list package filepaths that
would be produced")\n"
  printf -- "$(gettext "  --printsrcinfo   Print the generated SRCINFO and
exit")\n"
  printf -- "$(gettext "  --sign           Sign the resulting package with
%s")\n" "gpg"
@@ -1030,10 +1036,11 @@ ARGLIST=("$@")
 # Parse Command Line Options.
 OPT_SHORT="AcCdefFghiLmop:rRsSV"
 OPT_LONG=('allsource' 'check' 'clean' 'cleanbuild' 'config:' 'force'
'geninteg'
-          'help' 'holdver' 'ignorearch' 'install' 'key:' 'log' 'noarchive'
'nobuild'
-          'nocolor' 'nocheck' 'nodeps' 'noextract' 'noprepare' 'nosign'
'packagelist'
-          'printsrcinfo' 'repackage' 'rmdeps' 'sign' 'skipchecksums'
'skipinteg'
-          'skippgpcheck' 'source' 'syncdeps' 'verifysource' 'version')
+          'help' 'holdver' 'ignorearch' 'install' 'key:' 'log' 'noarchive'
+          'nobuild' 'nocolor' 'nocheck' 'nodeps' 'noextract' 'noprepare'
+          'nosign' 'nowait' 'packagelist' 'printsrcinfo' 'repackage'
'rmdeps'
+          'sign' 'skipchecksums' 'skipinteg' 'skippgpcheck' 'source'
'syncdeps'
+          'verifysource' 'version')

 # Pacman Options
 OPT_LONG+=('asdeps' 'noconfirm' 'needed' 'noprogressbar')
@@ -1074,6 +1081,7 @@ while true; do
  --nocheck)        RUN_CHECK='n' ;;
  --noprepare)      RUN_PREPARE='n' ;;
  --nosign)         SIGNPKG='n' ;;
+ --nowait)         NOWAIT=1 ;;
  -o|--nobuild)     BUILDPKG=0 NOBUILD=1 ;;
  -p)               shift; BUILDFILE=$1 ;;
  --packagelist)    BUILDPKG=0 PACKAGELIST=1 IGNOREARCH=1;;

Comments

Allan McRae Feb. 16, 2021, 1:30 a.m. UTC | #1
On 13/2/21 3:21 am, Daniel Radetsky wrote:
> Apologies if gmail ruins this. I'm used to github prs & don't have time to
> setup mutt at the moment.
> 
> If pacman is in use, makepkg -si will wait until it is available. This
> can be undesirable if you aren't running makepkg -si in the normal
> interactive way. For example, if you're running it from ansible (so
> shell output is supressed), and you also have a pacman -Syuw running in
> another terminal which you forgot to confirm, it will hang forever and
> you won't know why.
> 
> More generally, if you're running it in any kind of non-interactive
> environment, you may want it to fail fast instead of waiting for some
> other condition to resolve.

I'm not commenting on the patch itself, but rather the idea.

My initial reaction to this was "no".  If you are running makepkg from
ansible but "pacman -Syuw" manually in another terminal, you are doing
many things wrong!

This waiting for pacman was added under the assumption that pacman
operations are relatively short, and will eventually finish.  I'm not
sure the use case that a user run pacman but switched terminals and
forgot to confirm the operation is something we should consider.

So, I am still leaning to rejecting this, but am open to being convinced
otherwise.

Allan
Eli Schwartz Feb. 16, 2021, 1:47 a.m. UTC | #2
On 2/15/21 8:30 PM, Allan McRae wrote:
> On 13/2/21 3:21 am, Daniel Radetsky wrote:
>> Apologies if gmail ruins this. I'm used to github prs & don't have time to
>> setup mutt at the moment.
>>
>> If pacman is in use, makepkg -si will wait until it is available. This
>> can be undesirable if you aren't running makepkg -si in the normal
>> interactive way. For example, if you're running it from ansible (so
>> shell output is supressed), and you also have a pacman -Syuw running in
>> another terminal which you forgot to confirm, it will hang forever and
>> you won't know why.
>>
>> More generally, if you're running it in any kind of non-interactive
>> environment, you may want it to fail fast instead of waiting for some
>> other condition to resolve.
> 
> I'm not commenting on the patch itself, but rather the idea.
> 
> My initial reaction to this was "no".  If you are running makepkg from
> ansible but "pacman -Syuw" manually in another terminal, you are doing
> many things wrong!
> 
> This waiting for pacman was added under the assumption that pacman
> operations are relatively short, and will eventually finish.  I'm not
> sure the use case that a user run pacman but switched terminals and
> forgot to confirm the operation is something we should consider.
> 
> So, I am still leaning to rejecting this, but am open to being convinced
> otherwise.

The other problem I have with this in concept is, it is based on the
notion that you need --nowait because you've redirected stdout/stderr
for makepkg to /dev/null and you have zero ways to detect a completely
infinite loop that makepkg tries hard to notify you about.

This is not a use case I'm anxious to support, but... users may
implement this themselves by using:

makepkg && makepkg --packagelist | sudo pacman --noconfirm -U -

I would consider requiring this more verbose process to be a fair
tradeoff for people that have edge case needs, rather than adding a new
and IMHO confusing user-interfacing option.
Emil Velikov Feb. 16, 2021, 11:30 a.m. UTC | #3
Hi Allan,

On Tue, 16 Feb 2021 at 01:30, Allan McRae <allan@archlinux.org> wrote:
>
> On 13/2/21 3:21 am, Daniel Radetsky wrote:
> > Apologies if gmail ruins this. I'm used to github prs & don't have time to
> > setup mutt at the moment.
> >
> > If pacman is in use, makepkg -si will wait until it is available. This
> > can be undesirable if you aren't running makepkg -si in the normal
> > interactive way. For example, if you're running it from ansible (so
> > shell output is supressed), and you also have a pacman -Syuw running in
> > another terminal which you forgot to confirm, it will hang forever and
> > you won't know why.
> >
> > More generally, if you're running it in any kind of non-interactive
> > environment, you may want it to fail fast instead of waiting for some
> > other condition to resolve.
>
> I'm not commenting on the patch itself, but rather the idea.
>
> My initial reaction to this was "no".  If you are running makepkg from
> ansible but "pacman -Syuw" manually in another terminal, you are doing
> many things wrong!
>
> This waiting for pacman was added under the assumption that pacman
> operations are relatively short, and will eventually finish.  I'm not
> sure the use case that a user run pacman but switched terminals and
> forgot to confirm the operation is something we should consider.
>
Would you be opposed to having the wait/no-wait logic within
pacman/libalpm itself?
It is something that I've wanted, plus a few friends have mentioned it as well.

Consider the following:
 - we no longer leak locking internals outside of libalpm - makepkg, tests, etc
 - pacman can take some time - relatively large update, slow internet
connection, slow dkms builds (esp. for the nvidia drivers)

The bonus point: using "pacman --wait" would greatly simplify the
locking needed for archlinux-repro.
The bug you mentioned a week or so ago - the fix is an extra set of
nested locks for a) initial setup vs b) installing asp/devtools to
fetch the PKGBUILD.

Thanks
Emil
Daniel Radetsky Feb. 24, 2021, 4:05 a.m. UTC | #4
I realize that my own use case may seem like a very unlikely set of
circumstances, but I don't think it's that crazy if you generalize it
a bit.

The point is that running `makepkg -si` from ansible isn't impossible.
Sure, maybe you shouldn't do it, but people will.
Ansible suppresses stdout/stderr. This is 100% WONTFIX as far as the
ansible people are concerned. So saying that
makepkg is trying very hard to tell you about it's infinite loop isn't
really very satisfying.

Now, I can't be the only person who's ever had a stray lockfile left
around by a pacman operation that went wrong.
Like I was midway through downloading a bunch of updates and my laptop
ran out of power of something. It happens.
Now I can fix this on my laptop easy enough, but the use case for
ansible is keeping my 80 servers in sync. The
idea that one of these is going to have a stray lockfile from
something going wrong isn't inconceivable. But as far as I
can tell, there's no way to say `pacman --am-i-locked-if-so-exit-1`.
Which is maybe what we actually want here; it's fair
enough to expect the guy writing ansible scripts to know how to record
variables from commands, to be able to look up
this flag in the pacman manual page, and only proceed with makepkg if
the test is ok.

What I don't think is desirable is requiring our hero to manually test
for the existence of /var/lib/pacman/db.lck. For one thing, I
don't think this file is referenced anywhere in the pacman
documentation. It's not in any of the man pages. `man -K db.lck` turns
up nothing. Frankly, I don't even remember how I know about it.
Probably I poked around trying to fix a bad lock & found it.

I'm not sure Eli's workaround meets the case given that we're talking
about running `makepkg -si`. It would work if
there were no deps & I didn't have to run `makepkg -s && makepkg
--packagelist`, but if I do, I think this will infinite loop
just as much as otherwise, since both pass through run_pacman.

Frankly, I'm not really sure why waiting in a loop for pacman to
unlock forever is even desirable to begin with.
I tend to think it should just fail fast. This prevents this whole
journey of discovery from occuring in the first place.
We never have to figure out why it's looping forever. But given that
it already works this way, a flag to disable
it seems reasonable. Or a way to just ask pacman if it's locked. Or something.

For example, if you were to move the looping logic into pacman itself,
and I could set an env var which would
ensure that it didn't loop, that would probably work.

Patch

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index b39433f3..a1a450dd 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -67,6 +67,7 @@  NOARCHIVE=0
 NOBUILD=0
 NODEPS=0
 NOEXTRACT=0
+NOWAIT=0
 PKGFUNC=0
 PKGVERFUNC=0
 PREPAREFUNC=0
@@ -236,6 +237,10 @@  run_pacman() {
  cmd=(su root -c "$(printf '%q ' "${cmd[@]}")")
  fi
  local lockfile="$(pacman-conf DBPath)/db.lck"
+ if (( NOWAIT )) && [[ -f $lockfile ]]; then
+ msg "$(gettext "Pacman is currently in use, exiting")"
+ exit $E_FAIL
+ fi
  while [[ -f $lockfile ]]; do
  local timer=0
  msg "$(gettext "Pacman is currently in use, please wait...")"
@@ -984,6 +989,7 @@  usage() {
  printf -- "$(gettext "  --nocheck        Do not run the %s function in
the %s")\n" "check()" "$BUILDSCRIPT"
  printf -- "$(gettext "  --noprepare      Do not run the %s function in
the %s")\n" "prepare()" "$BUILDSCRIPT"
  printf -- "$(gettext "  --nosign         Do not create a signature for
the package")\n"
+ printf -- "$(gettext "  --nowait         Exit immediately if pacman is in