Subject: Re: [bug#36048] [PATCH] guix: import: hackage: handle hackage revisions

From: Robert Vollmert <[email-masked]>
Date: Thu, 13 Jun 2019 21:40:15 +0200

> On 13. Jun 2019, at 20:09, Timothy Sample <[email-masked]> wrote:
> 
> Hi Robert,
> 
> Robert Vollmert <[email-masked]> writes:
> 
>> Hi Timothy,
>> 
>> thanks for the detailed feedback, this is very helpful!
> 
> You’re welcome!
> 
>> I’ve sent an updated patch addressing some of the concerns, but have
>> some questions regarding others. (I just realized that the documentation
>> updates probably anticipate multiple return values.)
> 
> Yes.
> 
>>> On 13. Jun 2019, at 04:28, Timothy Sample <[email-masked]> wrote:
>>>> +  (let-values (((port get-hash) (open-sha256-input-port port)))
>> 
>>>> +    (cons
>>>> +      (read-cabal (canonical-newline-port port))
>>>> +      (bytevector->nix-base32-string (get-hash)))))
>> 
>> […]
>> 
>>> Also, I think returning multiple values would be more natural here
>>> (i.e., replace “cons” with “values”).
>> 
>> I tried building it that way to begin with, but I’m having issues
>> making it work (nicely, or maybe at all). I think it comes down to
>> later functions optionally failing with a single #f-value. Judging
>> by the lack of infrastructure, I imagine functions that return different
>> numbers of values are not idiomatic scheme. Should this be changed to
>> return two values (#f #f) on failure? Or to raise an exception and
>> handle it higher up when we want to ignore a failure?
>> 
>> Currently, implementing this with values/let-values results in me
>> doing more or less a combination of let-values and match, at which
>> point it seems that any potential benefits of using multiple values
>> as opposed to a pair/list are lost. (There’s no match-values form is
>> there?)
> 
> I’m not sure I understand the problem.  Here’s what I had in mind:
> 
> diff --git a/guix/import/hackage.scm b/guix/import/hackage.scm
> index 2853037797..9ba3c4b58e 100644
> --- a/guix/import/hackage.scm
> +++ b/guix/import/hackage.scm
> @@ -120,8 +120,8 @@ version is returned."
> (define (read-cabal-and-hash port)
>   "Read a cabal file and its base32 hash from a port."
>   (let-values (((port get-hash) (open-sha256-input-port port)))
> -    (cons (read-cabal (canonical-newline-port port))
> -          (bytevector->nix-base32-string (get-hash)))))
> +    (values (read-cabal (canonical-newline-port port))
> +            (bytevector->nix-base32-string (get-hash)))))
> 
> (define (hackage-fetch-and-hash name-version)
>   "Return the Cabal file and hash for the package NAME-VERSION, or #f on
> @@ -129,7 +129,7 @@ failure. If the version part is omitted from the package name, then return
> the latest version."
>   (guard (c ((and (http-get-error? c)
>                   (= 404 (http-get-error-code c)))
> -             #f))                       ;"expected" if package is unknown
> +             (values #f #f)))           ;"expected" if package is unknown

This is the point: I tried to keep returning a single #f to signal failure.

I sent an updated patch, let me know if I missed something. Thanks!

Robert