GNU bug report logs - #41119
[PATCH] fix some issues with (guix nar)

Previous Next

Full log

Message #8 received at 41119 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Caleb Ristvedt <caleb.ristvedt <at> cune.org>
Cc: 41119 <at> debbugs.gnu.org
Subject: Re: [bug#41119] [PATCH] fix some issues with (guix nar)
Date: Thu, 07 May 2020 10:05:08 +0200

Hi!

Caleb Ristvedt <caleb.ristvedt <at> cune.org> skribis:

> From b2c66b443bd42e05820cfb3920c96f1894820587 Mon Sep 17 00:00:00 2001
> From: Caleb Ristvedt <caleb.ristvedt <at> cune.org>
> Date: Wed, 6 May 2020 11:48:21 -0500
> Subject: [PATCH 1/2] nar: 'finalize-store-file' follows proper store lock
>  protocol.
>
> * guix/nar.scm (finalize-store-file): check for deletion token when acquiring
>   lock, write deletion token and delete lock file before releasing lock.

[...]

> +  ;; TODO: make this reusable
> +  (define (acquire-lock filename)

For consistency, s/filename/file/ please.  :-)

> +    (let ((port (lock-file filename)))
> +      (if (zero? (stat:size (stat port)))
> +          port
> +          (begin

Could you add a comment, like: “If FILE is non-empty, that’s because it
contains the deletion token, so try again.”

>          (when lock?
> +          (delete-file (string-append target ".lock"))
> +          (display "d" lock)
> +          (force-output lock)

Also a comment explaining why we’re writing a deletion token.

It’s a fine point of the daemon that I had totally overlooked.  I wonder
what the implications might have been.

> From 43ee61b405b01038b3e7c84aba64521ab8a62236 Mon Sep 17 00:00:00 2001
> From: Caleb Ristvedt <caleb.ristvedt <at> cune.org>
> Date: Wed, 6 May 2020 11:52:16 -0500
> Subject: [PATCH 2/2] nar: 'with-temporary-store-file' uses a single connection
>
> Previously the 'with-store' form was entered every time a different temporary
> file was tried.  This caused there to be as many simultaneous open connections
> as there were attempts, and prevented the (loop ...) call from being a tail
> call.  This change fixes that.
>
> * guix/nar.scm (with-temporary-store-file): open connection once prior to
>   entering the loop.

LGTM!

You can push both patches to ‘master’ (make sure “make authenticate”
passes before you do).

Thanks a lot for the quick fixes!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.