bug#47375: guix test failure: tests/print

bug#47375: guix test failure: tests/print

[Léo Le Bouter via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 3239 bytes --]

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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#47375: guix test failure: tests/print

[Julien Lepiller]

[-- Attachment #1: Type: text/plain, Size: 3861 bytes --]

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 :
>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 #2: Type: text/html, Size: 4295 bytes --]

bug#47375: guix test failure: tests/print

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]

Hi,

Julien Lepiller <julien@lepiller.eu> writes:

> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

bug#47375: guix test failure: tests/print

[Maxime Devos]

[-- Attachment #1.1: Type: text/plain, Size: 1337 bytes --]

On Wed, 2021-03-24 at 20:59 -0700, Chris Marusich wrote:
> [..]
> 
> 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 ...

<start guile code>
(define-module (mod)
  #:export (x y z))

(define (fun) 'stuff)
(define x fun)
(define y fun
(define z fun)
<end guile code>

... 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.

[-- Attachment #1.2: 0001-guix-Let-the-procedure-name-of-url-fetch-be-what-gui.patch --]
[-- Type: text/x-patch, Size: 4035 bytes --]

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(-)

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#47375: guix test failure: tests/print

[Maxime Devos]

[-- Attachment #1.1: Type: text/plain, Size: 79 bytes --]

Oops I messed up the commit message in the patch.
Revised patch is attached.

[-- Attachment #1.2: 0001-guix-Let-the-procedure-name-of-url-fetch-be-what-gui.patch --]
[-- Type: text/x-patch, Size: 2563 bytes --]

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(-)

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#47375: guix test failure: tests/print

[Ludovic Courtès]

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’.

bug#47375: guix test failure: tests/print

[Ludovic Courtès]

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’.

bug#47375: guix test failure: tests/print

[Léo Le Bouter via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 334 bytes --]

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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#47375: guix test failure: tests/print

[Ludovic Courtès]

Hi,

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

> 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’.

bug#47375: guix test failure: tests/print

[Léo Le Bouter via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

On Sun, 2021-03-28 at 18:25 +0200, Ludovic Courtès wrote:
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]