Subject: Re: bug#52555: [RFC PATCH 0/3] Decentralized substitute distribution with ERIS

From: Ludovic Courtès <[email-masked]>
Date: Sat, 14 Jan 2023 19:42:43 +0100

pukkamustard <[email-masked]> skribis:

> * guix/eris.scm: New file.
> * guix/eris/fs-store.scm: New file.
> * Makefile.am (MODULES): Add new files.
> * guix/scripts/publish.scm (bake-narinfo+nar): Use guix-eris-block-reducer.

[...]

> +(define %eris-block-store-directory
> +  (make-parameter
> +   (or (getenv "GUIX_ERIS_BLOCK_STORE_DIRECTORY")
> +       (string-append %state-directory "/eris"))))
> +
> +(define (guix-eris-block-reducer)
> +  "Returns a block reducer that stores blocks of ERIS encoded content."
> +  (eris-fs-store-reducer (%eris-block-store-directory)))

Maybe this should be private to (guix scripts publish)?

Also, the store directory should be /var/cache/guix/publish/eris by
default IMO.

> +(define (eris-fs-store-reducer store-directory)
> +  (case-lambda
> +    (() (mkdir-p store-directory))
> +
> +    ((result) result)
> +
> +    ((_ ref-block)
> +     (let* ((ref (car ref-block))
> +	    (b32 (base32-encode ref))
> +	    (pre (substring b32 0 2))
> +	    (suf (substring b32 2))
> +	    (pre-dir (string-append store-directory "/" pre))
> +	    (path (string-append pre-dir "/" suf))
> +	    (block (cdr ref-block)))
> +
> +       (mkdir-p pre-dir)
> +
> +       (unless (file-exists? path)
> +	 (call-with-output-file path
> +	   (lambda (port) (put-bytevector port block))
> +	   #:binary #t))
> +
> +       #t))))
> +
> +(define (eris-fs-store-ref store-directory)
> +  (lambda (ref)
> +    (let* ((b32 (base32-encode ref))
> +	   (pre (substring b32 0 2))
> +	   (suf (substring b32 2))
> +	   (path (string-append store-directory "/" pre "/" suf)))
> +      (if (file-exists? path)
> +	  (call-with-input-file path
> +	    (lambda (port) (get-bytevector-all port))
> +	    #:binary #t)
> +	  #f))))

Could you add docstrings, remove tabs, and use (ice-9 match) instead of
car/cdr?  :-)

Whole files (blocks, right?) get loaded in memory.  Is that OK or should
it be avoided?

There are time-of-check-to-time-of-use race conditions with those
‘file-exists?’ calls.  In the case of ‘ref’, you can instead write:

  (catch 'system-error
    (lambda ()
      (call-with-input-file …))
    (lambda args
      (if (= ENOENT (system-error-errno args))
          #f
          (apply throw args))))

In the case of ‘reducer’, perhaps it’d be safer to write the block
atomically with ‘with-atomic-file-output’?  (And remove the
‘file-exists?’ check too?)

Write “file” rather than “path” (the latter is used to refer to “search
paths”).  :-)

Ludo’.