[bug#31813] [PATCH] evaluate: Use a generic key to identify Cuirass arguments.

["Clément Lassieur" <clement@lassieur.org>]

Heya!

Ludovic Courtès <ludo@gnu.org> writes:

>>>> @@ -98,7 +99,7 @@ building things during evaluation~%")
>>>>                  (proc    (module-ref %user-module proc-name))
>>>>                  (commit  (assq-ref spec #:current-commit))
>>>>                  (name    (assq-ref spec #:name))
>>>> -                (args    `((,(string->symbol name)
>>>> +                (args    `((guix
>>>>                              (revision . ,commit)
>>>>                              (file-name . ,source))
>>>>                             ,@(or (assq-ref spec #:arguments) '())))
>>>
>>> If we do that, then everything is called ‘guix’.
>>
>> Why is it a problem?
>
> In theory you can have several inputs (checkouts) to a given jobset, and
> they need to have different names so that you can distinguish among
> them.

Right now 'cuirass-jobs' can't handle several checkouts, and evaluate.in
sends only one 'checkout'.  So if we want to support several inputs we
would need to modify both Guix and Cuirass.

And anyway if we force the argument key to 'guix', 'guix-checkout' or
'guix-modular', it wouldn't prevent us to add other inputs (checkouts)
later, I think.

>> I don't think the current situation is good because:
>>   - a simple mistake from the user gets their build task to silently
>>     vanish,
>>   - it is inconvenient to use guix-modular.scm with several different
>>     specifications,
>>   - that #:name key is useless if users can't choose a custom name,
>>   - allowing custom names would make it way easier to understand
>>     /api/latestbuilds.  For an example with custom names, see
>>     https://cuirass.lassieur.org:8081/api/latestbuilds?nr=100.
>
> I agree with all this.  :-)  I think the custom name should appear in
> the arguments passed to the build procedure, though.

But with our patch it *does* appear in the build procedure, because it
is still in 'eval' which is read by 'evaluate' in src/cuirass/base.scm.

--8<---------------cut here---------------start------------->8---
                (eval    `((#:specification . ,name)
                           (#:revision . ,commit)))
           pretty-print
            `(evaluation ,eval
                         ,(map (lambda (thunk) (thunk))
                               thunks))
--8<---------------cut here---------------end--------------->8---

What we want to modify is the key passed to 'proc', and it is only used
to extract the checkout argument (with 'assq-ref').

>> Using an auto-incrementing ID column could work, but I don't like it
>> for the reasons I explained above.
>
> You didn’t mention auto-incrementing ID above, did you?  It would seem
> like a simple solution to the problem.

If we were to do this,
  - that would allow several specs to have the same name,
  - that would not change the fact that users are forced to use the
    'guix-modular' (or 'guix') name for each spec.

The former is nice but not necessary (it could be a further commit), the
latter is the bug that we want to fix.

Mathieu and Clément