Christopher Allan Webber <cweb...@dustycloud.org> skribis: > Ludovic Courtès writes: > >>> +(define (ensure-gnutls) >>> + (if (not (force gnutls-module)) >>> + (throw 'gnutls-not-available "(gnutls) module not available"))) >> >> I wonder if this is the right exception, but I can’t think of anything >> better (there’s no generic “not supported” exception I think; (throw >> 'system-error … ENOSYS) would do that but it’s too vague.) > > I don't know... it's hard for me to tell when to use what exception > symbol in Guile! > > I prefer specific exceptions when a more general exception
Yes, I agree. I was just wondering out loud whether there might be another exception. Anyway, this one’s fine! >> What about leaving the ‘ensure-gnutls’ call and then simply use the >> GnuTLS symbols directly and rely on autoloading, as in (guix build >> download)? >> >> --8<---------------cut here---------------start------------->8--- >> ;; Autoload GnuTLS so that this module can be used even when GnuTLS is >> ;; not available. At compile time, this yields "possibly unbound >> ;; variable" warnings, but these are OK: we know that the variables will >> ;; be bound if we need them, because (guix download) adds GnuTLS as an >> ;; input in that case. >> >> ;; XXX: Use this hack instead of #:autoload to avoid compilation errors. >> ;; See <http://bugs.gnu.org/12202>. >> (module-autoload! (current-module) >> '(gnutls) '(make-session connection-end/client)) >> --8<---------------cut here---------------end--------------->8--- >> >> That would lead more concise and slightly more efficient code, and I >> think it would still work as expected in the absence of (gnutls). >> >> WDYT? > > So there was this converstaion on #guile: > > <civodul> mark_weaver: the autoload hack fails gracelessly when GnuTLS is > > missing > <civodul> that's fine in the context of Guix, but maybe not in a more > general > context > <paron_remote> oh :) > <paron_remote> civodul: what approach would you suggest then? > <mark_weaver> civodul: could we make it more graceful? > <civodul> yeah maybe with some explicit module hackery > <civodul> an explicit resolve-interface + module-ref > <civodul> something like that > <mark_weaver> sounds doable > > So... that's what lead me to change it. > > Admittedly I'm not totally clear what was meant by "the autoload hack > fails gracelessly", and what would be more graceful. Would it be > because it's trying to utilize a symbol that's not bound to anything? > > Which leads to the next question: if I did the autoload hack, what would > (ensure-gnutls) look like? Sorry for the confusing statements. :-) I think ‘ensure-gnutls’ would be exactly as in this patch, and the autoload hack would be exactly as shown above. Here it would fail “gracefully” in the sense that ‘ensure-gnutls’ would catch a potential problem early on and raise the ‘gnutls-not-available’ exception (you’d have to double-check that this is indeed the behavior we get, but I’m quite confident ;-)). In (guix build download) there’s no such protection so when (gnutls) is missing, users get an unbound variable right in the middle of the code. >>> + (define (read! bv start count) >>> + (define read-bv (get-bytevector-n record count)) >>> + (define read-bv-len (bytevector-length read-bv)) >>> + (bytevector-copy! read-bv 0 bv 0 read-bv-len) >>> + read-bv-len) >> >> Beware: ‘get-bytevector-n’ can return the EOF object instead of a >> number, so you need to check for that. (Conversely, ‘read!’ needs to >> return 0 to indicate EOF.) > > So that would look like this? > > (define (read! bv start count) > (define read-bv (get-bytevector-n record count)) > (if (eof-object? read-bv) > 0 > (let ((read-bv-len (bytevector-length read-bv))) > (bytevector-copy! read-bv 0 bv 0 read-bv-len) > read-bv-len))) Exactly. >>> + (define (open-socket) >>> + (let loop ((addresses addresses)) >> >> Or just “(define sock …”. > > Hm, is that a good idea? Does this need to happen before or within the > with-https-proxy? Oh you’re right, sorry. > From 91c0a4a728ca4bf2e9468cdc849c350dd3f7380f Mon Sep 17 00:00:00 2001 > From: Christopher Allan Webber <cweb...@dustycloud.org> > Date: Thu, 17 Sep 2015 15:14:54 -0500 > Subject: [PATCH] web: Add https support through gnutls. > > Since importing gnutls directly would result in a dependency cycle, > we load gnutls lazily. > > This uses code originally written for Guix by Ludovic > > * module/web/client.scm: (%http-receive-buffer-size) > (warn-no-gnutls-return-false, gnutls-module, ensure-gnutls) > (gnutls-ref, tls-wrap): New variables. > (open-socket-for-uri): Wrap in tls when uri scheme is https. > * doc/ref/web.texi (open-socket-for-uri): Document gnutls usage. [...] > @deffn {Scheme Procedure} open-socket-for-uri uri > -Return an open input/output port for a connection to URI. > +Return an open input/output port for a connection to URI. Guile > +dynamically loads gnutls for https support; for more information, see > +@xref{Guile Preparations, > +how to install the GnuTLS bindings for Guile,, gnutls-guile, > +GnuTLS-Guile}. @xref generates a “See” for the beginning of a sentence, so it should be: … support. @xref{…}, for more information. Also, “HTTPS” and “GnuTLS”. :-) The rest is all good for me, so the only remaining bits are the autoload thing and maybe the bug you observed? Thanks! Ludo’.
