Le Wed, 06 Nov 2019 14:24:54 +0100,
Ludovic Courtès <ludo@gnu.org> a écrit :
Toggle quote (88 lines)
> Hello!
>
> Julien Lepiller <julien@lepiller.eu> skribis:
>
> >>From 5d86226f318a111cc1bdf5a6f044c6f540f51b45 Mon Sep 17 00:00:00
> >>2001
> > From: Julien Lepiller <julien@lepiller.eu>
> > Date: Fri, 25 Oct 2019 21:39:21 +0200
> > Subject: [PATCH] guix: package: lock profiles when processing them.
> >
> > * guix/scripts/package.scm (process-actions): Get a per-profile
> > lock to prevent concurrent actions on profiles.
> > * guix/build/syscalls.scm (with-file-lock/no-wait): New procedure.
> > (lock-file): Take a #:wait? key.
>
> Nice! Could you make the syscalls.scm changes a separate patch?
>
> > +(define (call-with-file-lock/no-wait file thunk handler)
> > + (let ((port (catch 'system-error
> > + (lambda ()
> > + (catch 'flock-error
> > + (lambda ()
> > + (lock-file file #:wait? #f))
> > + handler))
> > + (lambda args
> > + ;; When using the statically-linked Guile in the
> > initrd,
> > + ;; 'fcntl-flock' returns ENOSYS
> > unconditionally. Ignore
> > + ;; that error since we're typically the only
> > process running
> > + ;; at this point.
> > + (if (or (= ENOSYS (system-error-errno args)) (=
> > 'flock-error args))
>
> Please remove tabs. :-)
>
> This is wrong because (1) ‘args’ is always a list, and (2) ‘=’ is
> defined for numbers, not for symbols and lists.
>
> I think you actually want to catch two exceptions here: ‘system-error’
> and ‘flock-error’. For that, you have to do:
>
> (catch #t
> (lambda ()
> (lock-file …))
> (lambda (key . args)
> (match key
> ('flock-error …)
> ('system-error
> (if (= ENOSYS (system-error-errno (cons key args)))
> …))
> (_
> (apply throw key args)))))
>
> Does that make sense?
>
> > + ;; First, acquire a lock on the profile, to ensure only one guix
> > process
> > + ;; is modifying it at a time.
> > + (with-file-lock/no-wait
> > + (string-append profile ".lock")
>
> Nitpick: I’d move the lock file name on the same line as
> ‘with-file-lock/no-wait’.
>
> > + (lambda (key . args)
> > + (leave (G_ "profile ~a is locked by another guix process.~%")
> > + profile))
>
> s/guix// and remove the trailing period.
>
> Could you add a test for that in tests/guix-package.sh?
>
> One way to do it may be to do something like:
>
> echo '(sleep 60) > /…/manifest.scm
> guix package -m /…/manifest.scm -p whatever &
> pid=$!
> if guix install emacs -p whatever; then false; else true; fi
> kill $pid
>
> Could you send updated patches?
>
> Thanks!
>
> Ludo’.
Attached are updated patches! I also made sure the new test passes.