From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: hackage importer Date: Sun, 15 Mar 2015 15:38:15 +0100 Message-ID: <87k2yiqqaw.fsf@gnu.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:42923) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YX9gT-00032I-Aw for guix-devel@gnu.org; Sun, 15 Mar 2015 10:38:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YX9gP-0003Oz-65 for guix-devel@gnu.org; Sun, 15 Mar 2015 10:38:29 -0400 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:52529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YX9gP-0003Ov-2e for guix-devel@gnu.org; Sun, 15 Mar 2015 10:38:25 -0400 In-Reply-To: (Federico Beffa's message of "Fri, 13 Mar 2015 18:59:24 +0100") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Federico Beffa Cc: Guix-devel 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#config= urations 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 =E2=80=98ghc-=E2=80=99 right from the start, since that=E2=80= =99s really what it is. WDYT? > Obviously the tests in part 5 were used along the way and will be > removed. More precisely, they=E2=80=99ll 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 =E2=80=98read-cabal=E2=80=99 that would do = the actual parsing and return a first-class cabal object, and then =E2=80=98eval-cabal= =E2=80=99 (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 =E2=80=98foo->bar=E2=80=99. 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=E2=80=99ll become outdated eventually? > +(define guix-name-prefix "haskell-") s/guix-name-prefix/package-name-prefix/ and rather =E2=80=9Cghc-=E2=80=9D I= MO. > +;; 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 =E2=80=98regexp-exec=E2=80=99 inst= ead of =E2=80=98string-match=E2=80=99. > +;; 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 co= mments. > +;; 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 =E2=80=98read-cabal=E2=80=99 to return a record, say= , that can be directly manipulated (just like =E2=80=98read=E2=80=99 returns a Scheme obj= ect.) 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 {} =E2=80=9Conly indentation-structured files=E2=80=9D > +;; structured files are not handled. > +(define (cabal->key-values lines) I think this is the one I would call =E2=80=98read-cabal=E2=80=99. > +;; Find if a string represent a conditional > +(define condition-rx > + (make-regexp "^if +(.*)$")) The comment should rather be =E2=80=9CRegexp for conditionals.=E2=80=9D and= be placed below =E2=80=98define=E2=80=99. > +;; Part 3: > +;; > +;; So far we did read the cabal file and extracted flags information. No= w 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 =E2=80=98read-cabal=E2=80=99 ret= urns 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 =E2=80=98hackage-name->package-name=E2=80=99? > +;; Split the comma separated list of dependencies coming from the cabal = file > +;; and return a list with inputs suitable for the GUIX package. Current= ly 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 gen= erated =E2=80=9CGenerate=E2=80=9D > +;; S-expressions may include conditionals as defined in the cabal file. > +;; During this process we discard the version information of the package= s. > +(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 =E2=80=98ghc-standard-libraries= =E2=80=99 is already lowercase.) > +;; Part 5: > +;; > +;; Some variables used to test the varisous functions. =E2=80=9Cvarious=E2=80=9D > +(define test-dep-3 > + '((("executable cabal" "if flag(cips)" "build-depends") > + ("fbe >=3D 0.2")) > + (("executable cabal" "else" "build-depends") > + ("fbeee >=3D 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-Opti= ons") > +;; (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=E2=80=99.