GNU bug report logs - #71038
[PATCH 0/2] Enable specifying the available builtin builders.

Previous Next

Full log

View this message in rfc822 format

From: Christopher Baines <mail <at> cbaines.net>
To: Simon Tournier <zimon.toutoune <at> gmail.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Mathieu Othacehe <othacehe <at> gnu.org>, Ludovic Courtès <ludo <at> gnu.org>, Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>, Christopher Baines <guix <at> cbaines.net>, 71038 <at> debbugs.gnu.org
Subject: [bug#71038] [PATCH 1/2] guix: store: Enable specifying the available builtin builders.
Date: Tue, 11 Jun 2024 20:26:22 +0100

[Message part 1 (text/plain, inline)]

Simon Tournier <zimon.toutoune <at> gmail.com> writes:

> On Sun, 26 May 2024 at 09:10, Christopher Baines <mail <at> cbaines.net> wrote:
>
>>>>  (define* (open-connection #:optional (uri (%daemon-socket-uri))
>>>>                            #:key port (reserve-space? #t) cpu-affinity
>>>> -                          non-blocking?)
>>>> +                          non-blocking? assume-available-builtin-builders)
>>>
>>> Why add the variable %assume-available-builtin-builders and default to
>>> it?
>>>
>>> Something like:
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> (define %assume-available-builtin-builders
>>>   "List of builtin builders supported by the builder Guix daemon."
>>>   (list "download" "git-download"))
>>>
>>> (define* (open-connection #:optional (uri (%daemon-socket-uri))
>>>                           #:key port (reserve-space? #t) cpu-affinity
>>>                           non-blocking?)
>>>                           non-blocking?
>>>                           (assume-available-builtin-builders %assume-available-builtin-builders))
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>> And then default to this %assume-available-builtin-builders elsewhere in
>>> [PATCH 2/2].  IMHO, it changes almost nothing but it would help to know
>>> (document) what to pass as argument.
>>
>> I think it's sensible to not use a fixed list by default, but check what
>> the daemon supports.
>
> Do you mean dynamically construct the proposal of
> %assume-available-builtin-builders?  Why not.
>
> Aside, my point is to provide a default value for the new argument and
> not let it free.  Because when reading the source code, not knowing its
> type, neither any meaningful value make it hard to remember what it use
> or how to use it, IMHO.  That’s why I am suggesting something like
> %assume-available-builtin-builders that collects the acceptable values
> – for the most recent daemon, indeed; well it would simplify the
> documentation of this new parameter / argument.

I'm not sure I follow. I guess open-connection could have a
#:build-in-builders argument that expects a procedure that takes the
store connection, and returns the list of strings. That would allow it
to have the default of %built-in-builders from (guix store).

While this adds the flexibility for users to provide their own way of
setting the builtin builders enabled on a connection, I'm not sure
there's a need for it currently and I don't think it addresses your
concern about not knowing what value to provide.

It sounds easier just to make it clear from the docstrings that you
provide a list of strings, and have it default to #f to indicate to the
daemon's buildin builders.

[signature.asc (application/pgp-signature, inline)]

Previous Next

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