Federico Beffa <be...@ieee.org> 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 <cabal> 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 from the cabal file > +;; and return a list with inputs suitable for the GUIX package. Currently > the > +;; version information is discarded. s/GUIX/Guix/ > +(define (split-dependencies ls) > + (define (split-at-comma d) > + (map > + (lambda (m) > + (let ((name (guix-name (match:substring m 1)))) > + (list name (list 'unquote (string->symbol name))))) > + (list-matches dependencies-rx d))) I think it could use simply: (map string-trim-both (string-tokenize d (char-set-complement (char-set #\,)))) > +;; Genrate an S-expression containing the list of dependencies. The > generated “Generate” > +;; S-expressions may include conditionals as defined in the cabal file. > +;; During this process we discard the version information of the packages. > +(define (dependencies-cond->sexp meta) [...] > + (match (match:substring rx-result 1) > + ((? (cut member <> > + ;; GUIX names are all lower-case. > + (map (cut string-downcase <>) > + ghc-standard-libraries))) s/GUIX/Guix/ I find it hard to read. Typically, I would introduce: (define (standard-library? name) (member name ghc-standard-libraries)) and use it here (with the assumption that ‘ghc-standard-libraries’ is already lowercase.) > +;; Part 5: > +;; > +;; Some variables used to test the varisous functions. “various” > +(define test-dep-3 > + '((("executable cabal" "if flag(cips)" "build-depends") > + ("fbe >= 0.2")) > + (("executable cabal" "else" "build-depends") > + ("fbeee >= 0.3")) > + )) No hanging parens please. > +;; Run some tests > + > +;; (display (cabal->key-values > +;; (call-with-input-file "mtl.cabal" read-cabal))) > +;; (display (cabal->key-values > +;; (call-with-input-file "/home/beffa/tmp/cabal-install.cabal" > read-cabal))) > +;; (display (get-flags (pre-process-entries-keys (cabal->key-values > test-5)))) > +;; (newline) > +;; (display (conditional->sexp-like test-cond-2)) > +;; (newline) > +;; (display > +;; (eval-flags (conditional->sexp-like test-cond-6) > +;; (get-flags (pre-process-entries-keys (cabal->key-values > test-6))))) > +;; (newline) > +;; (key->values (cabal->key-values test-1) "name") > +;; (newline) > +;; (key-start-end->entries (cabal->key-values test-4) "Library" "CC-Options") > +;; (newline) > +;; (eval-tests (conditional->sexp-like test-cond-6)) This should definitely go to tests/hackage.scm. > + (display (_ "Usage: guix import hackage PACKAGE-NAME > +Import and convert the Hackage package for PACKAGE-NAME. If PACKAGE-NAME > +includes a suffix constituted by a dash followed by a numerical version (as > +used with GUIX packages), then a definition for the specified version of the s/GUIX/Guix/ Also, guix/scripts/import.scm must be added to po/guix/POTFILES.in, for i18n. Thanks! Ludo’.
