Subject: Re: [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter.

From: Christopher Baines <[email-masked]>
Date: Sun, 15 Mar 2020 21:35:28 +0000

Ludovic Courtès <[email-masked]> writes:

> Hi Chris!
>
> Christopher Baines <[email-masked]> skribis:
>
>> Ludovic Courtès <[email-masked]> writes:
>>
>>> Christopher Baines <[email-masked]> skribis:
>>>
>>>> +    (with-store store
>>>> +      (parameterize
>>>> +          ((%lint-checker-store-connection store))
>>>
>>> Actually it means that now ‘guix lint’ systematically connects to the
>>> daemon.
>>
>> I guess that's the effect, were you meaning this would make a better
>> message in the commit?
>
> I mean that it’s a visible change.  Before, you could run all the
> linters but this one without having a daemon running; now you need a
> daemon up and running.

Ah, yeah, that's a good point.

>>> I wonder if we could arrange to open the connection lazily, and to
>>> somehow carry state across linter invocations.  Perhaps
>>> ‘check-derivation’ should be monadic, with a field in <checker>
>>> indicating that.  Sounds complicated though.
>>>
>>> Thoughts?
>>
>> I did wonder if the code could somehow transparently be made more
>> efficient. Quite often database clients manage a pool of connections,
>> and when you perform a database operation, a connection from the pool is
>> checked out, and then returned once you're finished. But as you say,
>> this could be complicated. I think parameters can be set with
>> connections, and I'm not quiet sure what the interface should be.
>>
>> I also did think about somehow passing the store connection in to the
>> lint checker more explicitly, but I'm not sure how to generalise that.
>
> There could be a <checker> field indicating either that (1) the
> procedure takes an optional store parameter, or that (2) the procedure
> is monadic in ‘%store-monad’.
>
> #2 seems more complicated to implement that #1 though.
>
> For #1, ‘guix lint’ could check whether:
>
>   (any checker-require-store? checkers)
>
> is true, and if it is, it could open a connection and pass it on as
> needed.
>
> WDYT?
>
> If that seems good to you, I guess you can go ahead with it (let’s just
> not lose our hair on it!).

I've finally got around to looking at this again. I've sent some new
patches which add a field to the <lint-checker> record, and adjust the
running of lint checkers as well as the derivation checker to use a
single store connection.

Let me know what you think?

Thanks,

Chris