[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#36048] [PATCH] guix: import: hackage: handle hackage revisions
From: |
Robert Vollmert |
Subject: |
[bug#36048] [PATCH] guix: import: hackage: handle hackage revisions |
Date: |
Thu, 13 Jun 2019 21:40:15 +0200 |
> On 13. Jun 2019, at 20:09, Timothy Sample <address@hidden> wrote:
>
> Hi Robert,
>
> Robert Vollmert <address@hidden> 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 <address@hidden> 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