[bug#71038] [PATCH 1/2] guix: store: Enable specifying the available builtin builders.

[Simon Tournier <zimon.toutoune@gmail.com>]

Hi Chris,

On Sun, 26 May 2024 at 09:10, Christopher Baines <mail@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.

Considering your patch, how do I know that I could used it, as [1]:

--8<---------------cut here---------------start------------->8---
As in:

  (open-connection
    #:assume-available-builtin-builders '("download"))
--8<---------------cut here---------------end--------------->8---

Because it is not clear from the docstring.  And there is many
procedures that would require some docstring update with this new
procedure argument. :-)


1: bug#67250: builtin:git-download capability detection not working for the bordeaux build farm
Ludovic Courtès <ludo@gnu.org>
Wed, 22 Nov 2023 11:19:42 +0100
id:87bkbmm6o1.fsf@gnu.org
https://issues.guix.gnu.org/67250
https://issues.guix.gnu.org/msgid/87bkbmm6o1.fsf@gnu.org
https://yhetil.org/guix/87bkbmm6o1.fsf@gnu.org

Cheers,
simon