Re: hackage importer broken
Federico Beffa writes: > Hi, > > I notice that with a recent Guix checkout (commit > d8e85b20325073d90cfaf3060889d59d91362deb) the hackage importer doesn't > work and the problem seems to be with Guile itself or the lalr parser > coming with it: > > --- > GNU Guile 2.0.13 > Copyright (C) 1995-2016 Free Software Foundation, Inc. > > Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'. > This program is free software, and you are welcome to redistribute it > under certain conditions; type `,show c' for details. > > Enter `,help' for help. > scheme@(guile-user)> ,m (guix import hackage) > scheme@(guix import hackage)> (canonical-newline-port (http-fetch > "https://hackage.haskell.org/package/hmatrix/hmatrix.cabal"; > #:verify-certificate? #f)) > $2 = # > scheme@(guix import hackage)> (read-cabal $2) > system/base/lalr.upstream.scm:1851:2: In procedure ___push: > system/base/lalr.upstream.scm:1851:2: Wrong number of arguments to > # This error looks familiar. I think I’ve seen it when I played with Guile 2.2 and the load path included modules from both versions of Guile. -- Ricardo GPG: BCA6 89B6 3655 3801 C3C6 2150 197A 5888 235F ACAC https://elephly.net
Re: hackage importer broken
On Fri, Feb 3, 2017 at 2:53 PM, Federico Beffa wrote:
> Hi,
>
> I notice that with a recent Guix checkout (commit
> d8e85b20325073d90cfaf3060889d59d91362deb) the hackage importer doesn't
> work and the problem seems to be with Guile itself or the lalr parser
> coming with it:
>
> ---
> GNU Guile 2.0.13
> Copyright (C) 1995-2016 Free Software Foundation, Inc.
>
> Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
> This program is free software, and you are welcome to redistribute it
> under certain conditions; type `,show c' for details.
>
> Enter `,help' for help.
> scheme@(guile-user)> ,m (guix import hackage)
> scheme@(guix import hackage)> (canonical-newline-port (http-fetch
> "https://hackage.haskell.org/package/hmatrix/hmatrix.cabal";
> #:verify-certificate? #f))
> $2 = #
> scheme@(guix import hackage)> (read-cabal $2)
> system/base/lalr.upstream.scm:1851:2: In procedure ___push:
> system/base/lalr.upstream.scm:1851:2: Wrong number of arguments to
> #
>
> Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue.
> scheme@(guix import hackage) [1]> ,locals
> Local variables:
> $3 = delta = 1
> $4 = new-category = 6
> $5 = lvalue = (# #)
> While executing meta-command:
> ERROR: In procedure frame-local-ref: Argument 2 out of range: 3
> scheme@(guix import hackage) [1]>
> ---
>
> Notice that inspecting the stack of the backtrace results in an error!
>
> The importer does work as expected with Guile 2.0.11:
>
> ---
> GNU Guile 2.0.11
> Copyright (C) 1995-2014 Free Software Foundation, Inc.
>
> Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'.
> This program is free software, and you are welcome to redistribute it
> under certain conditions; type `,show c' for details.
>
> Enter `,help' for help.
> scheme@(guile-user)> ,m (guix import hackage)
> scheme@(guix import hackage)> (canonical-newline-port (http-fetch
> "https://hackage.haskell.org/package/hmatrix/hmatrix.cabal";
> #:verify-certificate? #f))
> $1 = #
> scheme@(guix import hackage)> (read-cabal $1)
> $2 = (("name" ("hmatrix")) ("version" ("0.18.0.0")) ("license"
> ("BSD3")) ("license-file" ("LICENSE")) ("author" ("Alberto Ruiz"))
> ("maintainer" ("Alberto Ruiz")) ("stability" ("provisional"))
> ("homepage" ("https://github.com/albertoruiz/hmatrix";)) ("synopsis"
> ("Numeric Linear Algebra")) ("description" ("Linear systems, matrix
> decompositions, and other numerical computations based on BLAS and
> LAPACK. . Standard interface: \"Numeric.LinearAlgebra\". . Safer
> interface with statically checked dimensions:
> \"Numeric.LinearAlgebra.Static\". . Code examples:
> ")) ("category"
> ("Math")) ("tested-with" ("GHC==8.0")) ("cabal-version" (">=1.8"))
> ("build-type" ("Simple")) ("extra-source-files" ("THANKS.md
> CHANGELOG")) ("extra-source-files" ("src/Internal/C/lapack-aux.h"))
> (section flag "openblas" (("description" ("Link with OpenBLAS
> (https://github.com/xianyi/OpenBLAS) optimized libraries."))
> ("default" ("False")) ("manual" ("True" (section library
> (("build-depends" ("base >= 4.8 && < 5, binary, array, deepseq,
> random, split, bytestring, storable-complex, vector >= 0.8"))
> ("hs-source-dirs" ("src")) ("exposed-modules" ("Numeric.LinearAlgebra
> Numeric.LinearAlgebra.Devel Numeric.LinearAlgebra.Data
> Numeric.LinearAlgebra.HMatrix Numeric.LinearAlgebra.Static"))
> ("other-modules" ("Internal.Vector Internal.Devel Internal.Vectorized
> Internal.Matrix Internal.ST Internal.IO Internal.Element
> Internal.Conversion Internal.LAPACK Internal.Numeric
> Internal.Algorithms Internal.Random Internal.Container Internal.Sparse
> Internal.Convolution Internal.Chain Numeric.Vector Internal.CG
> Numeric.Matrix Internal.Util Internal.Modular Internal.Static"))
> ("c-sources" ("src/Internal/C/lapack-aux.c
> src/Internal/C/vector-aux.c")) ("extensions"
> ("ForeignFunctionInterface")) ("ghc-options" ("-Wall
> -fno-warn-missing-signatures -fno-warn-orphans -fprof-auto"))
> ("cc-options" ("-O4 -Wall")) (if (arch "x86_64") (("cc-options"
> ("-msse2"))) ()) (if (arch "i386") (("cc-options" ("-msse2"))) ()) (if
> (os "OSX") ((if (flag "openblas") (("extra-lib-dirs"
> ("/opt/local/lib/openblas/lib")) ("extra-libraries" ("openblas")))
> (("extra-libraries" ("blas lapack" ("extra-lib-dirs"
> ("/opt/local/lib/")) ("include-dirs" ("/opt/local/include/"))
> ("extra-lib-dirs" ("/usr/local/lib/")) ("include-dirs"
> ("/usr/local/include/")) (if (arch "i386") (("cc-options" ("-arch
> i386"))) ()) ("frameworks" ("Accelerate"))) ()) (if (os "freebsd")
> ((if (flag "openblas") (("extra-lib-dirs"
> ("/usr/local/lib/openblas/lib")) ("extra-libraries" ("openblas"
Re: hackage importer problem
John J Foerch writes: > Hello, > > I'm seeing a problem with the hackage importer with the git-annex > package: > > $ guix import hackage git-annex > Syntax error: unexpected token : custom-setup (at line 316, column 0) > Syntax error: unexpected end of input > guix import: error: failed to download cabal file for package 'git-annex' > > git-annex.cabal is here: > > http://hackage.haskell.org/package/git-annex-6.20160808/git-annex.cabal > > Is it a bug in the importer? Hi, the hackage importer only support packages using 'Build-type' of type 'Simple'. The packages in question uses a 'Build-type' of type 'Custom'. See https://www.haskell.org/cabal/users-guide/developing-packages.html#package-descriptions I've not studied the variant 'Custom' of 'Build-type', so I could be wrong. But, looking at the 'git-annex' Cabal file the only offending part is the section starting with the header 'custom-setup' on line 316. Making the parser recognize this type of section should not be too difficult. If you are interested, take a look at the file 'guix/build/cabal.scm', line 149. That's where we define the tokens for the supported section types and where a new section of type 'custom-build' would have to be added (with related functions). Regards, Fede
Re: hackage importer
On Tue, Jun 9, 2015 at 9:38 AM, Ludovic Courtès wrote: > OK to commit, thank you! Pushed. > PS: Commit 751630c adds n-ary >>= for your pleasure. ;-) Thanks :-) Fede
Re: hackage importer
Federico Beffa skribis: > On Fri, Jun 5, 2015 at 9:30 AM, Ludovic Courtès wrote: [...] >>> +(define (make-stack) >>> + "Creates a simple stack closure. Actions on the generated stack are >>> +requested by calling it with one of the following symbols as the first >>> +argument: 'empty?, 'push!, 'top, 'pop! and 'clear!. The action 'push! is >>> the >>> +only one requiring a second argument corresponding to the object to be >>> added >>> +to the stack." >>> + (let ((stack '())) >>> +(lambda (msg . args) >>> + (cond ((eqv? msg 'empty?) (null? stack)) >>> +((eqv? msg 'push!) (set! stack (cons (first args) stack))) >>> +((eqv? msg 'top) (if (null? stack) '() (first stack))) >>> +((eqv? msg 'pop!) (match stack >>> +((e r ...) (set! stack (cdr stack)) e) >>> +(_ #f))) >>> +((eqv? msg 'clear!) (set! stack '())) >>> +(else #f) >> >> Fair enough. :-) I wonder what happens exactly when trying to return >> monadic values in the parser. > > Given that the parser repeatedly calls the tunk generated by > 'make-lexer' without passing any state or knowing anything about to > which monad it may belong to, I thought that it would not work. But, > as you see, I'm new to Scheme, new to monads, and new to Lisp in > general. I think the rules can return any kind of value, so there shouldn’t be a problem with returning monadic values (of course it won’t bind them for you, but that’s not a problem.) Anyway, an exercise for later. ;-) >>> +;; Stack to track the structure of nested blocks >>> +(define context-stack (make-stack)) >> >> What about making it either a SRFI-39 parameter, or a parameter to >> ‘make-cabal-parser’? > > I made it a parameter. Thanks for suggesting it! It made me realize > what they are really used for :-) > Do you think it is correct to say that they serve the purpose of > special variables in Lisp? (I'm looking a little bit into Common Lisp > as well.) Not sure what you mean by “special variables” (and I’m not familiar with CL), but the concept is fairly common: It’s dynamic scoping, which is the default in elisp, sometimes called “fluids”, sometimes “parameters.” > From 8a28ed0f3c3077ce12d4924c59e317c52a68a77e Mon Sep 17 00:00:00 2001 > From: Federico Beffa > Date: Sun, 26 Apr 2015 11:22:29 +0200 > Subject: [PATCH] import: hackage: Refactor parsing code and add new options. > > * guix/import/cabal.scm: New file. > * guix/import/hackage.scm: Update to use the new Cabal parsing module. > * tests/hackage.scm: Update tests. > * guix/scripts/import/hackage.scm: Add new '--cabal-environment' and '--stdin' > options. > * doc/guix.texi: ... and document them. > * Makefile.am (MODULES): Add 'guix/import/cabal.scm', > 'guix/import/hackage.scm' and 'guix/scripts/import/hackage.scm'. > (SCM_TESTS): Add 'tests/hackage.scm'. OK to commit, thank you! (I had not realized the hackage.scm files were missing from the Makefile until now.) Thanks, Ludo’. PS: Commit 751630c adds n-ary >>= for your pleasure. ;-)
Re: hackage importer
On Fri, Jun 5, 2015 at 9:30 AM, Ludovic Courtès wrote:
>> I've removed the test with TABs because the Cabal documentation says
>> explicitly that they are not allowed.
>> https://www.haskell.org/cabal/users-guide/developing-packages.html#package-descriptions
>
> But are they actually used in practice?
When I prepared the very first version of the importer I did find one
case among all the ones I tried. I believe that now that package has
been update to a new version and doesn't include TABs anymore.
> [...]
>
>> +(define (make-stack)
>> + "Creates a simple stack closure. Actions on the generated stack are
>> +requested by calling it with one of the following symbols as the first
>> +argument: 'empty?, 'push!, 'top, 'pop! and 'clear!. The action 'push! is
>> the
>> +only one requiring a second argument corresponding to the object to be added
>> +to the stack."
>> + (let ((stack '()))
>> +(lambda (msg . args)
>> + (cond ((eqv? msg 'empty?) (null? stack))
>> +((eqv? msg 'push!) (set! stack (cons (first args) stack)))
>> +((eqv? msg 'top) (if (null? stack) '() (first stack)))
>> +((eqv? msg 'pop!) (match stack
>> +((e r ...) (set! stack (cdr stack)) e)
>> +(_ #f)))
>> +((eqv? msg 'clear!) (set! stack '()))
>> +(else #f)
>
> Fair enough. :-) I wonder what happens exactly when trying to return
> monadic values in the parser.
Given that the parser repeatedly calls the tunk generated by
'make-lexer' without passing any state or knowing anything about to
which monad it may belong to, I thought that it would not work. But,
as you see, I'm new to Scheme, new to monads, and new to Lisp in
general.
>
>> +;; Stack to track the structure of nested blocks
>> +(define context-stack (make-stack))
>
> What about making it either a SRFI-39 parameter, or a parameter to
> ‘make-cabal-parser’?
I made it a parameter. Thanks for suggesting it! It made me realize
what they are really used for :-)
Do you think it is correct to say that they serve the purpose of
special variables in Lisp? (I'm looking a little bit into Common Lisp
as well.)
>> +(define* (hackage->guix-package package-name #:key
>> +(include-test-dependencies? #t)
>> +(read-from-stdin? #f)
>> +(cabal-environment '()))
>
> Instead of #:read-from-stdin?, what about adding a #:port argument?
> That way, it would either read from PORT, or fetch from Hackage. That
> seems more idiomatic and more flexible.
Absolutely! Changed.
>
> Also please mention it in the docstring.
Done.
>
>> -(test-assert "hackage->guix-package test 3"
>> - (eval-test-with-cabal test-cabal-3))
>> -
>> -(test-assert "conditional->sexp-like"
>> - (match
>> -(eval-cabal-keywords
>> - (conditional->sexp-like test-cond-1)
>> - '(("debug" . "False")))
>> -(('and ('or ('string-match "darwin" ('%current-system)) ('not '#f)) '#t)
>> - #t)
>> -(x
>> - (pk 'fail x #f
>
> I’m a bit scared when we add new code *and* remove tests. ;-)
>
> Could you add a couple of representative tests? I’m sure you ran tests
> by hand at the REPL, so it should be a matter of adding them in the file.
The reason for deleting the test is that that particular function
doesn't exist anymore. The functionality that it did provide is now
integrated in the parser. So, I've added one new test which exercises
'read-cabal' with a bunch of nested conditionals.
Thanks for the review!
Fede
From 8a28ed0f3c3077ce12d4924c59e317c52a68a77e Mon Sep 17 00:00:00 2001
From: Federico Beffa
Date: Sun, 26 Apr 2015 11:22:29 +0200
Subject: [PATCH] import: hackage: Refactor parsing code and add new options.
* guix/import/cabal.scm: New file.
* guix/import/hackage.scm: Update to use the new Cabal parsing module.
* tests/hackage.scm: Update tests.
* guix/scripts/import/hackage.scm: Add new '--cabal-environment' and '--stdin'
options.
* doc/guix.texi: ... and document them.
* Makefile.am (MODULES): Add 'guix/import/cabal.scm',
'guix/import/hackage.scm' and 'guix/scripts/import/hackage.scm'.
(SCM_TESTS): Add 'tests/hackage.scm'.
---
Makefile.am | 4 +
doc/guix.texi | 22 +-
guix/import/cabal.scm | 815
guix/import/hackage.scm | 703 --
guix/scripts/import/hackage.scm | 66 +++-
tests/hackage.scm | 88 +++--
6 files changed, 1017 insertions(+), 681 deletions(-)
create mode 100644 guix/import/cabal.scm
diff --git a/Makefile.am b/Makefile.am
index d54e281..b42a7f5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -89,6 +89,8 @@ MODULES = \
guix/import/utils.scm\
guix/import/gnu.scm\
guix/import/snix.scm\
+ guix/import/cabal.scm\
+ guix/import/hackage.scm \
guix/scripts/download.scm \
guix
Re: hackage importer
Howdy! Federico Beffa skribis: > On Sat, May 2, 2015 at 2:48 PM, Ludovic Courtès wrote: [...] >> This procedure is intimidating. I think this is partly due to its >> length, to the big let-values, the long identifiers, the many local >> variables, nested binds, etc. > > Ok, this procedure has now ... disappeared ... or rather it is now > hidden in a huge, but invisible macro ;-) > I've added support for braces delimited blocks. In so doing the > complexity of an ad-hoc solution increased further and decided that it > was time to study (and use) a proper parser. Yay, good idea! > But, a couple of words on your remarks: > > - Thanks to your comment about long list of local variables I > (re-)discovered the (test => expr) form of cond clauses. Very useful! > - The nested use of the >>= function didn't look nice and the reason > is that it is really meant as a way to sequence monadic functions as > in (>>= m f1 f2 ...). Unfortunately the current version of >>= in > guile only accepts 2 arguments (1 function), hence the nesting. It > would be nice to correct that :-) Sure, I have it in my to-do list. :-) I looked at it quickly and it seems less trivial than initially envisioned though. >>> +(define-record-type >>> + (make-cabal-package name version license home-page source-repository >>> + synopsis description >>> + executables lib test-suites >>> + flags eval-environment) >>> + cabal-package? >>> + (name cabal-package-name) >>> + (version cabal-package-version) >>> + (license cabal-package-license) >>> + (home-page cabal-package-home-page) >>> + (source-repository cabal-package-source-repository) >>> + (synopsis cabal-package-synopsis) >>> + (description cabal-package-description) >>> + (executables cabal-package-executables) >>> + (lib cabal-package-library) ; 'library' is a Scheme keyword >> >> There are no keyboards in Scheme. :-) > > ?? Oops, these should have read “keywords”, not “keyboards.” :-) >> The existing tests here are fine, but they are more like integration >> tests (they test the whole pipeline.) Maybe it would be nice to >> directly exercise ‘read-cabal’ and ‘eval-cabal’ individually? > > It is true that the tests are for the whole pipeline, but they catch > most of the problems (problems in any function along the chain) with > the smallest number of tests :-). I'm not very keen in doing fine > grained testing. Sorry. > > I've removed the test with TABs because the Cabal documentation says > explicitly that they are not allowed. > https://www.haskell.org/cabal/users-guide/developing-packages.html#package-descriptions But are they actually used in practice? > I've changed the second test to check the use of braces (multi-line > values have still to be indented). OK. > From f422ea9aff3aa8425c80eaadf50628c24d54495a Mon Sep 17 00:00:00 2001 > From: Federico Beffa > Date: Sun, 26 Apr 2015 11:22:29 +0200 > Subject: [PATCH] import: hackage: Refactor parsing code and add new options. > > * guix/import/cabal.scm: New file. > * guix/import/hackage.scm: Update to use the new Cabal parsing module. > * tests/hackage.scm: Update tests. > * guix/scripts/import/hackage.scm: Add new '--cabal-environment' and '--stdin' > options. > * doc/guix.texi: ... and document them. > * Makefile.am (MODULES): Add 'guix/import/cabal.scm', > 'guix/import/hackage.scm' and 'guix/scripts/import/hackage.scm'. > (SCM_TESTS): Add 'tests/hackage.scm'. [...] > +(define (make-stack) > + "Creates a simple stack closure. Actions on the generated stack are > +requested by calling it with one of the following symbols as the first > +argument: 'empty?, 'push!, 'top, 'pop! and 'clear!. The action 'push! is the > +only one requiring a second argument corresponding to the object to be added > +to the stack." > + (let ((stack '())) > +(lambda (msg . args) > + (cond ((eqv? msg 'empty?) (null? stack)) > +((eqv? msg 'push!) (set! stack (cons (first args) stack))) > +((eqv? msg 'top) (if (null? stack) '() (first stack))) > +((eqv? msg 'pop!) (match stack > +((e r ...) (set! stack (cdr stack)) e) > +(_ #f))) > +((eqv? msg 'clear!) (set! stack '())) > +(else #f) Fair enough. :-) I wonder what happens exactly when trying to return monadic values in the parser. > +;; Stack to track the structure of nested blocks > +(define context-stack (make-stack)) What about making it either a SRFI-39 parameter, or a parameter to ‘make-cabal-parser’? I’ve only read through quickly, but the rest of the file looks lean and clean! > +(define* (hackage->guix-package package-name #:key > +(include-test-dependencies? #t) > +(read-from-stdin? #f) > +(cabal-environment '())) Instead of #:read-from-stdin?, what about adding
Re: hackage importer
Hi, sorry for taking so long to answer! On Sat, May 2, 2015 at 2:48 PM, Ludovic Courtès wrote: >> Subject: [PATCH] import: hackage: Refactor parsing code and add new option. >> >> * guix/import/cabal.scm: New file. >> >> * guix/import/hackage.scm: Update to use the new Cabal parsing module. >> >> * tests/hackage.scm: Update tests for private functions. >> >> * guix/scripts/import/hackage.scm: Add new '--cabal-environment' option. >> >> * doc/guix.texi: ... and document it. >> >> * Makefile.am (MODULES): Add 'guix/import/cabal.scm', >> 'guix/import/hackage.scm' and 'guix/scripts/import/hackage.scm'. >> (SCM_TESTS): Add 'tests/hackage.scm'. > > No newlines between entries. Done. [...] > This procedure is intimidating. I think this is partly due to its > length, to the big let-values, the long identifiers, the many local > variables, nested binds, etc. Ok, this procedure has now ... disappeared ... or rather it is now hidden in a huge, but invisible macro ;-) I've added support for braces delimited blocks. In so doing the complexity of an ad-hoc solution increased further and decided that it was time to study (and use) a proper parser. But, a couple of words on your remarks: - Thanks to your comment about long list of local variables I (re-)discovered the (test => expr) form of cond clauses. Very useful! - The nested use of the >>= function didn't look nice and the reason is that it is really meant as a way to sequence monadic functions as in (>>= m f1 f2 ...). Unfortunately the current version of >>= in guile only accepts 2 arguments (1 function), hence the nesting. It would be nice to correct that :-) In any case, I had to give up with the state monad because the lalr parser in Guile doesn't play nice with the functional programming paradigm. >> +(define-record-type >> + (make-cabal-package name version license home-page source-repository >> + synopsis description >> + executables lib test-suites >> + flags eval-environment) >> + cabal-package? >> + (name cabal-package-name) >> + (version cabal-package-version) >> + (license cabal-package-license) >> + (home-page cabal-package-home-page) >> + (source-repository cabal-package-source-repository) >> + (synopsis cabal-package-synopsis) >> + (description cabal-package-description) >> + (executables cabal-package-executables) >> + (lib cabal-package-library) ; 'library' is a Scheme keyword > > There are no keyboards in Scheme. :-) ?? > [...] > >> + (define (impl haskell) >> +(let* ((haskell-implementation (or (assoc-ref env "impl") "ghc")) >> + (impl-rx-result-with-version >> +(string-match "([a-zA-Z0-9_]+)-([0-9.]+)" >> haskell-implementation)) >> + (impl-name (or (and=> impl-rx-result-with-version >> + (cut match:substring <> 1)) >> + haskell-implementation)) >> + (impl-version (and=> impl-rx-result-with-version >> +(cut match:substring <> 2))) >> + (cabal-rx-result-with-version >> +(string-match "([a-zA-Z0-9_-]+) *([<>=]+) *([0-9.]+) *" >> haskell)) >> + (cabal-rx-result-without-version >> +(string-match "([a-zA-Z0-9_-]+)" haskell)) >> + (cabal-impl-name (or (and=> cabal-rx-result-with-version >> + (cut match:substring <> 1)) >> +(match:substring >> + cabal-rx-result-without-version 1))) >> + (cabal-impl-version (and=> cabal-rx-result-with-version >> + (cut match:substring <> 3))) >> + (cabal-impl-operator (and=> cabal-rx-result-with-version >> + (cut match:substring <> 2))) >> + (comparison (and=> cabal-impl-operator >> + (cut string-append "string" <> > > Again I feel we need one or more auxiliary procedures and/or data types > here to simplify this part (fewer local variables), as well as shorter > identifiers. WDYT? I've added two help functions to make it easier to read. > The existing tests here are fine, but they are more like integration > tests (they test the whole pipeline.) Maybe it would be nice to > directly exercise ‘read-cabal’ and ‘eval-cabal’ individually? It is true that the tests are for the whole pipeline, but they catch most of the problems (problems in any function along the chain) with the smallest number of tests :-). I'm not very keen in doing fine grained testing. Sorry. I've removed the test with TABs because the Cabal documentation says explicitly that they are not allowed. https://www.haskell.org/cabal/users-guide/developing-packages.html#package-descriptions I've changed the second test to check the use of braces (multi-line values have still to be indented). Regards, Fede From f422ea9aff3aa8425c80ea
Re: hackage importer
Federico Beffa skribis: > Please find attached a patch reorganizing the code as you suggest. Woow, neat! Impressive work. I think this is a great improvement. I have a bunch of stylistic comments below, and some open questions as well. > From bc8cdab1e322a25002a3d9cf33eddd856c8a81d8 Mon Sep 17 00:00:00 2001 > From: Federico Beffa > Date: Sun, 26 Apr 2015 11:22:29 +0200 > Subject: [PATCH] import: hackage: Refactor parsing code and add new option. > > * guix/import/cabal.scm: New file. > > * guix/import/hackage.scm: Update to use the new Cabal parsing module. > > * tests/hackage.scm: Update tests for private functions. > > * guix/scripts/import/hackage.scm: Add new '--cabal-environment' option. > > * doc/guix.texi: ... and document it. > > * Makefile.am (MODULES): Add 'guix/import/cabal.scm', > 'guix/import/hackage.scm' and 'guix/scripts/import/hackage.scm'. > (SCM_TESTS): Add 'tests/hackage.scm'. No newlines between entries. [...] > +;; This record stores the state information needed during parsing of Cabal > +;; files. > +(define-record-type > + (make-cabal-parse-state lines minimum-indent indents conditionals > + true-group? true-group false-group > + true-group?-stack true-group-stack > false-group-stack) [...] > +(make-cabal-parse-state lines -1 '() '() #t '() '() '() '() > '( I’m not claiming this must done now, but it may improve readability to use ‘define-record-type*’. That way, with appropriate field default values, one could write something like: (cabal-parse-state (lines lines)) That would also allow the use of ‘inherit’, which is slightly less verbose than ‘set-fields’. WDYT? Besides, could you add comments to explain the meaning of the various fields? I’m particularly curious about ‘true-group?’ & co. ;-) > +(define (parse-cabal result) > + "Parse a Cabal file and append its content to RESULT (a list). Return the > +updated result as a monadic value in the state monad." > + (mlet* %state-monad ((state (current-state))) > +(match state > + (($ lines minimum-indent indents conditionals > + true-group? true-group false-group > + true-group?-stack true-group-stack > + false-group-stack) > + (let*-values > + (((current-indent line) > + (if (null? lines) > + (values 0 "") > + (line-indentation+rest (first lines > +((next-line-indent next-line) > + (if (or (null? lines) (null? (cdr lines))) > + (values 0 "") > + (line-indentation+rest (second lines > +((key-value-rx-result) (has-key? line)) > +((end-of-file?) (null? lines)) > +((is-simple-key-value?) (and (= next-line-indent current-indent) > + key-value-rx-result)) > +((is-multi-line-key-value?) (and (> next-line-indent > current-indent) > + key-value-rx-result)) > +((key) (and=> key-value-rx-result > + (lambda (rx-res) > +(string-downcase (match:substring rx-res 1) > +((value) (and=> key-value-rx-result (cut match:substring <> 2 > + (cond > + (end-of-file? (return (reverse result))) > + (is-simple-key-value? > + (>>= (state-add-entry (list key `(,value)) result (cdr lines)) > +parse-cabal)) > + (is-multi-line-key-value? > + (let*-values > + (((value-lst lines) > + (multi-line-value (cdr lines) > + (if (string-null? value) '() `(,value) > + (>>= (state-add-entry (list key value-lst) result lines) > + parse-cabal))) > + (else ; it's a section > + (let* ((section-header (string-tokenize (string-downcase line))) > + (section-type (string->symbol (first section-header))) > + (section-name (if (> (length section-header) 1) > + (second section-header) > + ""))) > + (mbegin %current-monad > + (set-current-state > +(set-fields state > +((cabal-parse-state-minimum-indent) > current-indent) > +((cabal-parse-state-lines) (cdr lines > + (>>= > +(>>= (parse-cabal-section '()) > + (lambda (section-contents) > + (mlet* %state-monad ((state (current-state))) > + (mbegin %current-monad > + (set-current-state > +(set-fields state > +((cabal-parse-state-mi
Re: hackage importer
Federico Beffa writes: > Please find attached a patch reorganizing the code as you suggest. Just noticed that I forgot to delete 'parse-cabal' from the public interface of the cabal module. Regards, Fede
Re: hackage importer
On Tue, Mar 31, 2015 at 3:33 PM, Ludovic Courtès wrote:
> I think it’s a matter of separating concerns. In my mind there are
> three distinct layers:
>
> 1. Cabal parsing (what I call ‘read-cabal’, because it’s the
> equivalent of ‘read’);
>
> 2. Cabal evaluation/instantiation for a certain set of flags, OS,
> etc. (what I call ‘eval-cabal’ because it’s the equivalent of
> ‘eval’);
>
> 3. Conversion of Cabal packages of Guix package sexps.
>
> My concern was about making sure these three phases were clearly visible
> in the code. Tu put it differently, #1 and #2 would conceptually be
> part of a Cabal parsing/evaluation library, while #3 would be the only
> Guix-specific part.
Please find attached a patch reorganizing the code as you suggest.
Regards,
Fede
From bc8cdab1e322a25002a3d9cf33eddd856c8a81d8 Mon Sep 17 00:00:00 2001
From: Federico Beffa
Date: Sun, 26 Apr 2015 11:22:29 +0200
Subject: [PATCH] import: hackage: Refactor parsing code and add new option.
* guix/import/cabal.scm: New file.
* guix/import/hackage.scm: Update to use the new Cabal parsing module.
* tests/hackage.scm: Update tests for private functions.
* guix/scripts/import/hackage.scm: Add new '--cabal-environment' option.
* doc/guix.texi: ... and document it.
* Makefile.am (MODULES): Add 'guix/import/cabal.scm',
'guix/import/hackage.scm' and 'guix/scripts/import/hackage.scm'.
(SCM_TESTS): Add 'tests/hackage.scm'.
---
Makefile.am | 4 +
doc/guix.texi | 17 +-
guix/import/cabal.scm | 902
guix/import/hackage.scm | 691 --
guix/scripts/import/hackage.scm | 14 +-
tests/hackage.scm | 18 +-
6 files changed, 1009 insertions(+), 637 deletions(-)
create mode 100644 guix/import/cabal.scm
diff --git a/Makefile.am b/Makefile.am
index d54e281..b42a7f5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -89,6 +89,8 @@ MODULES = \
guix/import/utils.scm\
guix/import/gnu.scm\
guix/import/snix.scm\
+ guix/import/cabal.scm\
+ guix/import/hackage.scm \
guix/scripts/download.scm \
guix/scripts/build.scm \
guix/scripts/archive.scm \
@@ -104,6 +106,7 @@ MODULES = \
guix/scripts/lint.scm\
guix/scripts/import/gnu.scm \
guix/scripts/import/nix.scm \
+ guix/scripts/import/hackage.scm \
guix/scripts/environment.scm \
guix/scripts/publish.scm \
guix.scm \
@@ -173,6 +176,7 @@ SCM_TESTS = \
tests/build-utils.scm\
tests/packages.scm\
tests/snix.scm\
+ tests/hackage.scm\
tests/store.scm\
tests/monads.scm\
tests/gexp.scm\
diff --git a/doc/guix.texi b/doc/guix.texi
index 70604b7..453e71f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -3201,14 +3201,25 @@ Specific command-line options are:
@table @code
@item --no-test-dependencies
@itemx -t
-Do not include dependencies only required to run the test suite.
+Do not include dependencies only required to run the test suites.
+@item --cabal-environment=@var{alist}
+@itemx -e @var{alist}
+@var{alist} is a Scheme alist defining the environment in which the
+Cabal conditionals are evaluated. The accepted keys are: @samp{os},
+@samp{arch}, @samp{impl} and a string representing the name of a flag.
+The value associated with a flag has to be either the symbol
+@verb{'true'} or @verb{'false'}. The value associated with other keys
+has to conform to the Cabal file format definition. The default value
+associated with the keys @samp{os}, @samp{arch} and @samp{impl} is
+@samp{linux}, @samp{x86_64} and @samp{ghc} respectively.
@end table
The command below imports meta-data for the latest version of the
-@code{HTTP} Haskell package without including test dependencies:
+@code{HTTP} Haskell package without including test dependencies and
+specifying the value of the flag @samp{network-uri} as @verb{'false'}:
@example
-guix import hackage -t HTTP
+guix import hackage -t -e "'((\"network-uri\" . false))" HTTP
@end example
A specific package version may optionally be specified by following the
diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
new file mode 100644
index 000..fd4bbd6
--- /dev/null
+++ b/guix/import/cabal.scm
@@ -0,0 +1,902 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2015 Federico Beffa
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should
Re: hackage importer
Federico Beffa skribis: > My intention wasn't to make an "universal" Cabal parser for two reasons: > (i) I've not found any full, formal description of the file format. I > could in principle deduce it from the Haskell code, but I'm just > starting to learn Haskell. > (ii) I don't see any use of Cabal files in the Scheme world, but maybe > I'm just blind :-) You’re right, of course ;-), but thinking in terms of separate libraries can help structure the code IMO. >> Anyway, I’ve probably used enough of your time by now. :-) >> If this discussion gives you ideas on how to structure the code, that is >> fine, but otherwise we can probably go with the architecture you >> propose. >> >> How does that sound? > > I think that restructuring the code as you suggest requires quite a > bit of effort. At this point in time I'm not ready to invest the > required time. If one day I will decide to work on improving the code > to make it handle block structured files, that may be the right moment > to reorganize it. Sounds good! > Please find attached updated patches with added documentation, two > more tests, and an option to disable the inclusion of dependencies > only requited by the test-suite of the package. > 'read-cabal' now takes a port and 'strip-cabal' was renamed as > suggested and made local to the former. If parsing fails now an > exception of type '&message' is raised with a meaningful message. OK. > From 633bfb5af57f707dea12ab747133182d085951ff Mon Sep 17 00:00:00 2001 > From: Federico Beffa > Date: Sat, 7 Mar 2015 17:23:14 +0100 > Subject: [PATCH 01/29] import: Add hackage importer. > > * guix/scripts/import.scm (importers): Add hackage. > * guix/scripts/import/hackage.scm: New file. > * po/guix/POTFILES.in: Add guix/scripts/import.scm. > * doc/guix.texi: Add section on 'hackage' importer. [...] > +The command below imports meta-data for latest version of the ^^^ + “the” > From efb8a09ce3aee85ef73206be2957ef6c4e3360a2 Mon Sep 17 00:00:00 2001 > From: Federico Beffa > Date: Sun, 8 Mar 2015 07:48:38 +0100 > Subject: [PATCH 02/29] import: Add hackage importer. > > * guix/import/hackage.scm: New file. > * tests/hackage.scm: New file. Perfect! OK to push these two. Thanks for your patience and for the great work! Ludo’.
Re: hackage importer
On Tue, Mar 31, 2015 at 3:33 PM, Ludovic Courtès wrote:
> Nice. TABs and odd indentation probably make good additional test cases
> to have in tests/cabal.scm.
I've added these test cases as suggested.
> I think it’s a matter of separating concerns. In my mind there are
> three distinct layers:
>
> 1. Cabal parsing (what I call ‘read-cabal’, because it’s the
> equivalent of ‘read’);
>
> 2. Cabal evaluation/instantiation for a certain set of flags, OS,
> etc. (what I call ‘eval-cabal’ because it’s the equivalent of
> ‘eval’);
>
> 3. Conversion of Cabal packages of Guix package sexps.
>
> My concern was about making sure these three phases were clearly visible
> in the code. Tu put it differently, #1 and #2 would conceptually be
> part of a Cabal parsing/evaluation library, while #3 would be the only
> Guix-specific part.
OK, now I see what you had in mind. Thanks for the explanation!
My intention wasn't to make an "universal" Cabal parser for two reasons:
(i) I've not found any full, formal description of the file format. I
could in principle deduce it from the Haskell code, but I'm just
starting to learn Haskell.
(ii) I don't see any use of Cabal files in the Scheme world, but maybe
I'm just blind :-)
For these reasons my target was to understand the minimum necessary to
produce a Guix package. In spite of this, I think, I ended up
handling most of it.
What's still missing is parsing of block structured (with braces) files.
> Anyway, I’ve probably used enough of your time by now. :-)
> If this discussion gives you ideas on how to structure the code, that is
> fine, but otherwise we can probably go with the architecture you
> propose.
>
> How does that sound?
I think that restructuring the code as you suggest requires quite a
bit of effort. At this point in time I'm not ready to invest the
required time. If one day I will decide to work on improving the code
to make it handle block structured files, that may be the right moment
to reorganize it.
Please find attached updated patches with added documentation, two
more tests, and an option to disable the inclusion of dependencies
only requited by the test-suite of the package.
'read-cabal' now takes a port and 'strip-cabal' was renamed as
suggested and made local to the former. If parsing fails now an
exception of type '&message' is raised with a meaningful message.
Regards,
Fede
From 633bfb5af57f707dea12ab747133182d085951ff Mon Sep 17 00:00:00 2001
From: Federico Beffa
Date: Sat, 7 Mar 2015 17:23:14 +0100
Subject: [PATCH 01/29] import: Add hackage importer.
* guix/scripts/import.scm (importers): Add hackage.
* guix/scripts/import/hackage.scm: New file.
* po/guix/POTFILES.in: Add guix/scripts/import.scm.
* doc/guix.texi: Add section on 'hackage' importer.
---
doc/guix.texi | 29 +++
guix/scripts/import.scm | 2 +-
guix/scripts/import/hackage.scm | 106
po/guix/POTFILES.in | 1 +
4 files changed, 137 insertions(+), 1 deletion(-)
create mode 100644 guix/scripts/import/hackage.scm
diff --git a/doc/guix.texi b/doc/guix.texi
index 549da80..8c90b2d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -3140,6 +3140,35 @@ bound to the @code{libreoffice} top-level attribute):
@example
guix import nix ~/path/to/nixpkgs libreoffice
@end example
+
+@item hackage
+@cindex hackage
+Import meta-data from Haskell community's central package archive
+@uref{https://hackage.haskell.org/, Hackage}. Information is taken from
+Cabal files and includes all the relevant information, including package
+dependencies.
+
+Specific command-line options are:
+
+@table @code
+@item --no-test-dependencies
+@itemx -t
+Do not include dependencies only required to run the test suite.
+@end table
+
+The command below imports meta-data for latest version of the
+@code{HTTP} Haskell package without including test dependencies:
+
+@example
+guix import hackage -t HTTP
+@end example
+
+A specific package version may optionally be specified by following the
+package name by a hyphen and a version number as in the following example:
+
+@example
+guix import hackage mtl-2.1.3.1
+@end example
@end table
The structure of the @command{guix import} code is modular. It would be
diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index 7e75c10..06b4c17 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -73,7 +73,7 @@ rather than \\n."
;;; Entry point.
;;;
-(define importers '("gnu" "nix" "pypi" "cpan"))
+(define importers '("gnu" "nix" "pypi" "cpan" "hackage"))
(define (resolve-importer name)
(let ((module (resolve-interface
diff --git a/guix/scripts/import/hackage.scm b/guix/scripts/import/hackage.scm
new file mode 100644
index 000..f7c18cd
--- /dev/null
+++ b/guix/scripts/import/hackage.scm
@@ -0,0 +1,106 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2015 Federico Beffa
+;;;
+;;; This file is pa
Re: hackage importer
Federico Beffa skribis:
> On Sun, Mar 29, 2015 at 3:58 PM, Ludovic Courtès wrote:
>>> On Thu, Mar 26, 2015 at 2:09 PM, Ludovic Courtès wrote:
Could you post the actual backtrace you get (?) when running the program
with LC_ALL=C?
>>>
>>> I doesn't backtrace, the function just gives the wrong result.
>>
>> Hmm, OK. Still sounds like an encoding error.
>
> After changing the character that I mentioned in the previous email it
> works correctly with LC_ALL=C.
Well, OK. We may still be doing something wrong wrt. encoding/decoding,
but I’m not sure what.
>>> Before working further on improving the interface, I want first to
>>> understand what are the root causes of the errors (especially the one
>>> causing the backtrace) and fix them.
>
> The problems turned out to be related to:
> * the use of TABs in some .cabal files. I've now updated a couple of regexp.
> * The following odd indentation which confused the parsing (this is
> the one which caused the backtrace):
>
> build-depends:
> base >= 4.3 && < 4.9
> , bytestring
> , filepath
> ...
>
> I've now improved the algorithm which can now handle this odd indentation.
Nice. TABs and odd indentation probably make good additional test cases
to have in tests/cabal.scm.
> I've now tested the importer with ca. 40 packages and (I believe) they
> are all handled without errors.
Woohoo!
>> Would it be possible for ‘read-cabal’ to instead return a
>> tree-structured sexp like:
>>
>> (if (os windows)
>> (build-depends (Win32 >= 2 && < 3))
>> (build-depends (unix >= 2.0 && < 2.8)))
>>
>> That would use a variant of ‘conditional->sexp-like’, essentially.
>>
>> (Of course the achieve that the parser must keep track of indentation
>> levels and everything, as you already implemented; I’m just commenting
>> on the interface here.)
>>
>> Then, if I could imagine:
>>
>> (eval-cabal '(("name" "foo")
>> ("version" "1.0"
>> ("executable cabal" (if (os windows) ...)))
>> => #
>>
>> This way the structure of the Cabal file would be preserved, only
>> converted to sexp form, which is easier to work with.
>>
>> Does that make sense?
>
> To be honest, I'm not sure I understand what you would like to achieve.
It’s really just about the architecture and layers of code.
> 'read-cabal' returns an object and, according to your proposal, you
> would like a function '(eval-cabal object)' returning a package. In
> the code that is exactly what '(hackage-module->sexp object)' does. Is
> it a matter of naming? (I've taken names from the python and perl
> build systems, but of course I can change them if desired.)
I think it’s a matter of separating concerns. In my mind there are
three distinct layers:
1. Cabal parsing (what I call ‘read-cabal’, because it’s the
equivalent of ‘read’);
2. Cabal evaluation/instantiation for a certain set of flags, OS,
etc. (what I call ‘eval-cabal’ because it’s the equivalent of
‘eval’);
3. Conversion of Cabal packages of Guix package sexps.
My concern was about making sure these three phases were clearly visible
in the code. Tu put it differently, #1 and #2 would conceptually be
part of a Cabal parsing/evaluation library, while #3 would be the only
Guix-specific part.
> Right now 'read-cabal' is fairly simple and easy to read and debug.
> Some complexity for the evaluation of conditionals is postponed and
> handled by the function '(dependencies-cond->sexp object)' which is
> used internally by '(hackage-module->sexp object)' to create the
> package.
>
> As far as I understand, you would like 'read-cabal' to directly
> evaluate conditionals.
No, precisely not. I’m saying ‘read-cabal’ should include an AST of
conditionals; that AST would be evaluated by ‘eval-cabal’.
Anyway, I’ve probably used enough of your time by now. :-)
If this discussion gives you ideas on how to structure the code, that is
fine, but otherwise we can probably go with the architecture you
propose.
How does that sound?
Thanks,
Ludo’.
Re: hackage importer
On Sun, Mar 29, 2015 at 3:58 PM, Ludovic Courtès wrote:
>> On Thu, Mar 26, 2015 at 2:09 PM, Ludovic Courtès wrote:
>>> Could you post the actual backtrace you get (?) when running the program
>>> with LC_ALL=C?
>>
>> I doesn't backtrace, the function just gives the wrong result.
>
> Hmm, OK. Still sounds like an encoding error.
After changing the character that I mentioned in the previous email it
works correctly with LC_ALL=C.
>> Before working further on improving the interface, I want first to
>> understand what are the root causes of the errors (especially the one
>> causing the backtrace) and fix them.
The problems turned out to be related to:
* the use of TABs in some .cabal files. I've now updated a couple of regexp.
* The following odd indentation which confused the parsing (this is
the one which caused the backtrace):
build-depends:
base >= 4.3 && < 4.9
, bytestring
, filepath
...
I've now improved the algorithm which can now handle this odd indentation.
I've now tested the importer with ca. 40 packages and (I believe) they
are all handled without errors.
> OK. I would rather have ‘read-cabal’ take an input port (like Scheme’s
> ‘read’) and return the list above; this would be the least surprising,
> more idiomatic approach. ‘strip-cabal’ (or
> ‘strip-insignificant-lines’?) would be an internal procedure used only
> by ‘read-cabal’.
That's no problem. The way it is right now makes it easier to test in
the REPL, but is in no way essential.
> Would it be possible for ‘read-cabal’ to instead return a
> tree-structured sexp like:
>
> (if (os windows)
> (build-depends (Win32 >= 2 && < 3))
> (build-depends (unix >= 2.0 && < 2.8)))
>
> That would use a variant of ‘conditional->sexp-like’, essentially.
>
> (Of course the achieve that the parser must keep track of indentation
> levels and everything, as you already implemented; I’m just commenting
> on the interface here.)
>
> Then, if I could imagine:
>
> (eval-cabal '(("name" "foo")
> ("version" "1.0"
> ("executable cabal" (if (os windows) ...)))
> => #
>
> This way the structure of the Cabal file would be preserved, only
> converted to sexp form, which is easier to work with.
>
> Does that make sense?
To be honest, I'm not sure I understand what you would like to achieve.
'read-cabal' returns an object and, according to your proposal, you
would like a function '(eval-cabal object)' returning a package. In
the code that is exactly what '(hackage-module->sexp object)' does. Is
it a matter of naming? (I've taken names from the python and perl
build systems, but of course I can change them if desired.)
To the representation of object:
Right now 'read-cabal' is fairly simple and easy to read and debug.
Some complexity for the evaluation of conditionals is postponed and
handled by the function '(dependencies-cond->sexp object)' which is
used internally by '(hackage-module->sexp object)' to create the
package.
As far as I understand, you would like 'read-cabal' to directly
evaluate conditionals. To achieve that, essentially all of the
functionality of '(dependencies-cond->sexp object)' would have to be
included in it, making 'read-cabal' a substantially more complex
function and simplifying the work of "later" functions. So, as I see
it, we would just move the complexity from one function to another
one.
In addition, with the current approach within
'(dependencies-cond->sexp object)': (i) I can easily discard
everything not related to depencendies before handling the
conditionals of interest. (ii) I have all the cabal file flags, even
if they come after the conditional in the file.
If I'm completely missing the point, could you please be more verbose
with your explanation.
Thanks for your patience!
Regards,
Fede
Re: hackage importer
Federico Beffa skribis: > On Thu, Mar 26, 2015 at 2:09 PM, Ludovic Courtès wrote: >> Could you post the actual backtrace you get (?) when running the program >> with LC_ALL=C? > > I doesn't backtrace, the function just gives the wrong result. Hmm, OK. Still sounds like an encoding error. I would expect the conversion of conditional expressions to sexps to be done in the parsing phase above, such that ‘read-cabal’ returns an object with some sort of an AST for those conditionals. Then this part would focus on the evaluation of those conditionals, like: ;; Evaluate the conditionals in CABAL according to FLAGS. Return an ;; evaluated Cabal object. (eval-cabal cabal flags) WDYT? > > I'm not sure this can be done, because flags must be declared in the > cabal file itself and the manual is not clear if the flags are > required to be declared before they are used or not (although it would > make sense). By ‘flags’ I actually meant things like ‘unix’, which appear in conditionals: (eval-cabal cabal '(unix)) > To see how this importer (and the haskell-build-system that I've > posted) performs I've now used it with several libraries (for the > moment I've packaged 26 libraries). While it runs nicely with most of > them, a couple posed problems resulting in package definitions with > some fields empty and 1 caused a backtrace. (Once I fixed the package > manually, the build-system worked fine for all of them.) > > Before working further on improving the interface, I want first to > understand what are the root causes of the errors (especially the one > causing the backtrace) and fix them. Sounds good, thanks! Ludo’.
Re: hackage importer
On Thu, Mar 26, 2015 at 2:09 PM, Ludovic Courtès wrote: > Could you post the actual backtrace you get (?) when running the program > with LC_ALL=C? I doesn't backtrace, the function just gives the wrong result. I think the problem was caused by the character § that I did introduce to mark some cabal test keywords. I've now changed character and the problem seems to be solved. > > [...] > >>> I would expect the conversion of conditional expressions to sexps to be >>> done in the parsing phase above, such that ‘read-cabal’ returns an >>> object with some sort of an AST for those conditionals. >>> >>> Then this part would focus on the evaluation of those conditionals, >>> like: >>> >>> ;; Evaluate the conditionals in CABAL according to FLAGS. Return an >>> ;; evaluated Cabal object. >>> (eval-cabal cabal flags) >>> >>> WDYT? I'm not sure this can be done, because flags must be declared in the cabal file itself and the manual is not clear if the flags are required to be declared before they are used or not (although it would make sense). https://www.haskell.org/cabal/users-guide/developing-packages.html#configurations [...] > Looks like we’re almost there. I hope the above makes sense! To see how this importer (and the haskell-build-system that I've posted) performs I've now used it with several libraries (for the moment I've packaged 26 libraries). While it runs nicely with most of them, a couple posed problems resulting in package definitions with some fields empty and 1 caused a backtrace. (Once I fixed the package manually, the build-system worked fine for all of them.) Before working further on improving the interface, I want first to understand what are the root causes of the errors (especially the one causing the backtrace) and fix them. Thanks for all your comments! I think they make sense to me and will try to accommodate them. Regards, Fede
Re: hackage importer
Federico Beffa skribis:
> I've created a test for hackage. In doing so I've had an issue with the
> locale and would like an advice: When I run "guix import hackage
> package-name" from the shell, or when I invoke a REPL with Geiser
> everything works fine. When I initially did run the test-suite with
>
> make check TESTS=tests/hackage.scm
>
> the conditionals conversion looked wrong. I've found that this is
> related to the choice of locale. To make the test work as desired I
> added '(setlocale LC_ALL "en_US.UTF-8")' to the file declaring the
> tests.
What’s the exact error you were getting? Character classes in regexps
are locale sensitive, I think, which could be one reason things behave
differently. There’s also the problem that, when running in the C
locale, ‘regexp-exec’ (and other C-implemented procedures) cannot be
passed any string that is not strictly ASCII.
Could you post the actual backtrace you get (?) when running the program
with LC_ALL=C?
[...]
>> I would expect the conversion of conditional expressions to sexps to be
>> done in the parsing phase above, such that ‘read-cabal’ returns an
>> object with some sort of an AST for those conditionals.
>>
>> Then this part would focus on the evaluation of those conditionals,
>> like:
>>
>> ;; Evaluate the conditionals in CABAL according to FLAGS. Return an
>> ;; evaluated Cabal object.
>> (eval-cabal cabal flags)
>>
>> WDYT?
>
> I think it's better to keep the parsing phase to a bare minimum and work
> with layers to keep each function as manageable and simple as possible.
>
> The way it works is as follows:
>
> 1. 'strip-cabal' reads the file, strips comment and empty lines and
> return a list. Each element of the list is a line from the Cabal file.
>
> 2. 'read-cabal' takes the above list and does the parsing required to
> read the indentation based structure of the file. It returns a list
> composed of list pairs. The pair is composed by a list of keys and a
> list of values. For example, the following snippet
>
> name: foo
> version: 1.0
> ...
>
> is returned as
>
> ((("name")("foo"))
> (("version") ("1.0"))
> ...)
OK. I would rather have ‘read-cabal’ take an input port (like Scheme’s
‘read’) and return the list above; this would be the least surprising,
more idiomatic approach. ‘strip-cabal’ (or
‘strip-insignificant-lines’?) would be an internal procedure used only
by ‘read-cabal’.
WDYT?
> This is enough for all the information that we need to build a Guix
> package, but dependencies.
>
> Dependencies are listed in 'Library' and 'Executable cabal' sections of
> the Cabal file as in the following example snippet:
>
> executable cabal
> build-depends:
> HTTP >= 4000.2.5 && < 4000.3
> ...
>
> and may include conditionals as in (continuing from above with the
> proper indentation)
>
> if os(windows)
> build-depends: Win32 >= 2 && < 3
> else
> build-depends: unix >= 2.0 && < 2.8
> ...
>
> Now, to make sense of the indentation based structure I need to keep a
> state indicating how many indentation levels we currently have and the
> name of the various sub-sections. For this reason I keep a one to one
> correspondence between indentation levels and section titles. That means
> that the above snipped is encoded as follows in my Cabal object:
>
> ((("executable cabal" "build-depends:") ("HTTP..."))
> (("executable cabal" "if os(windows)" "build-depends:") ("Win32 ..."))
> (("executable cabal" "else" "build-depends:") ("unix ..."))
> ...)
>
> If I split 'if' from the predicate 'os(windows)' then the level of
> indentation and the corresponding section header loose synchronization
> (different length).
Would it be possible for ‘read-cabal’ to instead return a
tree-structured sexp like:
(if (os windows)
(build-depends (Win32 >= 2 && < 3))
(build-depends (unix >= 2.0 && < 2.8)))
That would use a variant of ‘conditional->sexp-like’, essentially.
(Of course the achieve that the parser must keep track of indentation
levels and everything, as you already implemented; I’m just commenting
on the interface here.)
Then, if I could imagine:
(eval-cabal '(("name" "foo")
("version" "1.0"
("executable cabal" (if (os windows) ...)))
=> #
This way the structure of the Cabal file would be preserved, only
converted to sexp form, which is easier to work with.
Does that make sense?
> For this reason I only convert the predicates from Cabal syntax to
> Scheme syntax when I take the list of dependencies and convert the list
> in the form expected in a guix package.
>
> I hope to have clarified the strategy.
>
>>> +(define (guix-name name)
>>
>> Rather ‘hackage-name->package-name’?
>
> OK
>
>>> +;; Split the comma separated list of dependencies coming from the cabal
>>> file
>>> +;; and return a list with inputs suitable for the GUIX package. Currently
>>> the
>>> +;; version information is discarded.
>>
>> s/GUIX/Guix/
>
Re: hackage importer
l...@gnu.org (Ludovic Courtès) writes:
> I would use ‘ghc-’ right from the start, since that’s really what it
> is. WDYT?
Agreed!
>> Obviously the tests in part 5 were used along the way and will be
>> removed.
>
> More precisely, they’ll have to be turned into tests/hackage.scm
> (similar to tests/snix.scm and tests/pypi.scm.) :-)
I've created a test for hackage. In doing so I've had an issue with the
locale and would like an advice: When I run "guix import hackage
package-name" from the shell, or when I invoke a REPL with Geiser
everything works fine. When I initially did run the test-suite with
make check TESTS=tests/hackage.scm
the conditionals conversion looked wrong. I've found that this is
related to the choice of locale. To make the test work as desired I
added '(setlocale LC_ALL "en_US.UTF-8")' to the file declaring the
tests.
I suppose that running the 'guix import ...' command does get the locale
from the shell. Should I somehow force the locale within the '(guix
import hackage)' module?
>> +;; List of libraries distributed with ghc (7.8.4).
>> +(define ghc-standard-libraries
>> + '("haskell98"; 2.0.0.3
>> +"hoopl"; 3.10.0.1
>
> Maybe the version numbers in comments can be omitted since they’ll
> become outdated eventually?
OK.
>> +(define guix-name-prefix "haskell-")
>
> s/guix-name-prefix/package-name-prefix/ and rather “ghc-” IMO.
Agreed.
>
>> +;; Regular expression matching "key: value"
>> +(define key-value-rx
>> + "([a-zA-Z0-9-]+): *(\\w?.*)$")
>> +
>> +;; Regular expression matching a section "head sub-head ..."
>> +(define sections-rx
>> + "([a-zA-Z0-9\\(\\)-]+)")
>> +
>> +;; Cabal comment.
>> +(define comment-rx
>> + "^ *--")
>
> Use (make-regexp ...) directly, and then ‘regexp-exec’ instead of
> ‘string-match’.
I already had in mind to change these before sending the e-mail, but I
forgot. It's now done.
>
>> +;; Check if the current line includes a key
>> +(define (has-key? line)
>> + (string-match key-value-rx line))
>> +
>> +(define (comment-line? line)
>> + (string-match comment-rx line))
>> +
>> +;; returns the number of indentation spaces and the rest of the line.
>> +(define (line-indentation+rest line)
>
> Please turn all the comments above procedures this into docstrings.
Done.
>
>> +;; Part 1 main function: read a cabal fila and filter empty lines and
>> comments.
>> +;; Returns a list composed by the pre-processed lines of the file.
>> +(define (read-cabal port)
>
> s/fila/file/
> s/Returns/Return/
>
> I would expect ‘read-cabal’ to return a record, say, that can be
> directly manipulated (just like ‘read’ returns a Scheme object.) But
> here it seems to return an intermediate parsing result, right?
OK. I've renamed some functions and the new name should hopefully be a
better choice. What was 'read-cabal' has become 'strip-cabal' (which only
strips comments and empty lines).
>> +;; Parses a cabal file in the form of a list of lines as produced by
>> +;; READ-CABAL and returns its content in the following form:
>> +;;
>> +;; (((head1 sub-head1 ... key1) (value))
>> +;; ((head2 sub-head2 ... key2) (value2))
>> +;; ...).
>> +;;
>> +;; where all elements are strings.
>> +;;
>> +;; We try do deduce the format from the following document:
>> +;; https://www.haskell.org/cabal/users-guide/developing-packages.html
>> +;;
>> +;; Key values are case-insensitive. We therefore lowercase them. Values are
>> +;; case-sensitive.
>> +;;
>> +;; Currently only only layout structured files are parsed. Braces {}
>
> “only indentation-structured files”
OK
>
>> +;; structured files are not handled.
>> +(define (cabal->key-values lines)
>
> I think this is the one I would call ‘read-cabal’.
>
... and I've followed you suggestion and renamed this function to 'read-cabal'.
>> +;; Find if a string represent a conditional
>> +(define condition-rx
>> + (make-regexp "^if +(.*)$"))
>
> The comment should rather be “Regexp for conditionals.” and be placed
> below ‘define’.
OK
> I would expect the conversion of conditional expressions to sexps to be
> done in the parsing phase above, such that ‘read-cabal’ returns an
> object with some sort of an AST for those conditionals.
>
> Then this part would focus on the evaluation of those conditionals,
> like:
>
> ;; Evaluate the conditionals in CABAL according to FLAGS. Return an
> ;; evaluated Cabal object.
> (eval-cabal cabal flags)
>
> WDYT?
I think it's better to keep the parsing phase to a bare minimum and work
with layers to keep each function as manageable and simple as possible.
The way it works is as follows:
1. 'strip-cabal' reads the file, strips comment and empty lines and
return a list. Each element of the list is a line from the Cabal file.
2. 'read-cabal' takes the above list and does the parsing required to
read the indentation based structure of the file. It returns a list
composed of list pairs. The pair is composed by a list of keys and a
list of values. For example,
Re: hackage importer
Thanks for the review!
I'm currently on a business trip. Will look carefully at your comments
when I will be back.
Regards,
Fede
On Sun, Mar 15, 2015 at 2:38 PM, Ludovic Courtès wrote:
> Federico Beffa skribis:
>
>> please find attached an initial version of an importer for packages
>> from Hackage:
>> http://hackage.haskell.org/
>
> Woow, impressive piece of work!
>
> [...]
>
>> * The code handles dependencies with conditionals and tries to comply
>> with the description at
>> https://www.haskell.org/cabal/users-guide/developing-packages.html#configurations
>
> Neat.
>
>> * Cabal files include dependencies to libraries included with the
>> complier (at least with GHC). For the moment I just filter those out
>> assuming the packages are going to be compiled with GHC.
>
> Sounds good.
>
>> * The generated package name is prefixed with "haskell-" in a similar
>> way to Perl and Python packages. However, the cabal file includes
>> checks for the compiler implementation and the importer handles that
>> and keep the test in the generated package (see eval-impl in the code
>> and the description in the above link). If in the future there is
>> interest in supporting other Haskell compilers, then maybe we should
>> better prefix the packages according to the used compiler ("ghc-"
>> ...).
>
> I would use ‘ghc-’ right from the start, since that’s really what it
> is. WDYT?
>
>> Obviously the tests in part 5 were used along the way and will be
>> removed.
>
> More precisely, they’ll have to be turned into tests/hackage.scm
> (similar to tests/snix.scm and tests/pypi.scm.) :-)
>
> This looks like really nice stuff. There are patterns that are not
> sufficiently apparent IMO, like ‘read-cabal’ that would do the actual
> parsing and return a first-class cabal object, and then ‘eval-cabal’ (or
> similar) that would evaluate the conditionals in a cabal object. For
> the intermediate steps, I would expect conversion procedures from one
> representation to another, typically ‘foo->bar’.
>
> Some comments below.
>
>> +;; List of libraries distributed with ghc (7.8.4).
>> +(define ghc-standard-libraries
>> + '("haskell98"; 2.0.0.3
>> +"hoopl"; 3.10.0.1
>
> Maybe the version numbers in comments can be omitted since they’ll
> become outdated eventually?
>
>> +(define guix-name-prefix "haskell-")
>
> s/guix-name-prefix/package-name-prefix/ and rather “ghc-” IMO.
>
>> +;; Regular expression matching "key: value"
>> +(define key-value-rx
>> + "([a-zA-Z0-9-]+): *(\\w?.*)$")
>> +
>> +;; Regular expression matching a section "head sub-head ..."
>> +(define sections-rx
>> + "([a-zA-Z0-9\\(\\)-]+)")
>> +
>> +;; Cabal comment.
>> +(define comment-rx
>> + "^ *--")
>
> Use (make-regexp ...) directly, and then ‘regexp-exec’ instead of
> ‘string-match’.
>
>> +;; Check if the current line includes a key
>> +(define (has-key? line)
>> + (string-match key-value-rx line))
>> +
>> +(define (comment-line? line)
>> + (string-match comment-rx line))
>> +
>> +;; returns the number of indentation spaces and the rest of the line.
>> +(define (line-indentation+rest line)
>
> Please turn all the comments above procedures this into docstrings.
>
>> +;; Part 1 main function: read a cabal fila and filter empty lines and
>> comments.
>> +;; Returns a list composed by the pre-processed lines of the file.
>> +(define (read-cabal port)
>
> s/fila/file/
> s/Returns/Return/
>
> I would expect ‘read-cabal’ to return a record, say, that can be
> directly manipulated (just like ‘read’ returns a Scheme object.) But
> here it seems to return an intermediate parsing result, right?
>
>> +;; Parses a cabal file in the form of a list of lines as produced by
>> +;; READ-CABAL and returns its content in the following form:
>> +;;
>> +;; (((head1 sub-head1 ... key1) (value))
>> +;; ((head2 sub-head2 ... key2) (value2))
>> +;; ...).
>> +;;
>> +;; where all elements are strings.
>> +;;
>> +;; We try do deduce the format from the following document:
>> +;; https://www.haskell.org/cabal/users-guide/developing-packages.html
>> +;;
>> +;; Key values are case-insensitive. We therefore lowercase them. Values are
>> +;; case-sensitive.
>> +;;
>> +;; Currently only only layout structured files are parsed. Braces {}
>
> “only indentation-structured files”
>
>> +;; structured files are not handled.
>> +(define (cabal->key-values lines)
>
> I think this is the one I would call ‘read-cabal’.
>
>> +;; Find if a string represent a conditional
>> +(define condition-rx
>> + (make-regexp "^if +(.*)$"))
>
> The comment should rather be “Regexp for conditionals.” and be placed
> below ‘define’.
>
>> +;; Part 3:
>> +;;
>> +;; So far we did read the cabal file and extracted flags information. Now we
>> +;; need to evaluate the conditions and process the entries accordingly.
>
> I would expect the conversion of conditional expressions to sexps to be
> done in the parsing phase above, such that ‘read-cabal’ returns an
> object with some sort of an AST
Re: hackage importer
Federico Beffa skribis:
> please find attached an initial version of an importer for packages
> from Hackage:
> http://hackage.haskell.org/
Woow, impressive piece of work!
[...]
> * The code handles dependencies with conditionals and tries to comply
> with the description at
> https://www.haskell.org/cabal/users-guide/developing-packages.html#configurations
Neat.
> * Cabal files include dependencies to libraries included with the
> complier (at least with GHC). For the moment I just filter those out
> assuming the packages are going to be compiled with GHC.
Sounds good.
> * The generated package name is prefixed with "haskell-" in a similar
> way to Perl and Python packages. However, the cabal file includes
> checks for the compiler implementation and the importer handles that
> and keep the test in the generated package (see eval-impl in the code
> and the description in the above link). If in the future there is
> interest in supporting other Haskell compilers, then maybe we should
> better prefix the packages according to the used compiler ("ghc-"
> ...).
I would use ‘ghc-’ right from the start, since that’s really what it
is. WDYT?
> Obviously the tests in part 5 were used along the way and will be
> removed.
More precisely, they’ll have to be turned into tests/hackage.scm
(similar to tests/snix.scm and tests/pypi.scm.) :-)
This looks like really nice stuff. There are patterns that are not
sufficiently apparent IMO, like ‘read-cabal’ that would do the actual
parsing and return a first-class cabal object, and then ‘eval-cabal’ (or
similar) that would evaluate the conditionals in a cabal object. For
the intermediate steps, I would expect conversion procedures from one
representation to another, typically ‘foo->bar’.
Some comments below.
> +;; List of libraries distributed with ghc (7.8.4).
> +(define ghc-standard-libraries
> + '("haskell98"; 2.0.0.3
> +"hoopl"; 3.10.0.1
Maybe the version numbers in comments can be omitted since they’ll
become outdated eventually?
> +(define guix-name-prefix "haskell-")
s/guix-name-prefix/package-name-prefix/ and rather “ghc-” IMO.
> +;; Regular expression matching "key: value"
> +(define key-value-rx
> + "([a-zA-Z0-9-]+): *(\\w?.*)$")
> +
> +;; Regular expression matching a section "head sub-head ..."
> +(define sections-rx
> + "([a-zA-Z0-9\\(\\)-]+)")
> +
> +;; Cabal comment.
> +(define comment-rx
> + "^ *--")
Use (make-regexp ...) directly, and then ‘regexp-exec’ instead of
‘string-match’.
> +;; Check if the current line includes a key
> +(define (has-key? line)
> + (string-match key-value-rx line))
> +
> +(define (comment-line? line)
> + (string-match comment-rx line))
> +
> +;; returns the number of indentation spaces and the rest of the line.
> +(define (line-indentation+rest line)
Please turn all the comments above procedures this into docstrings.
> +;; Part 1 main function: read a cabal fila and filter empty lines and
> comments.
> +;; Returns a list composed by the pre-processed lines of the file.
> +(define (read-cabal port)
s/fila/file/
s/Returns/Return/
I would expect ‘read-cabal’ to return a record, say, that can be
directly manipulated (just like ‘read’ returns a Scheme object.) But
here it seems to return an intermediate parsing result, right?
> +;; Parses a cabal file in the form of a list of lines as produced by
> +;; READ-CABAL and returns its content in the following form:
> +;;
> +;; (((head1 sub-head1 ... key1) (value))
> +;; ((head2 sub-head2 ... key2) (value2))
> +;; ...).
> +;;
> +;; where all elements are strings.
> +;;
> +;; We try do deduce the format from the following document:
> +;; https://www.haskell.org/cabal/users-guide/developing-packages.html
> +;;
> +;; Key values are case-insensitive. We therefore lowercase them. Values are
> +;; case-sensitive.
> +;;
> +;; Currently only only layout structured files are parsed. Braces {}
“only indentation-structured files”
> +;; structured files are not handled.
> +(define (cabal->key-values lines)
I think this is the one I would call ‘read-cabal’.
> +;; Find if a string represent a conditional
> +(define condition-rx
> + (make-regexp "^if +(.*)$"))
The comment should rather be “Regexp for conditionals.” and be placed
below ‘define’.
> +;; Part 3:
> +;;
> +;; So far we did read the cabal file and extracted flags information. Now we
> +;; need to evaluate the conditions and process the entries accordingly.
I would expect the conversion of conditional expressions to sexps to be
done in the parsing phase above, such that ‘read-cabal’ returns an
object with some sort of an AST for those conditionals.
Then this part would focus on the evaluation of those conditionals,
like:
;; Evaluate the conditionals in CABAL according to FLAGS. Return an
;; evaluated Cabal object.
(eval-cabal cabal flags)
WDYT?
> +(define (guix-name name)
Rather ‘hackage-name->package-name’?
> +;; Split the comma separated list of dependencies coming
