[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
?
Your comment

This issue is archived.

To comment on this conversation send an email to 54236@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 54236
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch