[pacman-dev] [PATCH] repo-add: Remove lock on SIG{TERM, HUP, QUIT, INT, ERR}

[Lukas Fleischer]

On Tue, Oct 11, 2011 at 07:03:13PM +0200, Lukas Fleischer wrote:
> On Tue, Oct 11, 2011 at 11:29:31AM -0500, Dan McGee wrote:
> > On Tue, Oct 11, 2011 at 11:13 AM, Lukas Fleischer
> > <archlinux at cryptocrack.de> wrote:
> > > There's no need to keep the lock file if we interrupt repo-{add,remove}
> > > using ^C (same applies to other signals).
> > Definite +1 here.
> > 
> > > In contrast to a clean exit, we do not `rm -rf` the temporary directory
> > > here, so that it's still possible to analyze what went wrong.
> > I'm not sure what the right answer is here- I see where you're coming
> > from, but given that we give the user no indication we're leaving a
> > mess behind, I think we should either clean it up or tell the user we
> > did not. Also noticed that we print an error message with TERM even
> > when HUP and QUIT are caught.
> 
> I'm not really sure either, but I'd say that showing a message that
> includes "$tmpdir" might be the best thing to do here.
> 
> I also submitted a separate patch that fixes the SIGTERM message issue.

Ugh. It's a bit more complicated, even. Given that we use exit at the
very end of trap_exit(), this should already trigger clean_up()
automatically. The only case where this doesn't work is if we send a
signal when clean_up() is already being executed (it's not too hard to
reproduce that, actually). So just copying the two lines that unlink the
lock file and remove the directory from clean_up() might be the easiest
way to go?

> 
> > >
> > > Signed-off-by: Lukas Fleischer <archlinux at cryptocrack.de>
> > > ---
> > > Hrm. Second time I had to manually remove
> > > "/srv/ftp/community/os/$arch/community.db.tar.gz.lck" on sigurd in order
> > > not to lock the whole database after interrupting `/arch/db-update`.
> > >
> > > I hope I didn't miss anything here but this should hopefully not cause
> > > any other unforeseen issues...
> > >
> > >  scripts/repo-add.sh.in |    3 +++
> > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> > > index 29b150c..753200b 100644
> > > --- a/scripts/repo-add.sh.in
> > > +++ b/scripts/repo-add.sh.in
> > > @@ -522,6 +522,9 @@ remove() {
> > >  trap_exit() {
> > >        echo
> > >        error "$@"
> > > +
> > > +       (( CLEAN_LOCK )) && [[ -f $LOCKFILE ]] && rm -f "$LOCKFILE"
> > > +
> > >        exit 1
> > >  }
> > >
> > > --
> > > 1.7.7
> > >
> > >
> > >
> >