bug#21829: guix import hackage failures

bug#21829: guix import hackage failures

[Paul van der Walt]

Hello bug-guix,

I have found that the command `guix import hackage foo` sometimes fails.
I will try to provide a few examples.

-------------8<----------------------------------------------------------
$ guix import hackage xmonad-contrib

Starting download of /tmp/guix-file.Bt94ZV
From http://hackage.haskell.org/package/xmonad-contrib/xmonad-contrib.cabal...
 xmonad-contrib.cabal                         355KiB/s 00:00 | 13KiB transferred
Syntax error: unexpected token : true (at line 75, column 7)
Syntax error: unexpected end of input
guix import: error: failed to download cabal file for package '
FORMAT: error with call: (format #<output: file /dev/pts/1> "~:[~*~;guix ~a: ~]~afailed to download cabal file for package '~a<==='~%" import import error:  ===>)
        missing argument(s)
Backtrace:
In ice-9/boot-9.scm:
 157: 15 [catch #t #<catch-closure d75420> ...]
In unknown file:
   ?: 14 [apply-smob/1 #<catch-closure d75420>]
In ice-9/boot-9.scm:
  63: 13 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 432: 12 [eval # #]
In ice-9/boot-9.scm:
2401: 11 [save-module-excursion #<procedure e60940 at ice-9/boot-9.scm:4045:3 ()>]
4050: 10 [#<procedure e60940 at ice-9/boot-9.scm:4045:3 ()>]
1724: 9 [%start-stack load-stack #<procedure e29960 at ice-9/boot-9.scm:4041:10 ()>]
1729: 8 [#<procedure e6f270 ()>]
In unknown file:
   ?: 7 [primitive-load "/gnu/store/1pvqvwcck48izizrl17hygbb9r7bzi44-guix-0.8.3.abbe2c6/bin/.guix-real"]
In guix/ui.scm:
1173: 6 [run-guix-command import "hackage" "xmonad-contrib"]
In guix/scripts/import.scm:
 110: 5 [guix-import "hackage" "xmonad-contrib"]
In guix/scripts/import/hackage.scm:
 137: 4 [guix-import-hackage "xmonad-contrib"]
In ice-9/format.scm:
1593: 3 [format #<output: file /dev/pts/1> ...]
 766: 2 [format:format-work "~:[~*~;guix ~a: ~]~afailed to download cabal file for package '~a'~%" ...]
 200: 1 [tilde-dispatch]
In unknown file:
   ?: 0 [scm-error misc-error #f "~A" ("error in format") #f]

ERROR: In procedure scm-error:
ERROR: error in format
-------------8<----------------------------------------------------------

Of course, the errors will all be relatively similar since they seem to
be parsing errors.  Packages which fail include:

* base-compat
* base-orphans
* clock
* fast-logger
* fgl
* generic-deriving
* happy
* hscolour
* nats
* ObjectName
* old-locale
* QuickCheck
* SDL
* setenv
* split
* StateVar
* streaming-commons
* syb
* tagged
* transformers-base
* wai
* xmonad
* xmonad-contrib
* xmonad-extras
* zip-archive
* zlib

Cheers,
p.

bug#21829: guix import hackage failures

[Ludovic Courtès]

Paul van der Walt <paul@denknerd.org> skribis:

>  137: 4 [guix-import-hackage "xmonad-contrib"]
> In ice-9/format.scm:
> 1593: 3 [format #<output: file /dev/pts/1> ...]
>  766: 2 [format:format-work "~:[~*~;guix ~a: ~]~afailed to download cabal file for package '~a'~%" ...]
>  200: 1 [tilde-dispatch]
> In unknown file:
>    ?: 0 [scm-error misc-error #f "~A" ("error in format") #f]

Commit 5453de3 fixes the easy part.  :-)

Strangely, ‘define-diagnostic’ in (guix ui) was defined so that we can
get ‘format’ warnings, but the trick doesn’t work outside of (guix ui)
itself.

Thanks,
Ludo’.

bug#21829: guix import hackage failures

[Federico Beffa]

Hi,

I have checked the errors and have found the following:

* I do not get backtraces, but the following error:
------------------------------
$ ./pre-inst-env guix import hackage xmonad-contrib

Starting download of /tmp/guix-file.yf7Cor
From http://hackage.haskell.org/package/xmonad-contrib/xmonad-contrib.cabal...
 xmonad-contrib.cabal                         761KiB/s 00:00 | 13KiB transferred
Syntax error: unexpected token : true (at line 75, column 7)
Syntax error: unexpected end of input
guix import: error: failed to download cabal file for package 'xmonad-contrib'
------------------------------
  not sure what's different.

* The following packages fail because the file has DOS line endings:

  happy, base-compat, base-orphans, fast-logger, generic-deriving, ObjectName,
  SDL, setenv, split, StateVar, syb, transformers-base, wai, xmonad (+ 1 more
  problem), zlib (+ 1 more problem).

 Changing the encoding to UNIX line endings fixes the problem. This is
the number 1 problem. Is there a Guile way to easily fix this?

* nats gets imported correctly here?!

* The rest are genuine parsing errors. A brief summary of them and how
to fix them follows:
- xmonad-contrib, xmonad, xmonad-extras::
  + "if true" not handled.
  + "impl (ghc == 6.10.1) && arch (x86_64)" --> "impl(ghc == 6.10.1)
&& arch(x86_64)" OK
- clock, hscolour, QuickCheck::
  + "default   : Ture" --> "default: True" OK
  + "impl (ghc < 7.6)" --> "impl(ghc < 7.6)"
- fgl:: "description: {
         An ....
         }" ->
  No braces OK. Braces not handled outside of groups.
- old-locale:: "Cabal-Version:>=1.10" --> "Cabal-Version: >=1.10" OK
- streaming-common:: same group indentation level changes from 4 to 2 spaces!
  This file is buggy!
- tagged:: "if impl(ghc>=7.2 && <7.5)". Probably not handled correctly by parser
- zlib:: no final "\n" on last line

I will look into fixing them, but not immediately because of other
priorities. If somebody else is interested in diving into this, please
let me know, so that we don't duplicate efforts.

Regards,
Fede

bug#21829: guix import hackage failures

[Ludovic Courtès]

Federico Beffa <beffa@ieee.org> skribis:

> * I do not get backtraces, but the following error:

The backtrace thing was fixed in 5453de3d.

> * The following packages fail because the file has DOS line endings:
>
>   happy, base-compat, base-orphans, fast-logger, generic-deriving, ObjectName,
>   SDL, setenv, split, StateVar, syb, transformers-base, wai, xmonad (+ 1 more
>   problem), zlib (+ 1 more problem).
>
>  Changing the encoding to UNIX line endings fixes the problem. This is
> the number 1 problem. Is there a Guile way to easily fix this?

Could you explain how if fails exactly?

Not really “easily” unfortunately.  It depends on the APIs you use.

There’s the R6RS API from (rnrs io ports), which is supposed to
magically do line ending conversion but doesn’t seem to work:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (make-transcoder (utf-8-codec) (eol-style crlf))
$13 = #<r6rs:record:transcoder>
scheme@(guile-user)> (transcoded-port (open-string-input-port "foo\n\rbar\n\r") $13)
$14 = #<input: r6rs-transcoded-port 60725b0>
scheme@(guile-user)> (get-line $14)
$15 = "foo"
scheme@(guile-user)> (get-line $14)
$16 = "\rbar"
scheme@(guile-user)> (get-line $14)
$17 = "\r"
--8<---------------cut here---------------end--------------->8---

But then there are things like ‘string-tokenize’ that do the right
thing:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (string-tokenize "foo\n\rbar\n\rbaz")
$18 = ("foo" "bar" "baz")
--8<---------------cut here---------------end--------------->8---

Thanks,
Ludo’.

bug#21829: guix import hackage failures

[Federico Beffa]

On Wed, Nov 11, 2015 at 12:18 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Federico Beffa <beffa@ieee.org> skribis:
>
>> * I do not get backtraces, but the following error:
>
> The backtrace thing was fixed in 5453de3d.
>
>> * The following packages fail because the file has DOS line endings:
>>
>>   happy, base-compat, base-orphans, fast-logger, generic-deriving, ObjectName,
>>   SDL, setenv, split, StateVar, syb, transformers-base, wai, xmonad (+ 1 more
>>   problem), zlib (+ 1 more problem).
>>
>>  Changing the encoding to UNIX line endings fixes the problem. This is
>> the number 1 problem. Is there a Guile way to easily fix this?
>
> Could you explain how if fails exactly?

The extra character '\r' screws up the parsing because it was not
accounted for in the logic to recognize multi-line values and
indentation based block separation.

What do you think of a kind of piped filter as follows:

(define (call-with-input-file-eol-crlf->lf proc port)
  (let* ((port-pair (pipe))
         (input-port (match port-pair ((in . out) in)))
         (output-port (match port-pair ((in . out) out))))
    (letpar ((transcoder
              (let loop ((line (get-line port)))
                (unless (eof-object? line)
                  (write-line (string-trim-right line #\return) output-port)
                  (loop (get-line port)))
                (flush-output-port output-port)))
             (result (proc input-port)))
            (close-output-port output-port)
            (close-input-port input-port)
            result)))

Then instead of calling (read-cabal port) I would call
(call-with-input-file-eol-crlf->lf read-cabal port).

Regards,
Fede

bug#21829: guix import hackage failures

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

Federico Beffa <beffa@ieee.org> skribis:

> On Wed, Nov 11, 2015 at 12:18 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Federico Beffa <beffa@ieee.org> skribis:
>>
>>> * I do not get backtraces, but the following error:
>>
>> The backtrace thing was fixed in 5453de3d.
>>
>>> * The following packages fail because the file has DOS line endings:
>>>
>>>   happy, base-compat, base-orphans, fast-logger, generic-deriving, ObjectName,
>>>   SDL, setenv, split, StateVar, syb, transformers-base, wai, xmonad (+ 1 more
>>>   problem), zlib (+ 1 more problem).
>>>
>>>  Changing the encoding to UNIX line endings fixes the problem. This is
>>> the number 1 problem. Is there a Guile way to easily fix this?
>>
>> Could you explain how if fails exactly?
>
> The extra character '\r' screws up the parsing because it was not
> accounted for in the logic to recognize multi-line values and
> indentation based block separation.
>
> What do you think of a kind of piped filter as follows:
>
> (define (call-with-input-file-eol-crlf->lf proc port)
>   (let* ((port-pair (pipe))
>          (input-port (match port-pair ((in . out) in)))
>          (output-port (match port-pair ((in . out) out))))
>     (letpar ((transcoder
>               (let loop ((line (get-line port)))
>                 (unless (eof-object? line)
>                   (write-line (string-trim-right line #\return) output-port)
>                   (loop (get-line port)))
>                 (flush-output-port output-port)))
>              (result (proc input-port)))
>             (close-output-port output-port)
>             (close-input-port input-port)
>             result)))
>
> Then instead of calling (read-cabal port) I would call
> (call-with-input-file-eol-crlf->lf read-cabal port).

I wonder if it wouldn’t be easier to change the lexer to recognize line
feeds are white space or newlines, maybe along these lines:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 692 bytes --]

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 45d644a..0dd329c 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -259,10 +259,11 @@ otherwise return BOL (beginning-of-line)."
              (bol bol))
     (cond
      ((and (not (eof-object? c))
-           (or (char=? c #\space) (char=? c #\tab)))
+           (or (char-set-contains? char-set:whitespace c)))
       (read-char port)
       (loop (peek-char port) bol))
-     ((and (not (eof-object? c)) (char=? c #\newline))
+     ((and (not (eof-object? c))
+           (or (char=? c #\newline) (char=? c #\return)))
       (read-char port)
       (loop (peek-char port) #t))
      ((comment-line port c)

[-- Attachment #3: Type: text/plain, Size: 21 bytes --]


WDYT?

Ludo’.

bug#21829: guix import hackage failures

[Federico Beffa]

On Thu, Nov 12, 2015 at 10:07 AM, Ludovic Courtès <ludo@gnu.org> wrote:
> Federico Beffa <beffa@ieee.org> skribis:
> I wonder if it wouldn’t be easier to change the lexer to recognize line
> feeds are white space or newlines, maybe along these lines:

What you suggest is not enough. You have to tweak a couple of other
places as well.

I don't like having to mess around in token recognition code to
account for different eol styles. With my proposal I was trying to
abstract this away and make the eol style problem orthogonal to
parsing: first we convert to a "normal form" and then we operate on
it.  Should we find some files in 'mac' eol-style (in Emacs parlance)
then it would be trivial to adapt.

Could you be more explicit about what you do not like about this?

Regards,
Fede

bug#21829: guix import hackage failures

[Ludovic Courtès]

Federico Beffa <beffa@ieee.org> skribis:

> On Thu, Nov 12, 2015 at 10:07 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Federico Beffa <beffa@ieee.org> skribis:
>> I wonder if it wouldn’t be easier to change the lexer to recognize line
>> feeds are white space or newlines, maybe along these lines:
>
> What you suggest is not enough. You have to tweak a couple of other
> places as well.
>
> I don't like having to mess around in token recognition code to
> account for different eol styles. With my proposal I was trying to
> abstract this away and make the eol style problem orthogonal to
> parsing: first we convert to a "normal form" and then we operate on
> it.  Should we find some files in 'mac' eol-style (in Emacs parlance)
> then it would be trivial to adapt.
>
> Could you be more explicit about what you do not like about this?

I think the approach you suggest is fine, but I noticed that the places
I changed were too specific anyway (looking for #\space and #\tab
instead of the general char-set:whitespace class, for instance.)  So I
thought that it generalizing this would also fix the problem, that would
be nice, because the change is more general than just fixing the CRLF
issue.

But maybe I’m missing other places, which would make the change too
intrusive.  That’s why I was asking for your feedback.

Does that make sense?

If we go for the CRLF conversion port, we should avoid the pipe and
extra thread.  Instead, I would suggest something like:

  (define (canonical-newline-port port)
    "Return an input port that wraps PORT such that all newlines consist
  of a single carriage return."
    (make-custom-binary-input-port …))

WDYT?

Thanks,
Ludo’.

bug#21829: guix import hackage failures

[Federico Beffa]

On Thu, Nov 12, 2015 at 9:21 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> If we go for the CRLF conversion port, we should avoid the pipe and
> extra thread.  Instead, I would suggest something like:
>
>   (define (canonical-newline-port port)
>     "Return an input port that wraps PORT such that all newlines consist
>   of a single carriage return."
>     (make-custom-binary-input-port …))

I like this suggestion :-)

I never used custom ports. Is something like this OK? (Seems to work
in the REPL.)

---------------------------------------------------------------
(define (canonical-newline-port port)
  "Return an input port that wraps PORT such that all newlines consist
  of a single carriage return."
  (define (get-position)
    (if (port-has-port-position? port) (port-position port) #f))
  (define (set-position! position)
    (if (port-has-set-port-position!? port)
        (set-port-position! position port)
        #f))
  (define (close) (close-port port))
  (define (read! bv start n)
    (let loop ((count 0)
               (byte (get-u8 port)))
      (cond ((or (eof-object? byte) (= count n)) count)
            ((eqv? byte (char->integer #\return)) (loop count (get-u8 port)))
            (else
             (bytevector-u8-set! bv (+ start count) byte)
             (loop (+ count 1) (get-u8 port))))))
  (make-custom-binary-input-port "canonical-newline-port"
                                 read!
                                 get-position
                                 set-position!
                                 close))
---------------------------------------------------------------

IMO this is general enough that it could go into "guix/utils.scm". Are
you OK with this?

Regards,
Fede

bug#21829: guix import hackage failures

[Ludovic Courtès]

Federico Beffa <beffa@ieee.org> skribis:

> ---------------------------------------------------------------
> (define (canonical-newline-port port)
>   "Return an input port that wraps PORT such that all newlines consist
>   of a single carriage return."
>   (define (get-position)
>     (if (port-has-port-position? port) (port-position port) #f))
>   (define (set-position! position)
>     (if (port-has-set-port-position!? port)
>         (set-port-position! position port)
>         #f))
>   (define (close) (close-port port))
>   (define (read! bv start n)
>     (let loop ((count 0)
>                (byte (get-u8 port)))
>       (cond ((or (eof-object? byte) (= count n)) count)

BYTE is lost here in the case it is not EOF.

It may be best to move the (= count n) case right before the recursive
call below.

>             ((eqv? byte (char->integer #\return)) (loop count (get-u8 port)))

In practice this discards LF even if it’s not following CR; that’s
probably a good enough approximation, but an XXX comment would be
welcome.

>             (else
>              (bytevector-u8-set! bv (+ start count) byte)
>              (loop (+ count 1) (get-u8 port))))))
>   (make-custom-binary-input-port "canonical-newline-port"
>                                  read!
>                                  get-position
>                                  set-position!
>                                  close))
> ---------------------------------------------------------------
>
> IMO this is general enough that it could go into "guix/utils.scm". Are
> you OK with this?

Looks good!  Could you make a patch that does that, along with adding a
test or two in tests/utils.scm?

Thank you!

Ludo’.

bug#21829: guix import hackage failures

[Federico Beffa]

[-- Attachment #1: Type: text/plain, Size: 3107 bytes --]

On Fri, Nov 13, 2015 at 10:19 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Federico Beffa <beffa@ieee.org> skribis:
>
>> ---------------------------------------------------------------
>> (define (canonical-newline-port port)
>>   "Return an input port that wraps PORT such that all newlines consist
>>   of a single carriage return."
>>   (define (get-position)
>>     (if (port-has-port-position? port) (port-position port) #f))
>>   (define (set-position! position)
>>     (if (port-has-set-port-position!? port)
>>         (set-port-position! position port)
>>         #f))
>>   (define (close) (close-port port))
>>   (define (read! bv start n)
>>     (let loop ((count 0)
>>                (byte (get-u8 port)))
>>       (cond ((or (eof-object? byte) (= count n)) count)
>
> BYTE is lost here in the case it is not EOF.

Ooops. Thanks for catching it!

> It may be best to move the (= count n) case right before the recursive
> call below.
>
>>             ((eqv? byte (char->integer #\return)) (loop count (get-u8 port)))
>
> In practice this discards LF even if it’s not following CR; that’s
> probably a good enough approximation, but an XXX comment would be
> welcome.

This is intentional because, in my ignorance, I only know of uses of
'\r' before or after '\n'. Do you know of any other use in text files?

I've added an "XXX" comment, but I'm not sure what's its use.

>
>>             (else
>>              (bytevector-u8-set! bv (+ start count) byte)
>>              (loop (+ count 1) (get-u8 port))))))
>>   (make-custom-binary-input-port "canonical-newline-port"
>>                                  read!
>>                                  get-position
>>                                  set-position!
>>                                  close))
>> ---------------------------------------------------------------
>>
>> IMO this is general enough that it could go into "guix/utils.scm". Are
>> you OK with this?
>
> Looks good!  Could you make a patch that does that, along with adding a
> test or two in tests/utils.scm?

The attached patches fix the parsing of all but two of the failures
reported by Paul.
Two cabal files are still not imported correctly because they are buggy:

* streaming-commons: indentation changes from 4 to 2. But this is
explicitly forbidden. From [1]: "Field names may be indented, but all
field values in the same section must use the same indentation."

* fgl: uses braces to delimit the value of a field. As far as I
understand this is not allowed by [1]: "To continue a field value,
indent the next line relative to the field name." and "Flags,
conditionals, library and executable sections use layout to indicate
structure. ... As an alternative to using layout you can also use
explicit braces {}. ". Thus I understand that braces may be used to
delimit sections, not field values.

Obviously the official 'cabal' program is more permissive than the
description in the documentation.

Regards,
Fede

[1] https://www.haskell.org/cabal/users-guide/developing-packages.html

[-- Attachment #2: 0001-import-hackage-Add-recognition-of-true-and-false-sym.patch --]
[-- Type: text/x-diff, Size: 3028 bytes --]

From d13f06383d07e0ad4096ff7eb715264463738b0c Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 11 Nov 2015 10:39:38 +0100
Subject: [PATCH 1/6] import: hackage: Add recognition of 'true' and 'false'
 symbols.

* guix/import/cabal.scm (is-true, is-false, lex-true, lex-false): New procedures.
  (lex-word): Use them.
  (make-cabal-parser): Add TRUE and FALSE tokens.
  (eval): Add entries for 'true and 'false symbols.
---
 guix/import/cabal.scm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 45d644a..8d84e09 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -138,7 +138,7 @@ to the stack."
   "Generate a parser for Cabal files."
   (lalr-parser
    ;; --- token definitions
-   (CCURLY VCCURLY OPAREN CPAREN TEST ID VERSION RELATION
+   (CCURLY VCCURLY OPAREN CPAREN TEST ID VERSION RELATION TRUE FALSE
            (right: IF FLAG EXEC TEST-SUITE SOURCE-REPO BENCHMARK LIB OCURLY)
            (left: OR)
            (left: PROPERTY AND)
@@ -206,6 +206,8 @@ to the stack."
    (if-then     (IF tests OCURLY exprs CCURLY) : `(if ,$2 ,$4 ())
                 (IF tests open exprs close)    : `(if ,$2 ,$4 ()))
    (tests       (TEST OPAREN ID CPAREN)        : `(,$1 ,$3)
+                (TRUE)                         : 'true
+                (FALSE)                        : 'false
                 (TEST OPAREN ID RELATION VERSION CPAREN)
                 : `(,$1 ,(string-append $3 " " $4 " " $5))
                 (TEST OPAREN ID RELATION VERSION AND RELATION VERSION CPAREN)
@@ -350,6 +352,10 @@ matching a string against the created regexp."
 
 (define (is-if s) (string-ci=? s "if"))
 
+(define (is-true s) (string-ci=? s "true"))
+
+(define (is-false s) (string-ci=? s "false"))
+
 (define (is-and s) (string=? s "&&"))
 
 (define (is-or s) (string=? s "||"))
@@ -424,6 +430,10 @@ string with the read characters."
 
 (define (lex-if loc) (make-lexical-token 'IF loc #f))
 
+(define (lex-true loc) (make-lexical-token 'TRUE loc #t))
+
+(define (lex-false loc) (make-lexical-token 'FALSE loc #f))
+
 (define (lex-and loc) (make-lexical-token 'AND loc #f))
 
 (define (lex-or loc) (make-lexical-token 'OR loc #f))
@@ -489,6 +499,8 @@ LOC is the current port location."
   (let* ((w (read-delimited " ()\t\n" port 'peek)))
     (cond ((is-if w) (lex-if loc))
           ((is-test w port) (lex-test w loc))
+          ((is-true w) (lex-true loc))
+          ((is-false w) (lex-false loc))
           ((is-and w) (lex-and loc))
           ((is-or w) (lex-or loc))
           ((is-id w) (lex-id w loc))
@@ -714,6 +726,8 @@ the ordering operation and the version."
       (('os name) (os name))
       (('arch name) (arch name))
       (('impl name) (impl name))
+      ('true #t)
+      ('false #f)
       (('not name) (not (eval name)))
       ;; 'and' and 'or' aren't functions, thus we can't use apply
       (('and args ...) (fold (lambda (e s) (and e s)) #t (eval args)))
-- 
2.4.3


[-- Attachment #3: 0002-import-hackage-Imporve-parsing-of-tests.patch --]
[-- Type: text/x-diff, Size: 1941 bytes --]

From 445f1b6197c0e266027ac033c52629d990137171 Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 11 Nov 2015 11:22:42 +0100
Subject: [PATCH 2/6] import: hackage: Imporve parsing of tests.

* guix/import/cabal.scm (lex-word): Add support for tests with no spaces.
  (impl): Fix handling of operator "==".
---
 guix/import/cabal.scm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 8d84e09..615eca2 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -496,7 +496,7 @@ location."
 (define (lex-word port loc)
   "Process tokens which can be recognized by reading the next word form PORT.
 LOC is the current port location."
-  (let* ((w (read-delimited " ()\t\n" port 'peek)))
+  (let* ((w (read-delimited " <>=()\t\n" port 'peek)))
     (cond ((is-if w) (lex-if loc))
           ((is-test w port) (lex-test w loc))
           ((is-true w) (lex-true loc))
@@ -688,7 +688,11 @@ the ordering operation and the version."
                             (cut match:substring <> 2)))
            (version (and=> (with-ver-matcher-fn spec)
                            (cut match:substring <> 3))))
-      (values name operator version)))
+      (values name
+              (if (and (string? operator) (string= operator "=="))
+                  "="
+                  operator)
+              version)))
   
   (define (impl haskell)
     (let*-values (((comp-name comp-ver)
@@ -697,7 +701,7 @@ the ordering operation and the version."
                    (comp-spec-name+op+version haskell)))
       (if (and spec-ver comp-ver)
           (eval-string
-           (string-append "(string" spec-op " \"" comp-name "\""
+           (string-append "(string" spec-op " \"" comp-name "-" comp-ver "\""
                           " \"" spec-name "-" spec-ver "\")"))
           (string-match spec-name comp-name))))
   
-- 
2.4.3


[-- Attachment #4: 0003-import-hackage-Make-it-resilient-to-missing-final-ne.patch --]
[-- Type: text/x-diff, Size: 2583 bytes --]

From f796d814821289a98e401a3e3df13334a2e8689b Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 11 Nov 2015 15:31:46 +0100
Subject: [PATCH 3/6] import: hackage: Make it resilient to missing final
 newline.

* guix/import/cabal.scm (peek-next-line-indent): Check for missing final
  newline.
---
 guix/import/cabal.scm | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 615eca2..ca88ff5 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -226,19 +226,24 @@ to the stack."
   "This function can be called when the next character on PORT is #\newline
 and returns the indentation of the line starting after the #\newline
 character.  Discard (and consume) empty and comment lines."
-  (let ((initial-newline (string (read-char port))))
-    (let loop ((char (peek-char port))
-               (word ""))
-      (cond ((eqv? char #\newline) (read-char port)
-             (loop (peek-char port) ""))
-            ((or (eqv? char #\space) (eqv? char #\tab))
-             (let ((c (read-char port)))
-               (loop (peek-char port) (string-append word (string c)))))
-            ((comment-line port char) (loop (peek-char port) ""))
-            (else
-             (let ((len (string-length word)))
-               (unread-string (string-append initial-newline word) port)
-               len))))))
+  (if (eof-object? (peek-char port))
+      ;; If the file is missing the #\newline on the last line, add it and act
+      ;; as if it were there. This is needed for propoer operation of
+      ;; indentation based block recognition.
+      (begin (unread-char #\newline port) (read-char port) 0)
+      (let ((initial-newline (string (read-char port))))
+        (let loop ((char (peek-char port))
+                   (word ""))
+          (cond ((eqv? char #\newline) (read-char port)
+                 (loop (peek-char port) ""))
+                ((or (eqv? char #\space) (eqv? char #\tab))
+                 (let ((c (read-char port)))
+                   (loop (peek-char port) (string-append word (string c)))))
+                ((comment-line port char) (loop (peek-char port) ""))
+                (else
+                 (let ((len (string-length word)))
+                   (unread-string (string-append initial-newline word) port)
+                   len)))))))
 
 (define* (read-value port value min-indent #:optional (separator " "))
   "The next character on PORT must be #\newline.  Append to VALUE the
-- 
2.4.3


[-- Attachment #5: 0004-import-hackage-Make-parsing-of-tests-and-fields-more.patch --]
[-- Type: text/x-diff, Size: 2526 bytes --]

From 225164d2355afd6f9455251d87cbd34b08f68cdb Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 11 Nov 2015 16:20:45 +0100
Subject: [PATCH 4/6] import: hackage: Make parsing of tests and fields more
 flexible.

* guix/import/cabal.scm (is-test): Allow spaces between keyword and
  parentheses.
  (is-id): Add argument 'port'.  Allow spaces between keyword and column.
  (lex-word): Adjust call to 'is-id'.
---
 guix/import/cabal.scm | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index ca88ff5..257afa5 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -332,7 +332,7 @@ matching a string against the created regexp."
                 (make-regexp pat))))
     (cut regexp-exec rx <>)))
 
-(define is-property (make-rx-matcher "([a-z0-9-]+):[ \t]*(\\w?.*)$"
+(define is-property (make-rx-matcher "([a-z0-9-]+)[ \t]*:[ \t]*(\\w?.*)$"
                                      regexp/icase))
 
 (define is-flag (make-rx-matcher "^flag +([a-z0-9_-]+)"
@@ -365,17 +365,24 @@ matching a string against the created regexp."
 
 (define (is-or s) (string=? s "||"))
 
-(define (is-id s)
+(define (is-id s port)
   (let ((cabal-reserved-words
          '("if" "else" "library" "flag" "executable" "test-suite"
-           "source-repository" "benchmark")))
+           "source-repository" "benchmark"))
+        (spaces (read-while (cut char-set-contains? char-set:blank <>) port))
+        (c (peek-char port)))
+    (unread-string spaces port)
     (and (every (cut string-ci<> s <>) cabal-reserved-words)
-         (not (char=? (last (string->list s)) #\:)))))
+         (and (not (char=? (last (string->list s)) #\:))
+              (not (char=? #\: c))))))
 
 (define (is-test s port)
   (let ((tests-rx (make-regexp "os|arch|flag|impl"))
+        (spaces (read-while (cut char-set-contains? char-set:blank <>) port))
         (c (peek-char port)))
-    (and (regexp-exec tests-rx s) (char=? #\( c))))
+    (if (and (regexp-exec tests-rx s) (char=? #\( c))
+        #t
+        (begin (unread-string spaces port) #f))))
 
 ;; Lexers for individual tokens.
 
@@ -508,7 +515,7 @@ LOC is the current port location."
           ((is-false w) (lex-false loc))
           ((is-and w) (lex-and loc))
           ((is-or w) (lex-or loc))
-          ((is-id w) (lex-id w loc))
+          ((is-id w port) (lex-id w loc))
           (else (unread-string w port) #f))))
 
 (define (lex-line port loc)
-- 
2.4.3


[-- Attachment #6: 0005-utils-Add-canonical-newline-port.patch --]
[-- Type: text/x-diff, Size: 3155 bytes --]

From 1b26410f4a7a920382750bffbf5381394acafdbc Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Sat, 14 Nov 2015 15:00:36 +0100
Subject: [PATCH 5/6] utils: Add 'canonical-newline-port'.

* guix/utils.scm (canonical-newline-port): New procedure.
* tests/utils.scm ("canonical-newline-port"): New test.
---
 guix/utils.scm  | 34 ++++++++++++++++++++++++++++++++--
 tests/utils.scm |  6 ++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/guix/utils.scm b/guix/utils.scm
index 1542e86..7b589e6 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -29,7 +29,8 @@
   #:use-module (srfi srfi-39)
   #:use-module (srfi srfi-60)
   #:use-module (rnrs bytevectors)
-  #:use-module ((rnrs io ports) #:select (put-bytevector))
+  #:use-module (rnrs io ports)
+  #:use-module ((rnrs bytevectors) #:select (bytevector-u8-set!))
   #:use-module ((guix build utils)
                 #:select (dump-port package-name->name+version))
   #:use-module ((guix build syscalls) #:select (errno mkdtemp!))
@@ -90,7 +91,8 @@
             decompressed-port
             call-with-decompressed-port
             compressed-output-port
-            call-with-compressed-output-port))
+            call-with-compressed-output-port
+            canonical-newline-port))
 
 \f
 ;;;
@@ -746,6 +748,34 @@ elements after E."
             (if success?
                 (loop (absolute target) (+ depth 1))
                 file))))))
+
+(define (canonical-newline-port port)
+  "Return an input port that wraps PORT such that all newlines consist
+  of a single carriage return."
+  (define (get-position)
+    (if (port-has-port-position? port) (port-position port) #f))
+  (define (set-position! position)
+    (if (port-has-set-port-position!? port)
+        (set-port-position! position port)
+        #f))
+  (define (close) (close-port port))
+  (define (read! bv start n)
+    (let loop ((count 0)
+               (byte (get-u8 port)))
+      (cond ((eof-object? byte) count)
+            ((= count (- n 1))
+             (bytevector-u8-set! bv (+ start count) byte)
+             n)
+            ;; XXX: consume all LFs even if not followed by CR.
+            ((eqv? byte (char->integer #\return)) (loop count (get-u8 port)))
+            (else
+             (bytevector-u8-set! bv (+ start count) byte)
+             (loop (+ count 1) (get-u8 port))))))
+  (make-custom-binary-input-port "canonical-newline-port"
+                                 read!
+                                 get-position
+                                 set-position!
+                                 close))
 \f
 ;;;
 ;;; Source location.
diff --git a/tests/utils.scm b/tests/utils.scm
index b65d6d2..9d345e0 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -318,6 +318,12 @@
    (string-append (%store-prefix)
                   "/qvs2rj2ia5vci3wsdb7qvydrmacig4pg-bash-4.2-p24")))
 
+(test-equal "canonical-newline-port"
+  "This is a journey"
+  (let ((port (open-string-input-port
+               "This is a journey\r\n")))
+    (get-line (canonical-newline-port port))))
+
 (test-end)
 
 (false-if-exception (delete-file temp-file))
-- 
2.4.3


[-- Attachment #7: 0006-import-hackage-Handle-CRLF-end-of-line-style.patch --]
[-- Type: text/x-diff, Size: 1856 bytes --]

From c57be8cae9b3642beff1462acd32a0aee54ad7c6 Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Sat, 14 Nov 2015 15:15:00 +0100
Subject: [PATCH 6/6] import: hackage: Handle CRLF end of line style.

* guix/import/hackage.scm (hackage-fetch, hackage->guix-package): Do it.
---
 guix/import/hackage.scm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/guix/import/hackage.scm b/guix/import/hackage.scm
index 3baa514..8725ffa 100644
--- a/guix/import/hackage.scm
+++ b/guix/import/hackage.scm
@@ -22,7 +22,8 @@
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-1)
   #:use-module ((guix download) #:select (download-to-store))
-  #:use-module ((guix utils) #:select (package-name->name+version))
+  #:use-module ((guix utils) #:select (package-name->name+version
+                                       canonical-newline-port))
   #:use-module (guix import utils)
   #:use-module (guix import cabal)
   #:use-module (guix store)
@@ -84,7 +85,8 @@ version."
     (call-with-temporary-output-file
      (lambda (temp port)
        (and (url-fetch url temp)
-            (call-with-input-file temp read-cabal))))))
+            (call-with-input-file temp
+              (compose read-cabal canonical-newline-port)))))))
 
 (define string->license
   ;; List of valid values from
@@ -216,7 +218,7 @@ to the Cabal file format definition.  The default value associated with the
 keys \"os\", \"arch\" and \"impl\" is \"linux\", \"x86_64\" and \"ghc\"
 respectively."
   (let ((cabal-meta (if port
-                        (read-cabal port)
+                        (read-cabal (canonical-newline-port port))
                         (hackage-fetch package-name))))
     (and=> cabal-meta (compose (cut hackage-module->sexp <>
                                     #:include-test-dependencies? 
-- 
2.4.3

bug#21829: guix import hackage failures

[Ludovic Courtès]

Federico Beffa <beffa@ieee.org> skribis:

> On Fri, Nov 13, 2015 at 10:19 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Federico Beffa <beffa@ieee.org> skribis:

[...]

>> In practice this discards LF even if it’s not following CR; that’s
>> probably a good enough approximation, but an XXX comment would be
>> welcome.
>
> This is intentional because, in my ignorance, I only know of uses of
> '\r' before or after '\n'. Do you know of any other use in text files?

ISTR that some OSes (MacOS 9 and earlier?!  who cares?! :-)) use(d) a
single LF instead of a single CR.

Again that’s fine in practice I guess, but I always think it’s good to
add a note when we make an approximation so we can notice later, just in
case.

> The attached patches fix the parsing of all but two of the failures
> reported by Paul.
> Two cabal files are still not imported correctly because they are buggy:
>
> * streaming-commons: indentation changes from 4 to 2. But this is
> explicitly forbidden. From [1]: "Field names may be indented, but all
> field values in the same section must use the same indentation."
>
> * fgl: uses braces to delimit the value of a field. As far as I
> understand this is not allowed by [1]: "To continue a field value,
> indent the next line relative to the field name." and "Flags,
> conditionals, library and executable sections use layout to indicate
> structure. ... As an alternative to using layout you can also use
> explicit braces {}. ". Thus I understand that braces may be used to
> delimit sections, not field values.

Fair enough!

> Obviously the official 'cabal' program is more permissive than the
> description in the documentation.

We’re more royalist than the king!  ;-)

> From d13f06383d07e0ad4096ff7eb715264463738b0c Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 10:39:38 +0100
> Subject: [PATCH 1/6] import: hackage: Add recognition of 'true' and 'false'
>  symbols.
>
> * guix/import/cabal.scm (is-true, is-false, lex-true, lex-false): New procedures.
>   (lex-word): Use them.
>   (make-cabal-parser): Add TRUE and FALSE tokens.
>   (eval): Add entries for 'true and 'false symbols.

LGTM.

> From 445f1b6197c0e266027ac033c52629d990137171 Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 11:22:42 +0100
> Subject: [PATCH 2/6] import: hackage: Imporve parsing of tests.
>
> * guix/import/cabal.scm (lex-word): Add support for tests with no spaces.
>   (impl): Fix handling of operator "==".

LGTM, but I think it’d be great to add a test that illustrates the case
that this fixes (and to make sure it doesn’t come back later.)

> From f796d814821289a98e401a3e3df13334a2e8689b Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 15:31:46 +0100
> Subject: [PATCH 3/6] import: hackage: Make it resilient to missing final
>  newline.
>
> * guix/import/cabal.scm (peek-next-line-indent): Check for missing final
>   newline.

[...]

> +  (if (eof-object? (peek-char port))
> +      ;; If the file is missing the #\newline on the last line, add it and act
> +      ;; as if it were there. This is needed for propoer operation of
                                                      ^^^^
Typo.

> +      ;; indentation based block recognition.
> +      (begin (unread-char #\newline port) (read-char port) 0)

Isn’t this equivalent to: 0 ?

Could you add a test for this one?

> From 225164d2355afd6f9455251d87cbd34b08f68cdb Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 16:20:45 +0100
> Subject: [PATCH 4/6] import: hackage: Make parsing of tests and fields more
>  flexible.
>
> * guix/import/cabal.scm (is-test): Allow spaces between keyword and
>   parentheses.
>   (is-id): Add argument 'port'.  Allow spaces between keyword and column.
>   (lex-word): Adjust call to 'is-id'.

LGTM, and would be perfect with a test.  ;-)

> From 1b26410f4a7a920382750bffbf5381394acafdbc Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Sat, 14 Nov 2015 15:00:36 +0100
> Subject: [PATCH 5/6] utils: Add 'canonical-newline-port'.
>
> * guix/utils.scm (canonical-newline-port): New procedure.
> * tests/utils.scm ("canonical-newline-port"): New test.

[...]

> +(test-equal "canonical-newline-port"
> +  "This is a journey"
> +  (let ((port (open-string-input-port
> +               "This is a journey\r\n")))
> +    (get-line (canonical-newline-port port))))

I would rather use ‘get-string-all’ and make sure the result is exactly:

  "This is a journey\n"

(Because ‘get-line’ could have been doing its own thing regardless of
the EOL style.)

A test with several lines, including lines with just \n would be nice.

> From c57be8cae9b3642beff1462acd32a0aee54ad7c6 Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Sat, 14 Nov 2015 15:15:00 +0100
> Subject: [PATCH 6/6] import: hackage: Handle CRLF end of line style.
>
> * guix/import/hackage.scm (hackage-fetch, hackage->guix-package): Do it.

Rather “Use ‘canonical-newline-port’.” instead of “Do it.”

Thanks for all the work!

Ludo’.

bug#21829: guix import hackage failures

[Federico Beffa]

[-- Attachment #1: Type: text/plain, Size: 3321 bytes --]

On Sun, Nov 15, 2015 at 9:59 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Federico Beffa <beffa@ieee.org> skribis:
>> * guix/import/cabal.scm (lex-word): Add support for tests with no spaces.
>>   (impl): Fix handling of operator "==".
>
> LGTM, but I think it’d be great to add a test that illustrates the case
> that this fixes (and to make sure it doesn’t come back later.)

I've rewritten 'impl' and the new test that I've added covers this and more.

>> From f796d814821289a98e401a3e3df13334a2e8689b Mon Sep 17 00:00:00 2001
>> From: Federico Beffa <beffa@fbengineering.ch>
>> Date: Wed, 11 Nov 2015 15:31:46 +0100
>> Subject: [PATCH 3/6] import: hackage: Make it resilient to missing final
>>  newline.
>>
>> * guix/import/cabal.scm (peek-next-line-indent): Check for missing final
>>   newline.
>
> [...]
>
>> +  (if (eof-object? (peek-char port))
>> +      ;; If the file is missing the #\newline on the last line, add it and act
>> +      ;; as if it were there. This is needed for propoer operation of
>                                                       ^^^^
> Typo.
>
>> +      ;; indentation based block recognition.
>> +      (begin (unread-char #\newline port) (read-char port) 0)
>
> Isn’t this equivalent to: 0 ?

No. This is because at the start of a new line we check if and how
many indentation blocks have ended. If the last line doesn't terminate
this check is no done.

>
> Could you add a test for this one?

I've removed the final newline from the test 'test-read-cabal-1".

>
>> From 225164d2355afd6f9455251d87cbd34b08f68cdb Mon Sep 17 00:00:00 2001
>> From: Federico Beffa <beffa@fbengineering.ch>
>> Date: Wed, 11 Nov 2015 16:20:45 +0100
>> Subject: [PATCH 4/6] import: hackage: Make parsing of tests and fields more
>>  flexible.
>>
>> * guix/import/cabal.scm (is-test): Allow spaces between keyword and
>>   parentheses.
>>   (is-id): Add argument 'port'.  Allow spaces between keyword and column.
>>   (lex-word): Adjust call to 'is-id'.
>
> LGTM, and would be perfect with a test.  ;-)

These are now exercised in "test-read-cabal-1".

> [...]
>
>> +(test-equal "canonical-newline-port"
>> +  "This is a journey"
>> +  (let ((port (open-string-input-port
>> +               "This is a journey\r\n")))
>> +    (get-line (canonical-newline-port port))))
>
> I would rather use ‘get-string-all’ and make sure the result is exactly:
>
>   "This is a journey\n"
>
> (Because ‘get-line’ could have been doing its own thing regardless of
> the EOL style.)
>
> A test with several lines, including lines with just \n would be nice.

OK. I've updated it and the test.

>
>> From c57be8cae9b3642beff1462acd32a0aee54ad7c6 Mon Sep 17 00:00:00 2001
>> From: Federico Beffa <beffa@fbengineering.ch>
>> Date: Sat, 14 Nov 2015 15:15:00 +0100
>> Subject: [PATCH 6/6] import: hackage: Handle CRLF end of line style.
>>
>> * guix/import/hackage.scm (hackage-fetch, hackage->guix-package): Do it.
>
> Rather “Use ‘canonical-newline-port’.” instead of “Do it.”

OK.

I've made 1 more change. The importer now peeks at the 'ghc' package
version and uses that as default implementation. Before, without using
the '-e' option, it was assuming "ghc", but no specific version.

Regards,
Fede

[-- Attachment #2: 0001-import-hackage-Add-recognition-of-true-and-false-sym.patch --]
[-- Type: text/x-diff, Size: 3028 bytes --]

From d13f06383d07e0ad4096ff7eb715264463738b0c Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 11 Nov 2015 10:39:38 +0100
Subject: [PATCH 1/8] import: hackage: Add recognition of 'true' and 'false'
 symbols.

* guix/import/cabal.scm (is-true, is-false, lex-true, lex-false): New procedures.
  (lex-word): Use them.
  (make-cabal-parser): Add TRUE and FALSE tokens.
  (eval): Add entries for 'true and 'false symbols.
---
 guix/import/cabal.scm | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 45d644a..8d84e09 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -138,7 +138,7 @@ to the stack."
   "Generate a parser for Cabal files."
   (lalr-parser
    ;; --- token definitions
-   (CCURLY VCCURLY OPAREN CPAREN TEST ID VERSION RELATION
+   (CCURLY VCCURLY OPAREN CPAREN TEST ID VERSION RELATION TRUE FALSE
            (right: IF FLAG EXEC TEST-SUITE SOURCE-REPO BENCHMARK LIB OCURLY)
            (left: OR)
            (left: PROPERTY AND)
@@ -206,6 +206,8 @@ to the stack."
    (if-then     (IF tests OCURLY exprs CCURLY) : `(if ,$2 ,$4 ())
                 (IF tests open exprs close)    : `(if ,$2 ,$4 ()))
    (tests       (TEST OPAREN ID CPAREN)        : `(,$1 ,$3)
+                (TRUE)                         : 'true
+                (FALSE)                        : 'false
                 (TEST OPAREN ID RELATION VERSION CPAREN)
                 : `(,$1 ,(string-append $3 " " $4 " " $5))
                 (TEST OPAREN ID RELATION VERSION AND RELATION VERSION CPAREN)
@@ -350,6 +352,10 @@ matching a string against the created regexp."
 
 (define (is-if s) (string-ci=? s "if"))
 
+(define (is-true s) (string-ci=? s "true"))
+
+(define (is-false s) (string-ci=? s "false"))
+
 (define (is-and s) (string=? s "&&"))
 
 (define (is-or s) (string=? s "||"))
@@ -424,6 +430,10 @@ string with the read characters."
 
 (define (lex-if loc) (make-lexical-token 'IF loc #f))
 
+(define (lex-true loc) (make-lexical-token 'TRUE loc #t))
+
+(define (lex-false loc) (make-lexical-token 'FALSE loc #f))
+
 (define (lex-and loc) (make-lexical-token 'AND loc #f))
 
 (define (lex-or loc) (make-lexical-token 'OR loc #f))
@@ -489,6 +499,8 @@ LOC is the current port location."
   (let* ((w (read-delimited " ()\t\n" port 'peek)))
     (cond ((is-if w) (lex-if loc))
           ((is-test w port) (lex-test w loc))
+          ((is-true w) (lex-true loc))
+          ((is-false w) (lex-false loc))
           ((is-and w) (lex-and loc))
           ((is-or w) (lex-or loc))
           ((is-id w) (lex-id w loc))
@@ -714,6 +726,8 @@ the ordering operation and the version."
       (('os name) (os name))
       (('arch name) (arch name))
       (('impl name) (impl name))
+      ('true #t)
+      ('false #f)
       (('not name) (not (eval name)))
       ;; 'and' and 'or' aren't functions, thus we can't use apply
       (('and args ...) (fold (lambda (e s) (and e s)) #t (eval args)))
-- 
2.4.3


[-- Attachment #3: 0002-import-hackage-Imporve-parsing-of-tests.patch --]
[-- Type: text/x-diff, Size: 2216 bytes --]

From d96a655a232ba77d7d71a5227c6d3c8bc8b983cc Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 11 Nov 2015 11:22:42 +0100
Subject: [PATCH 2/8] import: hackage: Imporve parsing of tests.

* guix/import/cabal.scm (lex-word): Add support for tests with no spaces.
  (impl): Rewrite.
---
 guix/import/cabal.scm | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 8d84e09..ed6394e 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -30,6 +30,7 @@
   #:use-module (srfi srfi-9 gnu)
   #:use-module (system base lalr)
   #:use-module (rnrs enums)
+  #:use-module (guix utils)
   #:export (read-cabal
             eval-cabal
             
@@ -496,7 +497,7 @@ location."
 (define (lex-word port loc)
   "Process tokens which can be recognized by reading the next word form PORT.
 LOC is the current port location."
-  (let* ((w (read-delimited " ()\t\n" port 'peek)))
+  (let* ((w (read-delimited " <>=()\t\n" port 'peek)))
     (cond ((is-if w) (lex-if loc))
           ((is-test w port) (lex-test w loc))
           ((is-true w) (lex-true loc))
@@ -696,11 +697,18 @@ the ordering operation and the version."
                   ((spec-name spec-op spec-ver)
                    (comp-spec-name+op+version haskell)))
       (if (and spec-ver comp-ver)
-          (eval-string
-           (string-append "(string" spec-op " \"" comp-name "\""
-                          " \"" spec-name "-" spec-ver "\")"))
+          (cond
+           ((not (string= spec-name comp-name)) #f)
+           ((string= spec-op "==") (string= spec-ver comp-ver))
+           ((string= spec-op ">=") (version>=? comp-ver spec-ver))
+           ((string= spec-op ">") (version>? comp-ver spec-ver))
+           ((string= spec-op "<=") (not (version>? comp-ver spec-ver)))
+           ((string= spec-op "<") (not (version>=? comp-ver spec-ver)))
+           (else
+            (raise (condition
+                    (&message (message "Failed to evaluate 'impl' test."))))))
           (string-match spec-name comp-name))))
-  
+
   (define (cabal-flags)
     (make-cabal-section cabal-sexp 'flag))
   
-- 
2.4.3


[-- Attachment #4: 0003-import-hackage-Make-it-resilient-to-missing-final-ne.patch --]
[-- Type: text/x-diff, Size: 2582 bytes --]

From 614f9a9b685bcefa4e355b8c259225b0f098bc72 Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 11 Nov 2015 15:31:46 +0100
Subject: [PATCH 3/8] import: hackage: Make it resilient to missing final
 newline.

* guix/import/cabal.scm (peek-next-line-indent): Check for missing final
  newline.
---
 guix/import/cabal.scm | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index ed6394e..0c26e40 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -227,19 +227,24 @@ to the stack."
   "This function can be called when the next character on PORT is #\newline
 and returns the indentation of the line starting after the #\newline
 character.  Discard (and consume) empty and comment lines."
-  (let ((initial-newline (string (read-char port))))
-    (let loop ((char (peek-char port))
-               (word ""))
-      (cond ((eqv? char #\newline) (read-char port)
-             (loop (peek-char port) ""))
-            ((or (eqv? char #\space) (eqv? char #\tab))
-             (let ((c (read-char port)))
-               (loop (peek-char port) (string-append word (string c)))))
-            ((comment-line port char) (loop (peek-char port) ""))
-            (else
-             (let ((len (string-length word)))
-               (unread-string (string-append initial-newline word) port)
-               len))))))
+  (if (eof-object? (peek-char port))
+      ;; If the file is missing the #\newline on the last line, add it and act
+      ;; as if it were there. This is needed for proper operation of
+      ;; indentation based block recognition.
+      (begin (unread-char #\newline port) (read-char port) 0)
+      (let ((initial-newline (string (read-char port))))
+        (let loop ((char (peek-char port))
+                   (word ""))
+          (cond ((eqv? char #\newline) (read-char port)
+                 (loop (peek-char port) ""))
+                ((or (eqv? char #\space) (eqv? char #\tab))
+                 (let ((c (read-char port)))
+                   (loop (peek-char port) (string-append word (string c)))))
+                ((comment-line port char) (loop (peek-char port) ""))
+                (else
+                 (let ((len (string-length word)))
+                   (unread-string (string-append initial-newline word) port)
+                   len)))))))
 
 (define* (read-value port value min-indent #:optional (separator " "))
   "The next character on PORT must be #\newline.  Append to VALUE the
-- 
2.4.3


[-- Attachment #5: 0004-import-hackage-Make-parsing-of-tests-and-fields-more.patch --]
[-- Type: text/x-diff, Size: 2526 bytes --]

From 81e55b496195cc9e9aa41a2cf57117326cf93245 Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 11 Nov 2015 16:20:45 +0100
Subject: [PATCH 4/8] import: hackage: Make parsing of tests and fields more
 flexible.

* guix/import/cabal.scm (is-test): Allow spaces between keyword and
  parentheses.
  (is-id): Add argument 'port'.  Allow spaces between keyword and column.
  (lex-word): Adjust call to 'is-id'.
---
 guix/import/cabal.scm | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 0c26e40..7755e3c 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -333,7 +333,7 @@ matching a string against the created regexp."
                 (make-regexp pat))))
     (cut regexp-exec rx <>)))
 
-(define is-property (make-rx-matcher "([a-z0-9-]+):[ \t]*(\\w?.*)$"
+(define is-property (make-rx-matcher "([a-z0-9-]+)[ \t]*:[ \t]*(\\w?.*)$"
                                      regexp/icase))
 
 (define is-flag (make-rx-matcher "^flag +([a-z0-9_-]+)"
@@ -366,17 +366,24 @@ matching a string against the created regexp."
 
 (define (is-or s) (string=? s "||"))
 
-(define (is-id s)
+(define (is-id s port)
   (let ((cabal-reserved-words
          '("if" "else" "library" "flag" "executable" "test-suite"
-           "source-repository" "benchmark")))
+           "source-repository" "benchmark"))
+        (spaces (read-while (cut char-set-contains? char-set:blank <>) port))
+        (c (peek-char port)))
+    (unread-string spaces port)
     (and (every (cut string-ci<> s <>) cabal-reserved-words)
-         (not (char=? (last (string->list s)) #\:)))))
+         (and (not (char=? (last (string->list s)) #\:))
+              (not (char=? #\: c))))))
 
 (define (is-test s port)
   (let ((tests-rx (make-regexp "os|arch|flag|impl"))
+        (spaces (read-while (cut char-set-contains? char-set:blank <>) port))
         (c (peek-char port)))
-    (and (regexp-exec tests-rx s) (char=? #\( c))))
+    (if (and (regexp-exec tests-rx s) (char=? #\( c))
+        #t
+        (begin (unread-string spaces port) #f))))
 
 ;; Lexers for individual tokens.
 
@@ -509,7 +516,7 @@ LOC is the current port location."
           ((is-false w) (lex-false loc))
           ((is-and w) (lex-and loc))
           ((is-or w) (lex-or loc))
-          ((is-id w) (lex-id w loc))
+          ((is-id w port) (lex-id w loc))
           (else (unread-string w port) #f))))
 
 (define (lex-line port loc)
-- 
2.4.3


[-- Attachment #6: 0005-utils-Add-canonical-newline-port.patch --]
[-- Type: text/x-diff, Size: 3227 bytes --]

From bdd4aa18e3f3a686ceae9040c8b7404984886ace Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Sat, 14 Nov 2015 15:00:36 +0100
Subject: [PATCH 5/8] utils: Add 'canonical-newline-port'.

* guix/utils.scm (canonical-newline-port): New procedure.
* tests/utils.scm ("canonical-newline-port"): New test.
---
 guix/utils.scm  | 34 ++++++++++++++++++++++++++++++++--
 tests/utils.scm |  6 ++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/guix/utils.scm b/guix/utils.scm
index 1542e86..7b589e6 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -29,7 +29,8 @@
   #:use-module (srfi srfi-39)
   #:use-module (srfi srfi-60)
   #:use-module (rnrs bytevectors)
-  #:use-module ((rnrs io ports) #:select (put-bytevector))
+  #:use-module (rnrs io ports)
+  #:use-module ((rnrs bytevectors) #:select (bytevector-u8-set!))
   #:use-module ((guix build utils)
                 #:select (dump-port package-name->name+version))
   #:use-module ((guix build syscalls) #:select (errno mkdtemp!))
@@ -90,7 +91,8 @@
             decompressed-port
             call-with-decompressed-port
             compressed-output-port
-            call-with-compressed-output-port))
+            call-with-compressed-output-port
+            canonical-newline-port))
 
 \f
 ;;;
@@ -746,6 +748,34 @@ elements after E."
             (if success?
                 (loop (absolute target) (+ depth 1))
                 file))))))
+
+(define (canonical-newline-port port)
+  "Return an input port that wraps PORT such that all newlines consist
+  of a single carriage return."
+  (define (get-position)
+    (if (port-has-port-position? port) (port-position port) #f))
+  (define (set-position! position)
+    (if (port-has-set-port-position!? port)
+        (set-port-position! position port)
+        #f))
+  (define (close) (close-port port))
+  (define (read! bv start n)
+    (let loop ((count 0)
+               (byte (get-u8 port)))
+      (cond ((eof-object? byte) count)
+            ((= count (- n 1))
+             (bytevector-u8-set! bv (+ start count) byte)
+             n)
+            ;; XXX: consume all LFs even if not followed by CR.
+            ((eqv? byte (char->integer #\return)) (loop count (get-u8 port)))
+            (else
+             (bytevector-u8-set! bv (+ start count) byte)
+             (loop (+ count 1) (get-u8 port))))))
+  (make-custom-binary-input-port "canonical-newline-port"
+                                 read!
+                                 get-position
+                                 set-position!
+                                 close))
 \f
 ;;;
 ;;; Source location.
diff --git a/tests/utils.scm b/tests/utils.scm
index b65d6d2..04a859f 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -318,6 +318,12 @@
    (string-append (%store-prefix)
                   "/qvs2rj2ia5vci3wsdb7qvydrmacig4pg-bash-4.2-p24")))
 
+(test-equal "canonical-newline-port"
+  "This is a journey\nInto the sound\nA journey ...\n"
+  (let ((port (open-string-input-port
+               "This is a journey\r\nInto the sound\r\nA journey ...\n")))
+    (get-string-all (canonical-newline-port port))))
+
 (test-end)
 
 (false-if-exception (delete-file temp-file))
-- 
2.4.3


[-- Attachment #7: 0006-import-hackage-Handle-CRLF-end-of-line-style.patch --]
[-- Type: text/x-diff, Size: 1881 bytes --]

From 32b848e0506d6deac0bd1130234e02fb645613ee Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Sat, 14 Nov 2015 15:15:00 +0100
Subject: [PATCH 6/8] import: hackage: Handle CRLF end of line style.

* guix/import/hackage.scm (hackage-fetch, hackage->guix-package): Use
  'canonical-newline-port'.
---
 guix/import/hackage.scm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/guix/import/hackage.scm b/guix/import/hackage.scm
index 3baa514..8725ffa 100644
--- a/guix/import/hackage.scm
+++ b/guix/import/hackage.scm
@@ -22,7 +22,8 @@
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-1)
   #:use-module ((guix download) #:select (download-to-store))
-  #:use-module ((guix utils) #:select (package-name->name+version))
+  #:use-module ((guix utils) #:select (package-name->name+version
+                                       canonical-newline-port))
   #:use-module (guix import utils)
   #:use-module (guix import cabal)
   #:use-module (guix store)
@@ -84,7 +85,8 @@ version."
     (call-with-temporary-output-file
      (lambda (temp port)
        (and (url-fetch url temp)
-            (call-with-input-file temp read-cabal))))))
+            (call-with-input-file temp
+              (compose read-cabal canonical-newline-port)))))))
 
 (define string->license
   ;; List of valid values from
@@ -216,7 +218,7 @@ to the Cabal file format definition.  The default value associated with the
 keys \"os\", \"arch\" and \"impl\" is \"linux\", \"x86_64\" and \"ghc\"
 respectively."
   (let ((cabal-meta (if port
-                        (read-cabal port)
+                        (read-cabal (canonical-newline-port port))
                         (hackage-fetch package-name))))
     (and=> cabal-meta (compose (cut hackage-module->sexp <>
                                     #:include-test-dependencies? 
-- 
2.4.3


[-- Attachment #8: 0008-import-hackage-Assume-current-ghc-package-version.patch --]
[-- Type: text/x-diff, Size: 1358 bytes --]

From 507404c508774e5edb1cda1027fee12dae263592 Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 25 Nov 2015 14:47:16 +0100
Subject: [PATCH 8/8] import: hackage: Assume current 'ghc' package version.

* guix/scripts/import/hackage.scm (%default-options): Do it.
  (ghc-default-version): New variable.
---
 guix/scripts/import/hackage.scm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/guix/scripts/import/hackage.scm b/guix/scripts/import/hackage.scm
index 97d042b..4e84278 100644
--- a/guix/scripts/import/hackage.scm
+++ b/guix/scripts/import/hackage.scm
@@ -19,6 +19,7 @@
 (define-module (guix scripts import hackage)
   #:use-module (guix ui)
   #:use-module (guix utils)
+  #:use-module (guix packages)
   #:use-module (guix scripts)
   #:use-module (guix import hackage)
   #:use-module (guix scripts import)
@@ -34,10 +35,13 @@
 ;;; Command-line options.
 ;;;
 
+(define ghc-default-version
+  (string-append "ghc-" (package-version (@ (gnu packages haskell) ghc))))
+
 (define %default-options
-  '((include-test-dependencies? . #t)
+  `((include-test-dependencies? . #t)
     (read-from-stdin? . #f)
-    ('cabal-environment . '())))
+    (cabal-environment . ,`(("impl" . ,ghc-default-version)))))
 
 (define (show-help)
   (display (_ "Usage: guix import hackage PACKAGE-NAME
-- 
2.4.3


[-- Attachment #9: 0007-import-hackage-Add-new-tests.patch --]
[-- Type: text/x-diff, Size: 2717 bytes --]

From bf0bc66ace3b2617178c28d9635dbb4bc3a89ce9 Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 25 Nov 2015 13:58:06 +0100
Subject: [PATCH 7/8] import: hackage: Add new tests.

* tests/hackage.scm (eval-test-with-cabal): Add optional argument.
  (test-cabal-3): New variable and test.
  (test-read-cabal-1): Exercise more parsing variants.
---
 tests/hackage.scm | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/tests/hackage.scm b/tests/hackage.scm
index 229bee3..b608ccd 100644
--- a/tests/hackage.scm
+++ b/tests/hackage.scm
@@ -50,8 +50,28 @@ build-depends:
 }
 ")
 
+;; Check compiler implementation test with and without spaces.
+(define test-cabal-3
+  "name: foo
+version: 1.0.0
+homepage: http://test.org
+synopsis: synopsis
+description: description
+license: BSD3
+library
+  if impl(ghc >= 7.2 && < 7.6)
+    Build-depends: ghc-a
+  if impl(ghc>=7.2&&<7.6)
+    Build-depends: ghc-b
+  if impl(ghc == 7.8)
+    Build-depends: 
+      HTTP       >= 4000.2.5 && < 4000.3,
+      mtl        >= 2.0      && < 3
+")
+
 ;; A fragment of a real Cabal file with minor modification to check precedence
-;; of 'and' over 'or'.
+;; of 'and' over 'or', missing final newline, spaces between keywords and
+;; parentheses and between key and column.
 (define test-read-cabal-1
   "name: test-me
 library
@@ -66,24 +86,23 @@ library
         Build-depends: base >= 3 && < 4
       else
         Build-depends: base < 3
-  if flag(base4point8) || flag(base4) && flag(base3)
+  if flag(base4point8) || flag (base4) && flag(base3)
     Build-depends: random
-  Build-depends: containers
+  Build-depends : containers
 
   -- Modules that are always built.
   Exposed-Modules:
-    Test.QuickCheck.Exception
-")
+    Test.QuickCheck.Exception")
 
 (test-begin "hackage")
 
-(define (eval-test-with-cabal test-cabal)
+(define* (eval-test-with-cabal test-cabal #:key (cabal-environment '()))
   (mock
    ((guix import hackage) hackage-fetch
     (lambda (name-version)
       (call-with-input-string test-cabal
         read-cabal)))
-   (match (hackage->guix-package "foo")
+   (match (hackage->guix-package "foo" #:cabal-environment cabal-environment)
      (('package
         ('name "ghc-foo")
         ('version "1.0.0")
@@ -116,6 +135,10 @@ library
 (test-assert "hackage->guix-package test 2"
   (eval-test-with-cabal test-cabal-2))
 
+(test-assert "hackage->guix-package test 3"
+  (eval-test-with-cabal test-cabal-3
+                        #:cabal-environment '(("impl" . "ghc-7.8"))))
+
 (test-assert "read-cabal test 1"
   (match (call-with-input-string test-read-cabal-1 read-cabal)
     ((("name" ("test-me"))
-- 
2.4.3

bug#21829: guix import hackage failures

[Ludovic Courtès]

Federico Beffa <beffa@ieee.org> skribis:

> On Sun, Nov 15, 2015 at 9:59 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Federico Beffa <beffa@ieee.org> skribis:
>>> * guix/import/cabal.scm (lex-word): Add support for tests with no spaces.
>>>   (impl): Fix handling of operator "==".
>>
>> LGTM, but I think it’d be great to add a test that illustrates the case
>> that this fixes (and to make sure it doesn’t come back later.)
>
> I've rewritten 'impl' and the new test that I've added covers this and more.

Great, thanks for following up!

>>> +      ;; indentation based block recognition.
>>> +      (begin (unread-char #\newline port) (read-char port) 0)
>>
>> Isn’t this equivalent to: 0 ?
>
> No. This is because at the start of a new line we check if and how
> many indentation blocks have ended. If the last line doesn't terminate
> this check is no done.

More generally, it looks like:

  (begin (do-effect!) (undo-effect!) val)

which I thought reduces to:

  val

Oh well, not crucial.

> From d13f06383d07e0ad4096ff7eb715264463738b0c Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 10:39:38 +0100
> Subject: [PATCH 1/8] import: hackage: Add recognition of 'true' and 'false'
>  symbols.
>
> * guix/import/cabal.scm (is-true, is-false, lex-true, lex-false): New procedures.
>   (lex-word): Use them.
>   (make-cabal-parser): Add TRUE and FALSE tokens.
>   (eval): Add entries for 'true and 'false symbols.

OK.

In general I think development, review, and quality benefit from adding
a test alongside a feature or bug-fix, even a small one like this (as
opposed to adding a test separately.)  We try to do this for the rest of
the repo.

Now, I don’t want to bother you more ;-), and the test added by the last
patch covers some of this, so that’s OK.

> From d96a655a232ba77d7d71a5227c6d3c8bc8b983cc Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 11:22:42 +0100
> Subject: [PATCH 2/8] import: hackage: Imporve parsing of tests.
>
> * guix/import/cabal.scm (lex-word): Add support for tests with no spaces.
>   (impl): Rewrite.

OK.

> From 614f9a9b685bcefa4e355b8c259225b0f098bc72 Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 15:31:46 +0100
> Subject: [PATCH 3/8] import: hackage: Make it resilient to missing final
>  newline.
>
> * guix/import/cabal.scm (peek-next-line-indent): Check for missing final
>   newline.

OK.

> From 81e55b496195cc9e9aa41a2cf57117326cf93245 Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 16:20:45 +0100
> Subject: [PATCH 4/8] import: hackage: Make parsing of tests and fields more
>  flexible.
>
> * guix/import/cabal.scm (is-test): Allow spaces between keyword and
>   parentheses.
>   (is-id): Add argument 'port'.  Allow spaces between keyword and column.
>   (lex-word): Adjust call to 'is-id'.

LGTM.

> From bdd4aa18e3f3a686ceae9040c8b7404984886ace Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Sat, 14 Nov 2015 15:00:36 +0100
> Subject: [PATCH 5/8] utils: Add 'canonical-newline-port'.
>
> * guix/utils.scm (canonical-newline-port): New procedure.
> * tests/utils.scm ("canonical-newline-port"): New test.

OK.

> From 32b848e0506d6deac0bd1130234e02fb645613ee Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Sat, 14 Nov 2015 15:15:00 +0100
> Subject: [PATCH 6/8] import: hackage: Handle CRLF end of line style.
>
> * guix/import/hackage.scm (hackage-fetch, hackage->guix-package): Use
>   'canonical-newline-port'.

OK.

> From 507404c508774e5edb1cda1027fee12dae263592 Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 25 Nov 2015 14:47:16 +0100
> Subject: [PATCH 8/8] import: hackage: Assume current 'ghc' package version.
>
> * guix/scripts/import/hackage.scm (%default-options): Do it.
>   (ghc-default-version): New variable.

OK.

> From bf0bc66ace3b2617178c28d9635dbb4bc3a89ce9 Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 25 Nov 2015 13:58:06 +0100
> Subject: [PATCH 7/8] import: hackage: Add new tests.
>
> * tests/hackage.scm (eval-test-with-cabal): Add optional argument.
>   (test-cabal-3): New variable and test.
>   (test-read-cabal-1): Exercise more parsing variants.

OK.

Please add “Partly fixes <http://bugs.gnu.org/21829>.” or “Fixes
<http://bugs.gnu.org/21829>.” in the commit logs as appropriate (see
past commits for examples.)

Thank you for the hard work!

Ludo’.

bug#21829: guix import hackage failures

[Federico Beffa]

On Wed, Nov 25, 2015 at 10:45 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Federico Beffa <beffa@ieee.org> skribis:

[...]

>>>> +      ;; indentation based block recognition.
>>>> +      (begin (unread-char #\newline port) (read-char port) 0)
>>>
>>> Isn’t this equivalent to: 0 ?
>>
>> No. This is because at the start of a new line we check if and how
>> many indentation blocks have ended. If the last line doesn't terminate
>> this check is no done.
>
> More generally, it looks like:
>
>   (begin (do-effect!) (undo-effect!) val)
>
> which I thought reduces to:
>
>   val

Since we are doing IO, there are side effects. The key difference is
the result of '(port-column port)' and that triggers what I mentioned.

Thanks for the review.
Regards,
Fede

bug#21829: guix import hackage failures

[Ludovic Courtès]

Federico Beffa <beffa@ieee.org> skribis:

> On Wed, Nov 25, 2015 at 10:45 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Federico Beffa <beffa@ieee.org> skribis:
>
> [...]
>
>>>>> +      ;; indentation based block recognition.
>>>>> +      (begin (unread-char #\newline port) (read-char port) 0)
>>>>
>>>> Isn’t this equivalent to: 0 ?
>>>
>>> No. This is because at the start of a new line we check if and how
>>> many indentation blocks have ended. If the last line doesn't terminate
>>> this check is no done.
>>
>> More generally, it looks like:
>>
>>   (begin (do-effect!) (undo-effect!) val)
>>
>> which I thought reduces to:
>>
>>   val
>
> Since we are doing IO, there are side effects. The key difference is
> the result of '(port-column port)' and that triggers what I mentioned.

Oooh, I see, thanks for explaining!  Then what about a comment like:

  Read a newline from PORT to reset its ‘port-column’, as expected by
  the indentation-based block recognition code.

I think it would be helpful for those like me who are hard of hearing.
;-)

Ludo’.

bug#21829: guix import hackage failures

[Federico Beffa]

On Wed, Nov 25, 2015 at 10:45 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Please add “Partly fixes <http://bugs.gnu.org/21829>.” or “Fixes
> <http://bugs.gnu.org/21829>.” in the commit logs as appropriate (see
> past commits for examples.)

After committing I realized that I forgot about this.

Sorry!
Fede

bug#21829: guix import hackage failures

[Ludovic Courtès]

Federico Beffa <beffa@ieee.org> skribis:

> On Wed, Nov 25, 2015 at 10:45 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Please add “Partly fixes <http://bugs.gnu.org/21829>.” or “Fixes
>> <http://bugs.gnu.org/21829>.” in the commit logs as appropriate (see
>> past commits for examples.)
>
> After committing I realized that I forgot about this.

OK.  Good news is we can close the bug now.  :-)

Thanks,
Ludo’.