bug-guix
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#21829: guix import hackage failures


From: Federico Beffa
Subject: bug#21829: guix import hackage failures
Date: Wed, 25 Nov 2015 17:55:28 +0100

On Sun, Nov 15, 2015 at 9:59 PM, Ludovic Courtès <address@hidden> wrote:
> Federico Beffa <address@hidden> 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 <address@hidden>
>> 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 <address@hidden>
>> 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 <address@hidden>
>> 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: 0001-import-hackage-Add-recognition-of-true-and-false-sym.patch
Description: Text Data

Attachment: 0002-import-hackage-Imporve-parsing-of-tests.patch
Description: Text Data

Attachment: 0003-import-hackage-Make-it-resilient-to-missing-final-ne.patch
Description: Text Data

Attachment: 0004-import-hackage-Make-parsing-of-tests-and-fields-more.patch
Description: Text Data

Attachment: 0005-utils-Add-canonical-newline-port.patch
Description: Text Data

Attachment: 0006-import-hackage-Handle-CRLF-end-of-line-style.patch
Description: Text Data

Attachment: 0008-import-hackage-Assume-current-ghc-package-version.patch
Description: Text Data

Attachment: 0007-import-hackage-Add-new-tests.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]