[PATCH] add `files-tree' data source

[Sean Whitton]

Hello David,

On Mon 06 Dec 2021 at 09:51AM -04, David Bremner wrote:

> This allows the user to conveniently maintain a set of files in their consfig
> tree (or elsewhere) for deployment as data.
>
> Signed-off-by: David Bremner <david at tethera.net>
> ---
>
> I've been using this for a week or so, and the simple version handling
> seems good enough for my needs.

Thanks for this.  Happy to go with just FILE-WRITE-DATE for now.

> diff --git a/src/data/files-tree.lisp b/src/data/files-tree.lisp
> new file mode 100644
> index 0000000..5192c39
> --- /dev/null
> +++ b/src/data/files-tree.lisp
> +
> +(defmethod register-data-source ((type (eql :files-tree)) &key location version)
> +  "Provide the contents of a local directory on the machine running the root
> +Lisp. Register this data source multiple times to provide multiple trees.
> +LOCATION, IDEN1 and IDEN2 are joined to generate the path of a data file on
> +disk.  The version of the data provided is either VERSION or the file's last
> +modification time. This data source treats IDEN1 literally, so e.g. to use
> +IDEN1 = `_config', the user should create a subdirectory `_config' under
> +LOCATION."

If you could remember, please use two spaces after periods in docs.

I don't understand why there would be a VERSION parameter to
REGISTER-DATA-SOURCE -- do you have a use case in mind?  If not, maybe
we should just always use FILE-WRITE-DATE.  DATA:LOCAL-FILE only
provides a single file so VERSION makes sense there, I think, but I'm
not sure how it could make sense to supply a single version for the
whole tree.

At first I thought that IDEN1 and IDEN2 should be roundtripped through
STRING->FILENAME and FILENAME->STRING.  But that would make manual
management of the tree of files a pain.  I guess it's okay because the
data source won't ever be creating any files, only reading them.

It seems that what's intended is just to join LOCATION, IDEN1 and IDEN2
up into a pathname, leaving the user to organise which bits of the
pathname are in IDEN1 and which bits are in IDEN2, right?  I believe
that's what you're getting at in your remarks on _config.  Maybe you
could just say that explicitly, and that directory separators within
each of them count?  Otherwise, it's a bit obscure what you mean by
"literally" and why it only(?) applies to IDEN1.

In that case, perhaps you could specify that the directory separator
within IDEN1 and IDEN2 is always a forward slash, and then use
UIOP:PARSE-UNIX-NAMESTRING to obtain the pathnames to be joined.

I guess most people will store this tree of files in their consfig.  It
would be convenient not to have to specify the path to wherever you
happen to have cloned your consfig.  How about if LOCATION is a symbol,
then the root of the tree of files defaults to

    (merge-pathnames "data/" (asdf:system-source-directory location))

or some other way to avoid having to specify where your consfig is?

> +  (unless (directory-exists-p location)
> +    (error 'missing-data-source
> +           :text (format nil "~A does not exist, or is not a directory." location)))

Line is a bit long.  Please rearrange so it all fits in 80.

> +  (labels ((%make-path (iden1 iden2)
> +             (let* ((dir (pathname-directory location))
> +                    (name (pathname-name location))
> +                    (path (if name (append dir (list name)) dir)))
> +               (make-pathname :directory (append path (list iden1)) :name iden2)))

Haven't tested this, and I know you have another version, but here is
something that ought to work: IDEN1=foo.example.org IDEN2=/etc/conf.conf
and the file ought to be location/foo.example.org/etc/conf.conf.  This
facilitates (file:host-data-uploaded "/etc/conf.conf").

> +    (cons #'check #'extract)))
> +
> +
> +

Some extra whitespace here, it would seem.

-- 
Sean Whitton