Howdy! Christopher Baines skribis: > Ludovic Courtès writes: [...] >> Though I’m still unsure what the patch series starting at >> 7b812f7c84c43455cdd68a0e51b6ded018afcc8e was about. What was the end >> goal? > > So that was part of the creation of the (guix substitutes) module, > unpicking the code in the script to separate out some of the connection > caching was a prerequisite (discussion starts here > https://issues.guix.gnu.org/45409#5 ). > > I think separating out that module is still a good thing. It's allowed > for improvements in guix, the weather script doesn't now call in to the > substitute script code for example. I'd also like the separation for > things like the Guix Build Coordinator, which currently attempts to use > the substitute code from Guix. Right, I agree this is a worthy goal. Untangling the stateful bits is the hard part, as we see. :-) >> I also wonder if it introduced other issues. For >> example, 7b812f7c84c43455cdd68a0e51b6ded018afcc8e replaced a reference >> to ‘open-connection-for-uri/cached’ by one to >> ‘open-connection-for-uri/maybe’. Are we still using cached >> connections? > > At least on that commit, open-connection-for-uri/maybe calls > open-connection-for-uri/cached, so yes, still using cached connections. OK. >> Commit f50f5751fff4cfc6d5abba9681054569694b7a5c no longer passes the >> #:port parameter to ‘http-fetch’. > > Yeah, that change is sort of fine if you're just looking at how the > port/connection is handled, but that area is being fixed up here, and > because closing the port is something that happens, it's better to also > pass the port in. OK. >> Commit 20c08a8a45d0f137ead7c05e720456b2aea44402 does other things but at >> first sight I’m not sure what the effect is. > > So, open-connection-for-uri/maybe is like > open-connection-for-uri/cached, but it catches a couple of exceptions > relating to not being able to connect to a substitute server, it also > remembers about showing the messages. > > The second commit here is changing that slightly, to not apply to > process-substitution, however I do think that code might have applied in > the past (as open-connection-for-uri/maybe was used I believe). But I > think you're right in saying there's probably some overlap between the > error handling here and done by with-networking. Alright. >> If you’re confident we can move forward to fix the bug, that’s great >> (though we’ll need a good deal of testing), but I’d still like to >> clarify these points later on. > > Well, the changes I'm suggesting here seem reasonable to me. As for > testing, checking things basically work is easy enough, but I don't > currently have many ideas for how to test for when fetching things > doesn't go to plan (which can of course happen). I’ll do some testing of v2 on my end and report back. Thanks for the explanations! Ludo’.