| Message ID | 538acb9dc52f6992a5a65846db48f8b7382fb1be.camel@telenet.be |
|---|---|
| Headers | show |
| Series | Allow the github updater to update git sources | expand |
Hi, Maxime Devos <maximedevos@telenet.be> skribis: > Follow-up to <https://issues.guix.gnu.org/50072>. > > To test, you can do > > $ make check > $ ./pre-inst-env guix refresh -t github -u zig > > and verify that the version and sha256/base32 has been updated > (zig@0.9.0 doesn't work though; patches aren't applying cleanly). Nice, applied! One comment: > +(define (call-with-releases thunk tags releases) > + (mock ((guix http-client) http-fetch > + (lambda* (uri #:key headers) > + (unless (string-prefix? "mock://" uri) > + (error "the URI ~a should not be used" uri)) > + (define components > + (string-split (substring uri 8) #\/)) > + (pk 'stuff components headers) > + (define (scm->json-port scm) > + (open-input-string (scm->json-string scm))) > + (match components > + (("repos" "foo" "foomatics" "releases") > + (scm->json-port releases)) > + (("repos" "foo" "foomatics" "tags") > + (scm->json-port tags)) > + (rest (error "TODO ~a" rest))))) > + (parameterize ((%github-api "mock://")) > + (thunk)))) I think the whole point of having the ‘%github-api’ parameter is that it allows us to mock the HTTP server instead of having to override bindings such as ‘http-fetch’. I’d have a slight preference for doing that, similar to what is done in tests/cpan.scm for instance. WDYT? Thanks, Ludo’.
Ludovic Courtès schreef op zo 16-01-2022 om 23:19 [+0100]: > Hi, > > Maxime Devos <maximedevos@telenet.be> skribis: > > > Follow-up to <https://issues.guix.gnu.org/50072>. > > > > To test, you can do > > > > $ make check > > $ ./pre-inst-env guix refresh -t github -u zig > > > > and verify that the version and sha256/base32 has been updated > > (zig@0.9.0 doesn't work though; patches aren't applying cleanly). > > Nice, applied! > > One comment: > > > +(define (call-with-releases thunk tags releases) > > + (mock ((guix http-client) http-fetch [...]) > > + (parameterize ((%github-api "mock://")) > > + (thunk)))) > > I think the whole point of having the ‘%github-api’ parameter is that it > allows us to mock the HTTP server instead of having to override bindings > such as ‘http-fetch’. > > I’d have a slight preference for doing that, similar to what is done in > tests/cpan.scm for instance. WDYT? tests/cpan.scm uses 'with-http-server', which I do not find ideal because the answers the HTTP server gives depend on the order the HTTP server was queried, without verifying the URI. Mocking 'http-fetch' allows me not to worry about ordering and allows verifying the URI. It might be possible to modify 'with-http-server' into something (with-http-server*?) that allows looking at the HTTP headers and URI and dynamically generate a response based on that. Due to the mocking, %github-api isn't truly necessary, but having "https://api.github.com" in a single location helps avoiding typos like writing "http://" instead of "https://" somewhere, or adjusting the domain name if GitHub decided to change it for whatever reason (hopefully unlikely?), or if Tor becomes very popular among the general public and GitHub has an ".onion" address, then it could be changed to the ".onion" address easily, ... Greetings, Maxime.
Hi, Maxime Devos <maximedevos@telenet.be> skribis: > tests/cpan.scm uses 'with-http-server', which I do not find ideal > because the answers the HTTP server gives depend on the order the > HTTP server was queried, without verifying the URI. Mocking > 'http-fetch' allows me not to worry about ordering and allows verifying > the URI. Good point. > It might be possible to modify 'with-http-server' into something > (with-http-server*?) that allows looking at the HTTP headers and URI > and dynamically generate a response based on that. Yes, that’d be great. > Due to the mocking, %github-api isn't truly necessary, but having > "https://api.github.com" in a single location helps avoiding typos > like writing "http://" instead of "https://" somewhere, or adjusting > the domain name if GitHub decided to change it for whatever reason > (hopefully unlikely?), or if Tor becomes very popular among the general > public and GitHub has an ".onion" address, then it could be changed to > the ".onion" address easily, ... Yes, and also, setting ‘%github-api’ makes sure we don’t inadvertently talk to the real GitHub (there was a bug along these lines in the tests of one of the importers a while back.) Thanks, Ludo’.