Subject: Re: [bug#45409] [PATCH v3 3/3] guix: Split (guix substitutes) from (guix scripts substitute).

From: Christopher Baines <[email-masked]>
Date: Thu, 07 Jan 2021 22:29:23 +0000

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

> Christopher Baines <[email-masked]> skribis:
>
>> This means there's a module for working with substitutes, rather than all the
>> code sitting in the script. The need for this can be seen with the weather and
>> challenge scripts, that now don't have to use code from the substitute script,
>> but can instead use the substitute module.
>>
>> The separation here between the actual functionality of the substitute script
>> and the underlying functionality used both there and elsewhere should make
>> maintenance easier moving forward.
>>
>> This commit just moves code, none of the code should have been changed
>> significantly.
>>
>> * guix/scripts/substitute.scm (%narinfo-cache-directory, %narinfo-ttl,
>> %narinfo-negative-ttl, %narinfo-transient-error-ttl, %unreachable-hosts,
>> %max-cached-connections): Move variables to (guix substitutes).
>> (narinfo-cache-file, cached-narinfo, cache-narinfo!, narinfo-request, at-most,
>> http-multiple-get, read-to-eof, narinfo-from-file,
>> open-connection-for-uri/maybe, fetch-narinfos, lookup-narinfos,
>> lookup-narinfos/diverse, open-connection-for-uri/cached,
>> call-with-cached-connection): Move procedures to (guix substitutes).
>> (with-cached-connection): Move syntax rule to (guix substitutes).
>> * guix/substitutes.scm: New file.
>> * Makefile.am (MODULES): Add it.
>> * po/guix/POTFILES.in: Add 'guix/substitutes.scm'.
>
> I’m reluctant starting this new module while it still contains
> single-short-lived-process assumptions (connection caching & co.).
>
> How about proceeding like this:
>
>   1. Move ‘http-multiple-get’ to (guix http-client).
>
>   2. Postpone the (guix substitute) bit to a separate patch series to
>      leave us the time to polish things a bit and removes the
>      single-process assumptions, or just move fewer things to (guix
>      substitutes).
>
> You could push (guix narinfo) in the meantime since I think that one is
> almost ready.
>
> How does that sound?  Am I being too cautious?

Well, separating out the connection caching might be helpful for
cleaning things up in the Guix Build Coordinator, I had to add a mutex
currently as I'm guessing the caching isn't thread safe.

I think it's possible to separate it out if some of the error handling
is pushed down in to the http procedures, and if when they get an error
indicating the connection is unusable, they close the port.

I've pushed some rough commits for this to this branch:

  https://git.cbaines.net/guix/log/?h=prepare-to-move-guix-scripts-substitute-code

I'm still struggling with the tests, currently make check hangs, I think
on the challenge tests, and I don't currently have a plan to work out
why the test is hanging.