[PATCH] Two fixes for 'gexp->approximate-sexp', addressing some linter problems.

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Maxime Devos
Owner
unassigned
Submitted by
Maxime Devos
Severity
normal
M
M
Maxime Devos wrote on 3 Mar 2022 15:21
(address . guix-patches@gnu.org)
ffb6e750df4a97e19abcf2d3bd80df291266b969.camel@telenet.be
Hi guix,

Try "guix lint -c wrapper-inputs libaio". You'll see a false positive.
The first patch fixes it. This fix exposes another issue, causing
"guix lint -c wrapper-inputs hostapd" to backtrace. The second patches
fixes that.

Greetings,
Maxime.
From 2aae3582fec4ba6ca719eacaa61f17589b09755e Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Thu, 3 Mar 2022 13:57:03 +0000
Subject: [PATCH 1/2] gexp: Correctly handle unquoting S-exp objects.

TODO before committing: fix the link to issues.guix.gnu.org in tests/gexp.scm.

This fixes a false-positive in the linter:

guix lint -c 'wrapper-inputs' libaio

* guix/gexp.scm (gexp->approximate-sexp): Allow the 'thing' in <gexp-input> to
be a sexp, without approximation, by testing if it is a record.
* tests/gexp.scm ("unquoted sexp (not a gexp!)"): Test it.
---
guix/gexp.scm | 16 +++++++++-------
tests/gexp.scm | 15 ++++++++++++++-
2 files changed, 23 insertions(+), 8 deletions(-)

Toggle diff (83 lines)
diff --git a/guix/gexp.scm b/guix/gexp.scm
index 01dca902f7..c358662799 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -4,7 +4,7 @@
;;; Copyright © 2018 Jan Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2019, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
-;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2021, 2022 Maxime Devos <maximedevos@telenet.be>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -174,12 +174,14 @@ As a result, the S-expression will be approximate if GEXP has references."
(map (lambda (reference)
(match reference
(($ <gexp-input> thing output native)
- (if (gexp-like? thing)
- (gexp->approximate-sexp thing)
- ;; Simply returning 'thing' won't work in some
- ;; situations; see 'write-gexp' below.
- '(*approximate*)))
- (_ '(*approximate*))))
+ (cond ((gexp-like? thing)
+ (gexp->approximate-sexp thing))
+ ((not (record? thing)) ; a S-exp
+ thing)
+ (#true
+ ;; Simply returning 'thing' won't work in some
+ ;; situations; see 'write-gexp' below.
+ '(*approximate*))))))
(gexp-references gexp))))
(define (write-gexp gexp port)
diff --git a/tests/gexp.scm b/tests/gexp.scm
index ad8e1d57b8..5ac8a1c8ab 100644
--- a/tests/gexp.scm
+++ b/tests/gexp.scm
@@ -1,6 +1,6 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2014-2021 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2021-2022 Maxime Devos <maximedevos@telenet.be>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -120,6 +120,19 @@
(let ((inside (file-append coreutils "/bin/hello")))
(gexp->approximate-sexp #~(display '#$inside))))
+;; See <https://issues.guix.gnu.org/????>.
+(test-equal "unquoted sexp (not a gexp!)"
+ '(list #(foo) (foo) () "foo" foo #xf00)
+ (let ((inside/vector #(foo))
+ (inside/list '(foo))
+ (inside/empty '())
+ (inside/string "foo")
+ (inside/symbol 'foo)
+ (inside/number #xf00))
+ (gexp->approximate-sexp
+ #~(list #$inside/vector #$inside/list #$inside/empty #$inside/string
+ #$inside/symbol #$inside/number))))
+
(test-equal "no refs"
'(display "hello!")
(let ((exp (gexp (display "hello!"))))

base-commit: 877da38bd3f279d5618d6c6517f48b50541a1e4c
prerequisite-patch-id: 02723e31219a206a489935610553565886035b5c
prerequisite-patch-id: a459b4cdc55777caaaf388147b22bf63da979c1d
prerequisite-patch-id: 05987288b4e31e7c5de40d624d4c3150d880b97b
prerequisite-patch-id: e569fecf0a0045aa9b2220634d305efd0f4901d5
prerequisite-patch-id: 5edb85611913d7cd4000c9fe3f42ee06517df020
prerequisite-patch-id: 6832435e696002c76b2818f4aacb50aab287a227
prerequisite-patch-id: e5a8343cb7dd5c48b2d14a535998d0eef166cded
prerequisite-patch-id: 859427824cb6e22635aee667680609c6daff316b
prerequisite-patch-id: ccd7c42e84c99029ab51c08b6cf478883140a594
prerequisite-patch-id: 19e75f076ca3906e613c969241b83c5944a2528b
prerequisite-patch-id: 67c6a6401d3327f59c7814db59563a54b7fb22e6
prerequisite-patch-id: 6fd0c3aabf51d85f95633e4682667653c137cfb4
prerequisite-patch-id: db78631386ade908f53f9ccfe743cd679ea85047
--
2.30.2
From a34cb77369a6108e65be20ef36ab35bdf398daf1 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Thu, 3 Mar 2022 14:14:22 +0000
Subject: [PATCH 2/2] gexp: Correctly handle #$output in
'gexp->approximate-sexp'.

This addresses the following backtrace from
"guix lint -c wrapper-inputs hostapd":

Backtrace:ostapd@2.10 [wrapper-inputs]...
[...]
174:9 3 (gexp->approximate-sexp #<gexp (modify-phases %standard?>)
In srfi/srfi-1.scm:
586:17 2 (map1 (#<gexp-output out> #<gexp-input "pkg-config":o?>))
In guix/gexp.scm:
175:16 1 (_ _)
In ice-9/boot-9.scm:
1685:16 0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Throw to key `match-error' with args `("match" "no matching pattern" #<gexp-output out>)'.

* guix/gexp.scm (gexp->approximate-sexp): Handle the case where 'reference' is
a <gexp-output>,, by returning (*approximate*).
* tests/gexp.scm ("gexp->approximate-sexp, outputs"): Test it.
---
guix/gexp.scm | 3 ++-
tests/gexp.scm | 5 +++++
2 files changed, 7 insertions(+), 1 deletion(-)

Toggle diff (32 lines)
diff --git a/guix/gexp.scm b/guix/gexp.scm
index c358662799..22a6c6ab71 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -181,7 +181,8 @@ As a result, the S-expression will be approximate if GEXP has references."
(#true
;; Simply returning 'thing' won't work in some
;; situations; see 'write-gexp' below.
- '(*approximate*))))))
+ '(*approximate*))))
+ (($ <gexp-output>) '(*approximate*))))
(gexp-references gexp))))
(define (write-gexp gexp port)
diff --git a/tests/gexp.scm b/tests/gexp.scm
index 5ac8a1c8ab..5d98f836a7 100644
--- a/tests/gexp.scm
+++ b/tests/gexp.scm
@@ -147,6 +147,11 @@
(null? (gexp-inputs exp))
(gexp->sexp* exp))))
+(test-equal "gexp->approximate-sexp, outputs"
+ '(list 'out:foo (*approximate*) 'out:bar (*approximate*))
+ (gexp->approximate-sexp
+ #~(list 'out:foo #$output:foo 'out:bar #$output:bar)))
+
(test-equal "unquote"
'(display `(foo ,(+ 2 3)))
(let ((exp (gexp (display `(foo ,(+ 2 3))))))
--
2.30.2
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYiDO+BccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7m2pAPoDc7i6Sec2ogcYUJf3qIY2zZ9A
whSQ6Ntvo+D+GV3bSwEA6k3ZKg6qPhWatInKzncAef8TMjcRynoX3VeQObQP7QA=
=fuph
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 11 Mar 2022 23:22
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 54236@debbugs.gnu.org)
874k44w2mf.fsf@gnu.org
Hello,

Both patches LGTM, except for…

Maxime Devos <maximedevos@telenet.be> skribis:

Toggle quote (7 lines)
> From 2aae3582fec4ba6ca719eacaa61f17589b09755e Mon Sep 17 00:00:00 2001
> From: Maxime Devos <maximedevos@telenet.be>
> Date: Thu, 3 Mar 2022 13:57:03 +0000
> Subject: [PATCH 1/2] gexp: Correctly handle unquoting S-exp objects.
>
> TODO before committing: fix the link to issues.guix.gnu.org in tests/gexp.scm.

… this TODO. :-)

Is there an actual issue to refer to, or just this one?

Thanks,
Ludo’.
M
M
Maxime Devos wrote on 11 Mar 2022 23:34
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 54236@debbugs.gnu.org)
2db273749b8c489370969ebac214730818f288ff.camel@telenet.be
Ludovic Courtès schreef op vr 11-03-2022 om 23:22 [+0100]:
Toggle quote (2 lines)
> Is there an actual issue to refer to, or just this one?

Yes, it's a ‘self-referrent patch’. This patch
https://issues.guix.gnu.org/54236 refers to the corresponding issue

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYivOhRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7pmDAQC/hpKadRmz1qX2QH8/piEQ4RXn
M9Js1En9zkfOm8blfAD+JwiMmjYzMkRWD0yzGq2JqZM6/ZI+hdgrEb6gOpHnxAk=
=dY5z
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 13 Mar 2022 23:20
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 54236-done@debbugs.gnu.org)
87y21dsde2.fsf_-_@gnu.org
Maxime Devos <maximedevos@telenet.be> skribis:

Toggle quote (7 lines)
> Ludovic Courtès schreef op vr 11-03-2022 om 23:22 [+0100]:
>> Is there an actual issue to refer to, or just this one?
>
> Yes, it's a ‘self-referrent patch’. This patch
> <https://issues.guix.gnu.org/54236> refers to the corresponding issue
> <https://issues.guix.gnu.org/54236>.

Ah ah, got it. :-)

Applied, thanks!

Ludo’.
Closed
?