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 can't be
found appropriately... at lest you'll catch the right one if you try to
catch it in such a case.  I also like that the above exception helps the
user realize what isn't installed so they can resolve it.

But if someone defines something concrete they'd prefer we can switch to
that.

>> +(define (gnutls-ref symbol)
>> +  "Fetch method-symbol from the gnutls module"
>> +  (module-ref (force gnutls-module) symbol))
>> +
>>  (define current-http-proxy
>>    (make-parameter (let ((proxy (getenv "http_proxy")))
>>                      (and (not (equal? proxy ""))
>>                           proxy))))
>>  
>> +(define (tls-wrap port server)
>> +  "Return PORT wrapped in a TLS connection to SERVER.  SERVER must be a DNS
>> +host name without trailing dot."
>> +  (define (log level str)
>> +    (format (current-error-port)
>> +            "gnutls: [~a|~a] ~a" (getpid) level str))
>> +
>> +  (ensure-gnutls)
>> +
>> +  (let ((session ((gnutls-ref 'make-session)
>> +                  (gnutls-ref 'connection-end/client))))
>
> 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?  I think it's not nice to throw an exception
that the symbol is simply not in the current environment; that's not
helpful for a user.  (We'll still need to ensure that gnutls-version
resolves to a procedure anyway, given the bug I added the comment
about.)

>> +      (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)))

>> +  (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?

> Otherwise works for me!
>
> Could you document HTTPS support in the doc of ‘open-socket-for-uri’
> (info "(guile) Web Client")?  Probably with something like:
>
>   @xref{Guile Preparations,
>   how to install the GnuTLS bindings for Guile,, gnutls-guile,
>   GnuTLS-Guile}, for more information.

Done.

> Thank you Chris!
>
> Ludo’.

Updated patch attached.  Still needs advisement on the exception and
autoload bits though!

 - Chris