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

From: Ludovic Courtès <[email-masked]>
Date: Fri, 25 Oct 2019 23:21:34 +0200

Hello,

Julien Lepiller <[email-masked]> skribis:

> attached is a patch for guix package to grab a lock file. Note that I'm
> using flock, so it won't work on NFS shares. The other option would be
> to use fcntl but guile doesn't seem to implement the locking function
> from it.

(guix build syscalls) has it though, so you should probably use it.  :-)

> From 987e9711f1fa6bfd270e48ee5624f69696e7e5c4 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> 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.

[...]

> -  ;; First, process roll-backs, generation removals, etc.
> +  ;; First, acquire a lock on the profile, to ensure only one guix process
> +  ;; is modifying it at a time.
> +  (define lock-file (open (string-append profile ".lock") O_CREAT))
> +  (catch 'system-error
> +    (lambda _
> +      (flock lock-file (logior LOCK_EX LOCK_NB)))
> +    (lambda (key . args)
> +      (leave (G_ "profile ~a is being locked by another guix process.~%")
> +                 profile)))

Nitpick: "profile ~a is locked by another process~%".

> +  ;; Then, process roll-backs, generation removals, etc.
>    (for-each (match-lambda
>                ((key . arg)
>                 (and=> (assoc-ref %actions key)
> @@ -905,7 +915,10 @@ processed, #f otherwise."
>                               #:allow-collisions? allow-collisions?
>                               #:bootstrap? bootstrap?
>                               #:use-substitutes? substitutes?
> -                             #:dry-run? dry-run?))))
> +                             #:dry-run? dry-run?)))
> +
> +  ;; Finaly, close the lock file
> +  (close lock-file))

I’d recommend wrapping the body in ‘with-file-lock’ (from (guix build
syscalls)), which handles non-local exits.

However you’d first need to add a #:wait? argument to ‘with-file-lock’
and perhaps an additional argument to handle the already-locked case.
Or maybe call that ‘with-file-lock/no-wait’.

How does that sound?

Thanks for working on it!

Ludo’.