Message ID | 87lfy59x2x.fsf@ngyro.com |
---|---|
State | Accepted |
Headers | show |
Series | [bug#36048] guix: import: hackage: handle hackage revisions | expand |
Context | Check | Description |
---|---|---|
cbaines/applying patch | fail | Apply failed |
> On 13. Jun 2019, at 20:09, Timothy Sample <samplet@ngyro.com> wrote: > > Hi Robert, > > Robert Vollmert <rob@vllmrt.net> 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 <samplet@ngyro.com> 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
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 (let-values (((name version) (package-name->name+version name-version))) (let* ((url (hackage-cabal-url name version)) (port (http-fetch url)) @@ -141,9 +141,8 @@ the latest version." "Return the Cabal file for the package NAME-VERSION, or #f on failure. If the version part is omitted from the package name, then return the latest version." - (match (hackage-fetch-and-hash name-version) - ((cabal . hash) cabal) - (_ #f))) + (let-values (((cabal hash) (hackage-fetch-and-hash name-version))) + cabal)) (define string->license ;; List of valid values from @@ -318,16 +317,14 @@ symbol 'true' or 'false'. The value associated with other keys has to conform to the Cabal file format definition. The default value associated with the keys \"os\", \"arch\" and \"impl\" is \"linux\", \"x86_64\" and \"ghc\" respectively." - (match - (if port (read-cabal-and-hash port) - (hackage-fetch-and-hash package-name)) - ((cabal-meta . cabal-hash) - (and=> cabal-meta (compose (cut hackage-module->sexp <> - cabal-hash - #:include-test-dependencies? - include-test-dependencies?) - (cut eval-cabal <> cabal-environment)))) - (_ #f))) + (let-values (((cabal-meta cabal-hash) + (if port + (read-cabal-and-hash (canonical-newline-port port)) + (hackage-fetch-and-hash package-name)))) + (and=> cabal-meta (compose (cut hackage-module->sexp <> cabal-hash + #:include-test-dependencies? + include-test-dependencies?) + (cut eval-cabal <> cabal-environment))))) (define hackage->guix-package/m ;memoized variant (memoize hackage->guix-package))