GNU bug report logs - #21829
guix import hackage failures

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 21829 in the body.
You can then email your comments to 21829 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul van der Walt <paul <at> denknerd.org>
To: bug-guix <at> gnu.org
Subject: guix import hackage failures
Date: Wed, 04 Nov 2015 16:00:26 +0100

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.

Message #8 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Paul van der Walt <paul <at> denknerd.org>
Cc: 21829 <at> debbugs.gnu.org
Subject: Re: bug#21829: guix import hackage failures
Date: Thu, 05 Nov 2015 00:11:07 +0100

Paul van der Walt <paul <at> 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’.

Message #11 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: 21829 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>, 
 Paul van der Walt <paul <at> denknerd.org>
Subject: guix import hackage failures
Date: Tue, 10 Nov 2015 17:40:40 +0100

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

Message #14 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Wed, 11 Nov 2015 12:18:47 +0100

Federico Beffa <beffa <at> 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’.

Message #17 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Wed, 11 Nov 2015 22:29:20 +0100

On Wed, Nov 11, 2015 at 12:18 PM, Ludovic Courtès <ludo <at> gnu.org> wrote:
> Federico Beffa <beffa <at> 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

Message #20 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Thu, 12 Nov 2015 10:07:59 +0100

[Message part 1 (text/plain, inline)]

Federico Beffa <beffa <at> ieee.org> skribis:

> On Wed, Nov 11, 2015 at 12:18 PM, Ludovic Courtès <ludo <at> gnu.org> wrote:
>> Federico Beffa <beffa <at> 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:

[Message part 2 (text/x-patch, inline)]

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)

[Message part 3 (text/plain, inline)]

WDYT?

Ludo’.

Message #23 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Thu, 12 Nov 2015 17:54:48 +0100

On Thu, Nov 12, 2015 at 10:07 AM, Ludovic Courtès <ludo <at> gnu.org> wrote:
> Federico Beffa <beffa <at> 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

Message #26 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Thu, 12 Nov 2015 21:21:32 +0100

Federico Beffa <beffa <at> ieee.org> skribis:

> On Thu, Nov 12, 2015 at 10:07 AM, Ludovic Courtès <ludo <at> gnu.org> wrote:
>> Federico Beffa <beffa <at> 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’.

Message #29 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Fri, 13 Nov 2015 18:08:33 +0100

On Thu, Nov 12, 2015 at 9:21 PM, Ludovic Courtès <ludo <at> 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

Message #32 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Fri, 13 Nov 2015 22:19:57 +0100

Federico Beffa <beffa <at> 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’.

Message #35 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Sat, 14 Nov 2015 15:37:35 +0100

[Message part 1 (text/plain, inline)]

On Fri, Nov 13, 2015 at 10:19 PM, Ludovic Courtès <ludo <at> gnu.org> wrote:
> Federico Beffa <beffa <at> 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

[0001-import-hackage-Add-recognition-of-true-and-false-sym.patch (text/x-diff, attachment)]

[0002-import-hackage-Imporve-parsing-of-tests.patch (text/x-diff, attachment)]

[0003-import-hackage-Make-it-resilient-to-missing-final-ne.patch (text/x-diff, attachment)]

[0004-import-hackage-Make-parsing-of-tests-and-fields-more.patch (text/x-diff, attachment)]

[0005-utils-Add-canonical-newline-port.patch (text/x-diff, attachment)]

[0006-import-hackage-Handle-CRLF-end-of-line-style.patch (text/x-diff, attachment)]

Message #38 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Sun, 15 Nov 2015 21:59:37 +0100

Federico Beffa <beffa <at> ieee.org> skribis:

> On Fri, Nov 13, 2015 at 10:19 PM, Ludovic Courtès <ludo <at> gnu.org> wrote:
>> Federico Beffa <beffa <at> 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 <at> 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 <at> 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 <at> 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 <at> 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 <at> 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 <at> 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’.

Message #41 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Wed, 25 Nov 2015 17:55:28 +0100

[Message part 1 (text/plain, inline)]

On Sun, Nov 15, 2015 at 9:59 PM, Ludovic Courtès <ludo <at> gnu.org> wrote:
> Federico Beffa <beffa <at> 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 <at> 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 <at> 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 <at> 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

[0001-import-hackage-Add-recognition-of-true-and-false-sym.patch (text/x-diff, attachment)]

[0002-import-hackage-Imporve-parsing-of-tests.patch (text/x-diff, attachment)]

[0003-import-hackage-Make-it-resilient-to-missing-final-ne.patch (text/x-diff, attachment)]

[0004-import-hackage-Make-parsing-of-tests-and-fields-more.patch (text/x-diff, attachment)]

[0005-utils-Add-canonical-newline-port.patch (text/x-diff, attachment)]

[0006-import-hackage-Handle-CRLF-end-of-line-style.patch (text/x-diff, attachment)]

[0008-import-hackage-Assume-current-ghc-package-version.patch (text/x-diff, attachment)]

[0007-import-hackage-Add-new-tests.patch (text/x-diff, attachment)]

Message #44 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Wed, 25 Nov 2015 22:45:15 +0100

Federico Beffa <beffa <at> ieee.org> skribis:

> On Sun, Nov 15, 2015 at 9:59 PM, Ludovic Courtès <ludo <at> gnu.org> wrote:
>> Federico Beffa <beffa <at> 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 <at> 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 <at> 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 <at> 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 <at> 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 <at> 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 <at> 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 <at> 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 <at> 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’.

Message #47 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Thu, 26 Nov 2015 09:28:07 +0100

On Wed, Nov 25, 2015 at 10:45 PM, Ludovic Courtès <ludo <at> gnu.org> wrote:
> Federico Beffa <beffa <at> 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

Message #50 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Thu, 26 Nov 2015 09:46:00 +0100

Federico Beffa <beffa <at> ieee.org> skribis:

> On Wed, Nov 25, 2015 at 10:45 PM, Ludovic Courtès <ludo <at> gnu.org> wrote:
>> Federico Beffa <beffa <at> 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’.

Message #53 received at 21829 <at> debbugs.gnu.org (full text, mbox):

From: Federico Beffa <beffa <at> ieee.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 21829 <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Thu, 26 Nov 2015 18:23:32 +0100

On Wed, Nov 25, 2015 at 10:45 PM, Ludovic Courtès <ludo <at> 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

Message #58 received at 21829-done <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Federico Beffa <beffa <at> ieee.org>
Cc: 21829-done <at> debbugs.gnu.org, Paul van der Walt <paul <at> denknerd.org>
Subject: Re: guix import hackage failures
Date: Thu, 26 Nov 2015 20:56:41 +0100

Federico Beffa <beffa <at> ieee.org> skribis:

> On Wed, Nov 25, 2015 at 10:45 PM, Ludovic Courtès <ludo <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.