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: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Christopher Baines <mail <at> cbaines.net>
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: Mon, 27 May 2024 19:19:07 +0200

Hi Chris,

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.

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 <at> gnu.org>
Wed, 22 Nov 2023 11:19:42 +0100
id:87bkbmm6o1.fsf <at> gnu.org
https://issues.guix.gnu.org/67250
https://issues.guix.gnu.org/msgid/87bkbmm6o1.fsf <at> gnu.org
https://yhetil.org/guix/87bkbmm6o1.fsf <at> gnu.org

Cheers,
simon

Previous Next

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