bug#36375: [PATCH] Re: ‘guix package’ should lock the profile

["Ludovic Courtès" <ludo@gnu.org>]

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’.