import: hackage: `elif` conditionals not supported

  • Done
  • quality assurance status badge
Details
2 participants
  • Lars-Dominik Braun
  • Philip Munksgaard
Owner
unassigned
Submitted by
Philip Munksgaard
Severity
normal
P
L
L
Lars-Dominik Braun wrote on 14 May 2022 15:53
(name . Philip Munksgaard)(address . philip@munksgaard.me)(address . bug-guix@gnu.org)
Yn+0YPG3wzvdxRli@noor.fritz.box
Hi Philip,

Toggle quote (1 lines)
> Since Cabal 2.2 conditional blocks support the `elif` construct [0], but our hackage importer does not currently support such expressions. This causes importing packages like `raaz` [1] to fail.
attached patch series fixes the import for `raaz` by adding support for
elif and other minor adjustments. There still is syntax we do not support,
most importantly mixed indentation, which is already documented as xfail
via a testcase. I’m adding a few more.

Could you have a look please if these make sense?

Cheers,
Lars
Attachment: 0001-import-cabal-Support-elif-statement.patch
From dad8dbb8dde18716921523b5db722a168410740a Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Sat, 30 Apr 2022 15:39:34 +0200
Subject: [PATCH 2/5] import: cabal: Allow curly brackets in more positions.

* guix/import/cabal.scm (is-layout-property): Do not expect end of line.
(lex-layout-property): Check for newline.
(lex-property): Stop reading on closing curly bracket.
* tests/hackage.scm (test-read-cabal-2): New variable.
("read-cabal test: if brackets on the same line"): New test.
---
guix/import/cabal.scm | 11 ++++++++---
tests/hackage.scm | 16 ++++++++++++++++
2 files changed, 24 insertions(+), 3 deletions(-)

Toggle diff (72 lines)
diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index e1a082a31a..364fcc3176 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -354,7 +354,7 @@ (define* (make-rx-matcher pat #:optional (flag #f))
(make-regexp pat))))
(cut regexp-exec rx <>)))
-(define is-layout-property (make-rx-matcher "([a-z0-9-]+)[ \t]*:[ \t]*(\\w?[^{}]*)$"
+(define is-layout-property (make-rx-matcher "([a-z0-9-]+)[ \t]*:[ \t]*(\\w?[^{}]*)"
regexp/icase))
(define is-braced-property (make-rx-matcher "([a-z0-9-]+)[ \t]*:[ \t]*\\{[ \t]*$"
@@ -465,7 +465,10 @@ (define (lex-layout-property k-v-rx-res loc port)
(value (match:substring k-v-rx-res 2)))
(make-lexical-token
'PROPERTY loc
- (list key `(,(read-value port value (current-indentation)))))))
+ (list key `(,(if (eqv? (peek-char port) #\newline) ; The next character
+ ; is not necessarily a newline if a bracket follows the property.
+ (read-value port value (current-indentation))
+ value))))))
(define (lex-braced-property k-rx-res loc port)
(let ((key (string-downcase (match:substring k-rx-res 1))))
@@ -600,7 +603,9 @@ (define (lex-line port loc)
(else (unread-string s port) #f))))
(define (lex-property port loc)
- (let* ((s (read-delimited "\n" port 'peek)))
+ ;; Stop reading on a }, so closing brackets (for example during
+ ;; if-clauses) work properly.
+ (let* ((s (read-delimited "\n}" port 'peek)))
(cond
((is-braced-property s) => (cut lex-braced-property <> loc port))
((is-layout-property s) => (cut lex-layout-property <> loc port))
diff --git a/tests/hackage.scm b/tests/hackage.scm
index 38f75b268e..15309a3381 100644
--- a/tests/hackage.scm
+++ b/tests/hackage.scm
@@ -156,6 +156,12 @@ (define test-read-cabal-1
Exposed-Modules:
Test.QuickCheck.Exception")
+(define test-read-cabal-2
+ "name: test-me
+common defaults
+ if os(foobar) { cc-options: -DBARBAZ }
+") ; Intentional newline.
+
(test-begin "hackage")
(define-syntax-rule (define-package-matcher name pattern)
@@ -471,6 +477,16 @@ (define-package-matcher match-ghc-foo-revision
#t)
(x (pk 'fail x #f))))
+(test-assert "read-cabal test: if brackets on the same line"
+ (match (call-with-input-string test-read-cabal-2 read-cabal)
+ ((("name" ("test-me"))
+ ('section 'common "defaults"
+ (('if ('os "foobar")
+ (("cc-options" ("-DBARBAZ ")))
+ ()))))
+ #t)
+ (x (pk 'fail x #f))))
+
(define test-cabal-import
"name: foo
version: 1.0.0
--
2.35.1
From ba96d2e9af8f0b952bfa90f548e4e06dbdec4777 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Sat, 30 Apr 2022 15:39:51 +0200
Subject: [PATCH 3/5] import: cabal: Allow properties without space between key
and value.

* guix/import/cabal.scm (lex-word): Add colon to delimiters.
* tests/hackage.scm (test-cabal-property-no-space): New variable.
("hackage->guix-package test properties without space"): New test.
---
guix/import/cabal.scm | 2 +-
tests/hackage.scm | 19 +++++++++++++++++++
2 files changed, 20 insertions(+), 1 deletion(-)

Toggle diff (45 lines)
diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 364fcc3176..9f3862fa14 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -573,7 +573,7 @@ (define (lex-single-char port loc)
(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-elif w) (lex-elif loc))
((is-test w port) (lex-test w loc))
diff --git a/tests/hackage.scm b/tests/hackage.scm
index 15309a3381..4ce48b6baf 100644
--- a/tests/hackage.scm
+++ b/tests/hackage.scm
@@ -315,6 +315,25 @@ (define test-cabal-flag-executable
(test-assert "hackage->guix-package test flag executable"
(eval-test-with-cabal test-cabal-flag-executable match-ghc-foo))
+;; There is no mandatory space between property name and value.
+(define test-cabal-property-no-space
+ "name:foo
+version:1.0.0
+homepage:http://test.org
+synopsis:synopsis
+description:description
+license:BSD3
+common bench-defaults
+ ghc-options:-Wall
+executable cabal
+ build-depends:
+ HTTP >= 4000.2.5 && < 4000.3,
+ mtl >= 2.0 && < 3
+")
+
+(test-assert "hackage->guix-package test properties without space"
+ (eval-test-with-cabal test-cabal-property-no-space match-ghc-foo))
+
;; Check if-elif-else statements
(define test-cabal-if
"name: foo
--
2.35.1
From 5f4e1c75a34744f04916e1db8056669e20708ef0 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Sun, 1 May 2022 08:34:42 +0200
Subject: [PATCH 4/5] import: cabal: Allow curly bracket before else statement.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* guix/import/cabal.scm (is-else): Turn into procedure.
(lex-line): Move IS-ELSE…
(lex-word): …here.
* tests/hackage.scm (test-cabal-elif-brackets): Extend testcase.
---
guix/import/cabal.scm | 4 ++--
tests/hackage.scm | 5 ++++-
2 files changed, 6 insertions(+), 3 deletions(-)

Toggle diff (47 lines)
diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index 9f3862fa14..8f59a63cb9 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -384,7 +384,7 @@ (define is-benchmark (make-rx-matcher "^benchmark +([a-z0-9_-]+)"
(define is-lib (make-rx-matcher "^library *" regexp/icase))
-(define is-else (make-rx-matcher "^else" regexp/icase))
+(define (is-else s) (string-ci=? s "else"))
(define (is-elif s) (string-ci=? s "elif"))
@@ -576,6 +576,7 @@ (define (lex-word port loc)
(let* ((w (read-delimited " <>=():\t\n" port 'peek)))
(cond ((is-if w) (lex-if loc))
((is-elif w) (lex-elif loc))
+ ((is-else w) (lex-else loc))
((is-test w port) (lex-test w loc))
((is-true w) (lex-true loc))
((is-false w) (lex-false loc))
@@ -599,7 +600,6 @@ (define (lex-line port loc)
((is-custom-setup s) => (cut lex-custom-setup <> loc))
((is-benchmark s) => (cut lex-benchmark <> loc))
((is-lib s) (lex-lib loc))
- ((is-else s) (lex-else loc))
(else (unread-string s port) #f))))
(define (lex-property port loc)
diff --git a/tests/hackage.scm b/tests/hackage.scm
index 4ce48b6baf..98f9c34fd2 100644
--- a/tests/hackage.scm
+++ b/tests/hackage.scm
@@ -398,7 +398,10 @@ (define test-cabal-elif-brackets
elif os(guix) { Build-depends: ghc-c }
elif os(third) {
Build-depends: ghc-d }
- else
+ elif os(fourth)
+ {
+ Build-depends: ghc-d
+ } else
Build-depends: ghc-e
")
--
2.35.1
From bd67403967e86d41235ef3af2e142869bc6c5c48 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Sat, 14 May 2022 15:38:14 +0200
Subject: [PATCH 5/5] import: cabal: Document failing syntax through tests.

* tests/hackage.scm (test-read-cabal-brackets-newline): New variable.
(test-cabal-no-final-newline): Likewise.
("hackage->guix-package test without final newline",
"read-cabal test: property brackets on new line"): New tests.
---
tests/hackage.scm | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

Toggle diff (61 lines)
diff --git a/tests/hackage.scm b/tests/hackage.scm
index 98f9c34fd2..d7ecd0cc21 100644
--- a/tests/hackage.scm
+++ b/tests/hackage.scm
@@ -162,6 +162,16 @@ (define test-read-cabal-2
if os(foobar) { cc-options: -DBARBAZ }
") ; Intentional newline.
+;; Test opening bracket on new line.
+(define test-read-cabal-brackets-newline
+ "name: test-me
+common defaults
+ build-depends:
+ { foobar
+ , barbaz
+ }
+")
+
(test-begin "hackage")
(define-syntax-rule (define-package-matcher name pattern)
@@ -334,6 +344,21 @@ (define test-cabal-property-no-space
(test-assert "hackage->guix-package test properties without space"
(eval-test-with-cabal test-cabal-property-no-space match-ghc-foo))
+;; There may be no final newline terminating a property.
+(define test-cabal-no-final-newline
+"name: foo
+version: 1.0.0
+homepage: http://test.org
+synopsis: synopsis
+description: description
+license: BSD3
+executable cabal
+ build-depends: HTTP >= 4000.2.5 && < 4000.3, mtl >= 2.0 && < 3")
+
+(test-expect-fail 1)
+(test-assert "hackage->guix-package test without final newline"
+ (eval-test-with-cabal test-cabal-no-final-newline match-ghc-foo))
+
;; Check if-elif-else statements
(define test-cabal-if
"name: foo
@@ -509,6 +534,15 @@ (define-package-matcher match-ghc-foo-revision
#t)
(x (pk 'fail x #f))))
+(test-expect-fail 1)
+(test-assert "read-cabal test: property brackets on new line"
+ (match (call-with-input-string test-read-cabal-brackets-newline read-cabal)
+ ((("name" ("test-me"))
+ ('section 'common "defaults"
+ (("build-depends" ("foobar , barbaz")))))
+ #t)
+ (x (pk 'fail x #f))))
+
(define test-cabal-import
"name: foo
version: 1.0.0
--
2.35.1
P
P
Philip Munksgaard wrote on 16 May 2022 21:08
(name . Lars-Dominik Braun)(address . lars@6xq.net)(name . John Kehayias)(address . bug-guix@gnu.org)
7c74e5b9-6c37-4b9b-aa3b-40a4a5373e03@www.fastmail.com
Hi Lars

On Sat, 14 May 2022, at 15:53, Lars-Dominik Braun wrote:
Toggle quote (8 lines)
> attached patch series fixes the import for `raaz` by adding support for
> elif and other minor adjustments. There still is syntax we do not support,
> most importantly mixed indentation, which is already documented as xfail
> via a testcase. I’m adding a few more.
>
> Could you have a look please if these make sense?
>

Looks like it works! Unfortunately, building raaz still fails (first because of the description, then because some internal libraries are added as external dependencies, and then for... other reasons I haven't figured out. But all of those are separate issues.
L
L
Lars-Dominik Braun wrote on 22 May 2022 11:55
(name . Philip Munksgaard)(address . philip@munksgaard.me)(address . 54752@debbugs.gnu.org)
YooIdNFy+tukdkN2@noor.fritz.box
Hi Philip,

thank you!

Toggle quote (2 lines)
> Unfortunately, building raaz still fails (first because of the description, then because some internal libraries are added as external dependencies, and then for... other reasons I haven't figured out. But all of those are separate issues.

You’re right, it fails with this error for me, which indicates haskell-build-system has a bug _somewhere_.

---snip---
starting phase `register'
running "runhaskell Setup.hs" with command "register" and parameters ("--gen-pkg-config=/gnu/store/8m0w1688syvcmhjr2ym1max430jmwjag-ghc-raaz-0.3.6/ghc-raaz-0.3.6.conf")
error: in phase 'register': uncaught exception:
system-error "fport_read" "~A" ("Is a directory") (21)
phase `register' failed after 6.9 seconds
Backtrace:
11 (primitive-load "/gnu/store/jpf2lwn33nq08nkqrkr5q232c51…")
In guix/build/gnu-build-system.scm:
906:2 10 (gnu-build #:source _ #:outputs _ #:inputs _ #:phases . #)
In ice-9/boot-9.scm:
1752:10 9 (with-exception-handler _ _ #:unwind? _ # _)
In srfi/srfi-1.scm:
634:9 8 (for-each #<procedure 7ffff2048e20 at guix/build/gnu-b…> …)
In ice-9/boot-9.scm:
1752:10 7 (with-exception-handler _ _ #:unwind? _ # _)
In guix/build/gnu-build-system.scm:
927:23 6 (_)
In guix/build/haskell-build-system.scm:
244:23 5 (register #:name _ #:system _ #:inputs _ #:outputs _)
In ice-9/ports.scm:
433:17 4 (call-with-input-file _ _ #:binary _ #:encoding _ # _)
In ice-9/rdelim.scm:
160:18 3 (read-string _)
In unknown file:
2 (read-char #<input: /gnu/store/8m0w1688syvcmhjr2ym1max4…>)
In ice-9/boot-9.scm:
1685:16 1 (raise-exception _ #:continuable? _)
1685:16 0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure fport_read: Is a directory
---snap---

Cheers,
Lars
P
P
Philip Munksgaard wrote on 2 Jun 2022 11:54
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 54752@debbugs.gnu.org)
d57680ba-11eb-4f11-95e8-cc50066cb3fb@www.fastmail.com
On Sun, 22 May 2022, at 11:55, Lars-Dominik Braun wrote:
Toggle quote (10 lines)
> Hi Philip,
>
> thank you!
>
>> Unfortunately, building raaz still fails (first because of the description, then because some internal libraries are added as external dependencies, and then for... other reasons I haven't figured out. But all of those are separate issues.
>
> You’re right, it fails with this error for me, which indicates
> haskell-build-system has a bug _somewhere_.
>

Yes, I don't think that should hold back these patches from going into wip-haskell.
L
L
Lars-Dominik Braun wrote on 6 Jun 2022 13:31
(name . Philip Munksgaard)(address . philip@munksgaard.me)(address . 54752-done@debbugs.gnu.org)
Yp3lmw9e4omrS8d8@noor.fritz.box
Hi,

fix pushed to master in commit 2c5d18e421e6c06f4a969f98585ec41aae8eb2e4.

Cheers,
Lars
Closed
?