guix test failure: tests/print

Done

Details

5 participants

Chris Marusich
Julien Lepiller
Léo Le Bouter
Ludovic Courtès
Maxime Devos

Owner: unassigned

Submitted by: Léo Le Bouter

Severity: normal

Léo Le Bouter wrote on 25 Mar 2021 02:44

Recipients:(address . bug-guix@gnu.org)

Message-ID:818412ea23ae7ce27a673afd1dfe3bb9d758c340.camel@zaclys.net

FAIL: tests/print
=================

test-name: simple package
location: /tmp/guix-build-guix-1.2.0-18.86dd54f.drv-
0/source/tests/print.scm:70
source:
+ (test-equal
+   "simple package"
+   `(define-public test ,pkg-source)
+   (package->code pkg))
expected-value: (define-public test (package (name "test") (version
"1.2.3") (source (origin (method url-fetch) (uri (string-append
"file:///tmp/test-" version ".tar.gz")) (sha256 (base32
"070pwb7brdcn1mfvplkd56vjc7lbz4iznzkqvfsakvgbv68k71ah")))) (build-
system (@ (guix build-system gnu) gnu-build-system)) (home-page "
http://gnu.org") (synopsis "Dummy") (description "This is a dummy
package.") (license license:gpl3+)))
actual-value: (define-public test (package (name "test") (version
"1.2.3") (source (origin (method url-fetch*) (uri (string-append
"file:///tmp/test-" version ".tar.gz")) (sha256 (base32
"070pwb7brdcn1mfvplkd56vjc7lbz4iznzkqvfsakvgbv68k71ah")))) (build-
system (@ (guix build-system gnu) gnu-build-system)) (home-page "
http://gnu.org") (synopsis "Dummy") (description "This is a dummy
package.") (license license:gpl3+)))
result: FAIL

test-name: package with inputs
location: /tmp/guix-build-guix-1.2.0-18.86dd54f.drv-
0/source/tests/print.scm:74
source:
+ (test-equal
+   "package with inputs"
+   `(define-public test ,pkg-with-inputs-source)
+   (package->code pkg-with-inputs))
expected-value: (define-public test (package (name "test") (version
"1.2.3") (source (origin (method url-fetch) (uri (string-append
"file:///tmp/test-" version ".tar.gz")) (sha256 (base32
"070pwb7brdcn1mfvplkd56vjc7lbz4iznzkqvfsakvgbv68k71ah")))) (build-
system (@ (guix build-system gnu) gnu-build-system)) (inputs
(quasiquote (("coreutils" (unquote (@ (gnu packages base) coreutils)))
("glibc" (unquote (@ (gnu packages base) glibc)) "debug")))) (home-page 
"http://gnu.org") (synopsis "Dummy") (description "This is a dummy
package.") (license license:gpl3+)))
actual-value: (define-public test (package (name "test") (version
"1.2.3") (source (origin (method url-fetch*) (uri (string-append
"file:///tmp/test-" version ".tar.gz")) (sha256 (base32
"070pwb7brdcn1mfvplkd56vjc7lbz4iznzkqvfsakvgbv68k71ah")))) (build-
system (@ (guix build-system gnu) gnu-build-system)) (inputs
(quasiquote (("coreutils" (unquote (@ (gnu packages base) coreutils)))
("glibc" (unquote (@ (gnu packages base) glibc)) "debug")))) (home-page 
"http://gnu.org") (synopsis "Dummy") (description "This is a dummy
package.") (license license:gpl3+)))
result: FAIL

Full log: https://ci.guix.gnu.org/build/117996/log/raw

Seems related to f7008ca71351e5368a7c1c5bc3fe88fb80b01298

This patch fixes the issue but I am not sure it retains the test's
original intent:

Toggle diff (17 lines)diff --git a/tests/print.scm b/tests/print.scm
index 3386590d3a..85735cf9ae 100644
--- a/tests/print.scm
+++ b/tests/print.scm
@@ -19,7 +19,7 @@
 (define-module (test-print)
   #:use-module (guix import print)
   #:use-module (guix build-system gnu)
-  #:use-module (guix download)
+  #:use-module (guix build download)
   #:use-module (guix packages)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (srfi srfi-64))

Let me know!

Thank you

Julien Lepiller wrote on 25 Mar 2021 02:54

Recipients:

Message-ID:C3C79618-8D9C-4830-BD49-BC2994CEBBE1@lepiller.eu

I don't think this is the right fix. Now you define packages with the incorrect url-fetch, so the test passes, but package->code would still not work as intended on actual packages that are properly defined.

It seems that the issue is package->code uses the internal name url-fetch* whereas it should be the exported name url-fetch. I think this is a legitimate bug in package->code and your patch incorrectly hides it.

Le 24 mars 2021 21:44:37 GMT-04:00, "Léo Le Bouter via Bug reports for GNU Guix" <bug-guix@gnu.org> a écrit :

Toggle quote (81 lines)>FAIL: tests/print
>=================
>
>test-name: simple package
>location: /tmp/guix-build-guix-1.2.0-18.86dd54f.drv-
>0/source/tests/print.scm:70
>source:
>+ (test-equal
>+   "simple package"
>+   `(define-public test ,pkg-source)
>+   (package->code pkg))
>expected-value: (define-public test (package (name "test") (version
>"1.2.3") (source (origin (method url-fetch) (uri (string-append
>"file:///tmp/test-" version ".tar.gz")) (sha256 (base32
>"070pwb7brdcn1mfvplkd56vjc7lbz4iznzkqvfsakvgbv68k71ah")))) (build-
>system (@ (guix build-system gnu) gnu-build-system)) (home-page "
>http://gnu.org") (synopsis "Dummy") (description "This is a dummy
>package.") (license license:gpl3+)))
>actual-value: (define-public test (package (name "test") (version
>"1.2.3") (source (origin (method url-fetch*) (uri (string-append
>"file:///tmp/test-" version ".tar.gz")) (sha256 (base32
>"070pwb7brdcn1mfvplkd56vjc7lbz4iznzkqvfsakvgbv68k71ah")))) (build-
>system (@ (guix build-system gnu) gnu-build-system)) (home-page "
>http://gnu.org") (synopsis "Dummy") (description "This is a dummy
>package.") (license license:gpl3+)))
>result: FAIL
>
>test-name: package with inputs
>location: /tmp/guix-build-guix-1.2.0-18.86dd54f.drv-
>0/source/tests/print.scm:74
>source:
>+ (test-equal
>+   "package with inputs"
>+   `(define-public test ,pkg-with-inputs-source)
>+   (package->code pkg-with-inputs))
>expected-value: (define-public test (package (name "test") (version
>"1.2.3") (source (origin (method url-fetch) (uri (string-append
>"file:///tmp/test-" version ".tar.gz")) (sha256 (base32
>"070pwb7brdcn1mfvplkd56vjc7lbz4iznzkqvfsakvgbv68k71ah")))) (build-
>system (@ (guix build-system gnu) gnu-build-system)) (inputs
>(quasiquote (("coreutils" (unquote (@ (gnu packages base) coreutils)))
>("glibc" (unquote (@ (gnu packages base) glibc)) "debug")))) (home-page
>
>"http://gnu.org") (synopsis "Dummy") (description "This is a dummy
>package.") (license license:gpl3+)))
>actual-value: (define-public test (package (name "test") (version
>"1.2.3") (source (origin (method url-fetch*) (uri (string-append
>"file:///tmp/test-" version ".tar.gz")) (sha256 (base32
>"070pwb7brdcn1mfvplkd56vjc7lbz4iznzkqvfsakvgbv68k71ah")))) (build-
>system (@ (guix build-system gnu) gnu-build-system)) (inputs
>(quasiquote (("coreutils" (unquote (@ (gnu packages base) coreutils)))
>("glibc" (unquote (@ (gnu packages base) glibc)) "debug")))) (home-page
>
>"http://gnu.org") (synopsis "Dummy") (description "This is a dummy
>package.") (license license:gpl3+)))
>result: FAIL
>
>Full log: https://ci.guix.gnu.org/build/117996/log/raw
>
>Seems related to f7008ca71351e5368a7c1c5bc3fe88fb80b01298
>
>This patch fixes the issue but I am not sure it retains the test's
>original intent:
>
>diff --git a/tests/print.scm b/tests/print.scm
>index 3386590d3a..85735cf9ae 100644
>--- a/tests/print.scm
>+++ b/tests/print.scm
>@@ -19,7 +19,7 @@
> (define-module (test-print)
>   #:use-module (guix import print)
>   #:use-module (guix build-system gnu)
>-  #:use-module (guix download)
>+  #:use-module (guix build download)
>   #:use-module (guix packages)
>   #:use-module ((guix licenses) #:prefix license:)
>   #:use-module (srfi srfi-64))
>
>Let me know!
>
>Thank you

Attachment: file

Chris Marusich wrote on 25 Mar 2021 04:59

Recipients:(name . Julien Lepiller)(address . julien@lepiller.eu)

Message-ID:87im5f7uj6.fsf@gmail.com

Hi,

Julien Lepiller <julien@lepiller.eu> writes:

Toggle quote (9 lines)

> I don't think this is the right fix. Now you define packages with the

> incorrect url-fetch, so the test passes, but package->code would still

> not work as intended on actual packages that are properly defined.

> It seems that the issue is package->code uses the internal name

> url-fetch* whereas it should be the exported name url-fetch. I think

> this is a legitimate bug in package->code and your patch incorrectly

> hides it.

I think Julien is correct.  The url-fetch* procedure from
guix/download.scm is here:

(define* (url-fetch* url hash-algo hash
                     #:optional name
                     #:key (system (%current-system))
                     (guile (default-guile))
                     executable?)
  "Return a fixed-output derivation that fetches data from URL (a string, or a
list of strings denoting alternate URLs), which is expected to have hash HASH
of type HASH-ALGO (a symbol).  By default, the file name is the base name of
URL; optionally, NAME can specify a different file name.  When EXECUTABLE? is
true, make the downloaded file executable.
...

And the url-fetch procedure from guix/build/download.scm is here:

(define* (url-fetch url file
                    #:key
                    (timeout 10) (verify-certificate? #t)
                    (mirrors '()) (content-addressed-mirrors '())
                    (hashes '())
                    print-build-trace?)
  "Fetch FILE from URL; URL may be either a single string, or a list of
string denoting alternate URLs for FILE.  Return #f on failure, and FILE
on success.
...

They do different things, even though they share the same name.  The
problem, apparently introduced with commit
f7008ca71351e5368a7c1c5bc3fe88fb80b01298, is that before the change,
package->code produced code that uses url-fetch, and after the change,
it produced code that uses url-fetch*.

Reverting the change fixes it.  I reverted it and tested it, and it does
fix the test.  I wonder if we can just revert it for now?  What do you
think, Ludo?

In guix/import/print.scm, package->code generates the code by invoking
(origin-method source) to get the origin's "method", and then invoking
(procedure-name method) on the method thus obtained.  It seems that
procedure-name returns the name "url-fetch*" (the name used privately in
the (guix download) module), but it should be returning the name
"url-fetch" (the public name exported by the (guix download) module).

I wonder if there is any way to get the public name of the procedure
programmatically, instead of the private one?

-- 
Chris

Maxime Devos wrote on 25 Mar 2021 08:55

Recipients:(address . 47375@debbugs.gnu.org)

Message-ID:3f09a5671f6a7c04405d8529f7ed60b1e7f8eb6e.camel@telenet.be

On Wed, 2021-03-24 at 20:59 -0700, Chris Marusich wrote:

Toggle quote (12 lines)> [..]
> 
> In guix/import/print.scm, package->code generates the code by invoking
> (origin-method source) to get the origin's "method", and then invoking
> (procedure-name method) on the method thus obtained.  It seems that
> procedure-name returns the name "url-fetch*" (the name used privately in
> the (guix download) module), but it should be returning the name
> "url-fetch" (the public name exported by the (guix download) module).
> 
> I wonder if there is any way to get the public name of the procedure
> programmatically, instead of the private one?

I believe there isn't, and I have spent some time around Guile's module

system. Note that one could do ...

(define-module (mod)

#:export (x y z))

(define (fun) 'stuff)

(define x fun)

(define y fun

(define z fun)

... in which case there would be more than one "public name".

However, there's a possible work-around. It is change the procedure name

of a procedure, with the following

(define name (let ((visible-name (lambda () 'stuff))) visible-name))

(procedure-name name) ; -> visible-name

Patch attached (though written against a somewhat outdated tree).

Does this patch fixes the issue? (I'm not in the loop.)

Greetings, Maxime.

From 04f36da115e2cb60835561f461899bb0d67bb927 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Thu, 25 Mar 2021 08:48:24 +0100
Subject: [PATCH] guix: Let the procedure name of "url-fetch*" be what "guix
 import" expects.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes: https://issues.guix.gnu.org/47375.
Reported-By: Léo Le Bouter <lle-bout@zaclys.net>.

* guix/download.scm
  (define*-visible-name): Define a syntax for overriding the procedure name
  of a procedure returned by "procedure-name".
  (url-fetch*): Use this syntax to change the procedure name to "url-fetch"
  as "guix import" expects.

+++ b/guix/download.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2019 Guy Fleury Iteriteka <hoonandon@gmail.com>
+;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? y
@@ -449,11 +450,19 @@ download by itself using its own dependencies."
                     ;; for that built-in is widespread.
                     #:local-build? #t)))

-(define* (url-fetch* url hash-algo hash
-                     #:optional name
-                     #:key (system (%current-system))
-                     (guile (default-guile))
-                     executable?)
+;; In guix/import/print.scm, the procedure package->code uses procedure-name
+;; and expects to see the user-visible procedure name.
+(define-syntax-rule (define*-with-name (name . args) procedure-name . code)
+  "Define a procedure as with 'define*', but in such a matter that
+'procedure-name' on NAME will return PROCEDURE-NAME instead of NAME."
+  (define name (let ((procedure-name (lambda* args . code))) procedure-name)))
+
+(define*-with-name (url-fetch* url hash-algo hash
+                               #:optional name
+                               #:key (system (%current-system))
+                               (guile (default-guile))
+                               executable?)
+  url-fetch
---
 guix/download.scm | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Toggle diff (39 lines)diff --git a/guix/download.scm b/guix/download.scm
index 30f69c0325..25c26a2ebb 100644
--- a/guix/download.scm
+++ b/guix/download.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2019 Guy Fleury Iteriteka <hoonandon@gmail.com>
+;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -449,11 +450,19 @@ download by itself using its own dependencies."
                     ;; for that built-in is widespread.
                     #:local-build? #t)))
 
-(define* (url-fetch* url hash-algo hash
-                     #:optional name
-                     #:key (system (%current-system))
-                     (guile (default-guile))
-                     executable?)
+;; In guix/import/print.scm, the procedure package->code uses procedure-name
+;; and expects to see the user-visible procedure name.
+(define-syntax-rule (define*-with-name (name . args) procedure-name . code)
+  "Define a procedure as with 'define*', but in such a matter that
+'procedure-name' on NAME will return PROCEDURE-NAME instead of NAME."
+  (define name (let ((procedure-name (lambda* args . code))) procedure-name)))
+
+(define*-with-name (url-fetch* url hash-algo hash
+                               #:optional name
+                               #:key (system (%current-system))
+                               (guile (default-guile))
+                               executable?)
+  url-fetch
   "Return a fixed-output derivation that fetches data from URL (a string, or a
 list of strings denoting alternate URLs), which is expected to have hash HASH
 of type HASH-ALGO (a symbol).  By default, the file name is the base name of
-- 
2.31.0

-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFxB2xccbWF4aW1lZGV2

b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7ptWAQCS7h5yq8j915ZalCd4AVnGeyY0

ga4POwj1P0VPLRicrwEAhWQTUVUqQQqepbspYuTf457adErsJTHGtVKeYyw2DgU=

=mSPI

-----END PGP SIGNATURE-----

Maxime Devos wrote on 25 Mar 2021 09:15

Recipients:(address . 47375@debbugs.gnu.org)

Message-ID:bd0986ed42b0dc6cae4109d4b47acecb6fc15906.camel@telenet.be

Oops I messed up the commit message in the patch.

Revised patch is attached.

From 2c6c28334173fff9112c36726d31d6adcebfbf2d Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Thu, 25 Mar 2021 08:48:24 +0100
Subject: [PATCH] guix: Let the procedure name of "url-fetch*" be what "guix
 import" expects.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes: https://issues.guix.gnu.org/47375.
Reported-By: Léo Le Bouter <lle-bout@zaclys.net>.

* guix/download.scm
  (define*-visible-name): Define a syntax for overriding the procedure name
  of a procedure returned by "procedure-name".
  (url-fetch*): Use this syntax to change the procedure name to "url-fetch"
  as "guix import" expects.
---
 guix/download.scm | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Toggle diff (39 lines)diff --git a/guix/download.scm b/guix/download.scm
index 30f69c0325..25c26a2ebb 100644
--- a/guix/download.scm
+++ b/guix/download.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2019 Guy Fleury Iteriteka <hoonandon@gmail.com>
+;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -449,11 +450,19 @@ download by itself using its own dependencies."
                     ;; for that built-in is widespread.
                     #:local-build? #t)))
 
-(define* (url-fetch* url hash-algo hash
-                     #:optional name
-                     #:key (system (%current-system))
-                     (guile (default-guile))
-                     executable?)
+;; In guix/import/print.scm, the procedure package->code uses procedure-name
+;; and expects to see the user-visible procedure name.
+(define-syntax-rule (define*-with-name (name . args) procedure-name . code)
+  "Define a procedure as with 'define*', but in such a matter that
+'procedure-name' on NAME will return PROCEDURE-NAME instead of NAME."
+  (define name (let ((procedure-name (lambda* args . code))) procedure-name)))
+
+(define*-with-name (url-fetch* url hash-algo hash
+                               #:optional name
+                               #:key (system (%current-system))
+                               (guile (default-guile))
+                               executable?)
+  url-fetch
   "Return a fixed-output derivation that fetches data from URL (a string, or a
 list of strings denoting alternate URLs), which is expected to have hash HASH
 of type HASH-ALGO (a symbol).  By default, the file name is the base name of
-- 
2.31.0

-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYFxGvBccbWF4aW1lZGV2

b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7mTgAP0YQkafhG4e13SqopnxRmRTN/co

QHC5GdUeKv+d4+Az8QD/doiFrml3Q8B1DCzZOwMg2aapEANpXzYbLZCW317a6Qs=

=6PnY

-----END PGP SIGNATURE-----

Ludovic Courtès wrote on 26 Mar 2021 00:23

Recipients:(name . Maxime Devos)(address . maximedevos@telenet.be)

Message-ID:877dlu4y3k.fsf@gnu.org

Hi!

Instead of renaming ‘url-fetch*’, I changed the thing that guesses the

procedure name so that it preferable uses the procedure’s public name,

rather than the name it has within its module. Done in

96aa98b6ca78ffb798e309acac3c3e5068422f30.

Thanks, and apologies for the breakage!

Ludo’.

Closed

Ludovic Courtès wrote on 26 Mar 2021 00:24

Recipients:(name . Léo Le Bouter)(address . lle-bout@zaclys.net)(address . 47375@debbugs.gnu.org)

Message-ID:8735wi4y1o.fsf@gnu.org

Léo Le Bouter <lle-bout@zaclys.net> skribis:

Toggle quote (2 lines)

> Full log: https://ci.guix.gnu.org/build/117996/log/raw

Speaking of which: please always build packages before pushing. :-)

Thanks,

Ludo’.

Léo Le Bouter wrote on 26 Mar 2021 00:27

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47375@debbugs.gnu.org)

Message-ID:afbc52353c757e9617795b99b8dcae02e502e6b9.camel@zaclys.net

On Fri, 2021-03-26 at 00:24 +0100, Ludovic Courtès wrote:

Toggle quote (9 lines)

> Léo Le Bouter <lle-bout@zaclys.net> skribis:

> > Full log: https://ci.guix.gnu.org/build/117996/log/raw

> Speaking of which: please always build packages before pushing. :-)

> Thanks,

> Ludo’.

I ran 'guix pull' but turns out that wasnt sufficient.

Ludovic Courtès wrote on 28 Mar 2021 18:25

Recipients:(name . Léo Le Bouter)(address . lle-bout@zaclys.net)(address . 47375@debbugs.gnu.org)

Message-ID:87blb3xn26.fsf@gnu.org

Hi,

Léo Le Bouter <lle-bout@zaclys.net> skribis:

Toggle quote (12 lines)> On Fri, 2021-03-26 at 00:24 +0100, Ludovic Courtès wrote:
>> Léo Le Bouter <lle-bout@zaclys.net> skribis:
>> 
>> > Full log: https://ci.guix.gnu.org/build/117996/log/raw
>> 
>> Speaking of which: please always build packages before pushing.  :-)
>> 
>> Thanks,
>> Ludo’.
>
> I ran 'guix pull' but turns out that wasnt sufficient.

When updating the ‘guix’ package, what you need to run is:

./pre-inst-env guix build guix

It’s similar to other packages.

In general, we update it when there are changes to the daemon and its

helper programs (‘guix substitute’, etc.). These are pretty much the

only reasons to update it.

HTH!

Ludo’.

Léo Le Bouter wrote on 28 Mar 2021 18:31

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47375@debbugs.gnu.org)

Message-ID:3c845c21b45238615d896bae7b61c075384a4af7.camel@zaclys.net

On Sun, 2021-03-28 at 18:25 +0200, Ludovic Courtès wrote:

Toggle quote (10 lines)> When updating the ‘guix’ package, what you need to run is:
> 
>   ./pre-inst-env guix build guix
> 
> It’s similar to other packages.
> 
> In general, we update it when there are changes to the daemon and its
> helper programs (‘guix substitute’, etc.).  These are pretty much the
> only reasons to update it.

Thanks will make sure to do that. I was a bit tired and I think I

misunderstood something in a discussion with Lfam on IRC that made me

only run 'guix pull' and restart guix-daemon.

Note, the reason I upgraded the 'guix' package here was to fix 'guix

pull' from older installations on powerpc64le-linux machines of mine.

Léo

Your comment

This issue is archived.

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

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date