unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] utils: Allow wrap-program to be called multiple times.
@ 2014-09-09 22:56 Eric Bavier
  2014-09-10 13:40 ` Ludovic Courtès
  2014-09-11 13:10 ` Ludovic Courtès
  0 siblings, 2 replies; 10+ messages in thread
From: Eric Bavier @ 2014-09-09 22:56 UTC (permalink / raw)
  To: guix-devel

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

Currently, if (@ (guix build utils) wrap-program) is called multiple
times for the same file, the original file ends up being overwritten.
This happened to me when trying to wrap a python program, which had
already once been wrapped by python-build-system.  The
python-build-system wrapper sets PYTHON_PATH, and I needed to wrap the
program again in order to set PATH.

Comments are very welcome on this patch to core-updates, as I hacked it
together rather quickly.

A description of what ends up happening, e.g.:

1) Initially::

  $ ls
  foo

2) Then after first call to wrap-program::

  $ ls
  foo -> ./.foo-wrap-01
  .foo-real
  .foo-wrap-01

3) And then after another call to wrap-program::

  $ ls
  foo -> ./.foo-wrap-02
  .foo-real
  .foo-wrap-01
  .foo-wrap-02


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-utils-Allow-wrap-program-to-be-called-multiple-times.patch --]
[-- Type: text/x-diff, Size: 2785 bytes --]

From 231130db4444685d8f3264e61d680634eaead9fb Mon Sep 17 00:00:00 2001
From: Eric Bavier <bavier@member.fsf.org>
Date: Tue, 9 Sep 2014 17:47:31 -0500
Subject: [PATCH] utils: Allow wrap-program to be called multiple times.

* guix/build/utils.scm (wrap-program): Multiple invocations of
  wrap-program for the same file create successive wrappers.
---
 guix/build/utils.scm |   26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 2f3dc9c..d4435b4 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -711,8 +711,24 @@ contents:
 This is useful for scripts that expect particular programs to be in $PATH, for
 programs that expect particular shared libraries to be in $LD_LIBRARY_PATH, or
 modules in $GUILE_LOAD_PATH, etc."
-  (let ((prog-real (string-append (dirname prog) "/." (basename prog) "-real"))
-        (prog-tmp  (string-append (dirname prog) "/." (basename prog) "-tmp")))
+  (define (wrapper-path num)
+    (format #f "~a/.~a-wrap-~2'0d" (dirname prog) (basename prog) num))
+  (let* ((current-wrappers
+          (find-files (dirname prog)
+                      (string-append "\\." (basename prog) "-wrap-.*")))
+         (wrapper-num (if (null? current-wrappers)
+                          0
+                          (string->number
+                           (string-take-right (last current-wrappers) 2))))
+         (wrapper-tgt (if (zero? wrapper-num)
+                          (let ((prog-real (string-append
+                                            (dirname prog) "/."
+                                            (basename prog) "-real")))
+                            (copy-file prog prog-real)
+                            prog-real)
+                          (wrapper-path wrapper-num)))
+         (wrapper     (wrapper-path (1+ wrapper-num)))
+         (prog-tmp    (string-append wrapper-tgt "-tmp")))
     (define (export-variable lst)
       ;; Return a string that exports an environment variable.
       (match lst
@@ -735,8 +751,6 @@ modules in $GUILE_LOAD_PATH, etc."
          (format #f "export ~a=\"$~a${~a:+:}~a\""
                  var var var (string-join rest ":")))))
 
-    (copy-file prog prog-real)
-
     (with-output-to-file prog-tmp
       (lambda ()
         (format #t
@@ -744,9 +758,11 @@ modules in $GUILE_LOAD_PATH, etc."
                 (which "bash")
                 (string-join (map export-variable vars)
                              "\n")
-                (canonicalize-path prog-real))))
+                (canonicalize-path wrapper-tgt))))
 
     (chmod prog-tmp #o755)
+    (rename-file prog-tmp wrapper)
+    (symlink wrapper prog-tmp)
     (rename-file prog-tmp prog)))
 
 ;;; Local Variables:
-- 
1.7.9.5


[-- Attachment #3: Type: text/plain, Size: 17 bytes --]


-- 
Eric Bavier

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-09 22:56 [PATCH] utils: Allow wrap-program to be called multiple times Eric Bavier
@ 2014-09-10 13:40 ` Ludovic Courtès
  2014-09-10 19:16   ` Eric Bavier
  2014-09-11 13:10 ` Ludovic Courtès
  1 sibling, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2014-09-10 13:40 UTC (permalink / raw)
  To: Eric Bavier; +Cc: guix-devel

Eric Bavier <ericbavier@gmail.com> skribis:

> Currently, if (@ (guix build utils) wrap-program) is called multiple
> times for the same file, the original file ends up being overwritten.
> This happened to me when trying to wrap a python program, which had
> already once been wrapped by python-build-system.  The
> python-build-system wrapper sets PYTHON_PATH, and I needed to wrap the
> program again in order to set PATH.

Wouldn’t it be easier or preferable to change ‘python-build-system’ to
offer a way to specify additional arguments for ‘wrap-program’?

The idea of having multiple indirections just to set a bunch of
environment variables doesn’t look appealing to me.

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-10 13:40 ` Ludovic Courtès
@ 2014-09-10 19:16   ` Eric Bavier
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Bavier @ 2014-09-10 19:16 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel


Ludovic Courtès writes:

> Eric Bavier <ericbavier@gmail.com> skribis:
>
>> Currently, if (@ (guix build utils) wrap-program) is called multiple
>> times for the same file, the original file ends up being overwritten.
>> This happened to me when trying to wrap a python program, which had
>> already once been wrapped by python-build-system.  The
>> python-build-system wrapper sets PYTHON_PATH, and I needed to wrap the
>> program again in order to set PATH.
>
> Wouldn’t it be easier or preferable to change ‘python-build-system’ to
> offer a way to specify additional arguments for ‘wrap-program’?

That was one possibility I considered.

> The idea of having multiple indirections just to set a bunch of
> environment variables doesn’t look appealing to me.

My thought was that it would allow better composability/decoupling of
packages, even if they aren't python packages.  E.g. if a package
inherits from another and needs to wrap an executable, the packager
might not want to bother, or should not need to bother, to check whether
that executable has already been wrapped.

In any case, wrap-program certainly should not silently overwrite an
existing '.foo-real'.

-- 
Eric Bavier

Please avoid sending me Word or PowerPoint attachments.
See http://www.gnu.org/philosophy/no-word-attachments.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-09 22:56 [PATCH] utils: Allow wrap-program to be called multiple times Eric Bavier
  2014-09-10 13:40 ` Ludovic Courtès
@ 2014-09-11 13:10 ` Ludovic Courtès
  2014-09-13  6:12   ` Eric Bavier
  1 sibling, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2014-09-11 13:10 UTC (permalink / raw)
  To: Eric Bavier; +Cc: guix-devel

Eric Bavier <ericbavier@gmail.com> skribis:

> Currently, if (@ (guix build utils) wrap-program) is called multiple
> times for the same file, the original file ends up being overwritten.

OK, you’ve convinced me that this improvement is welcome.

Some comments on the patch:

> From 231130db4444685d8f3264e61d680634eaead9fb Mon Sep 17 00:00:00 2001
> From: Eric Bavier <bavier@member.fsf.org>
> Date: Tue, 9 Sep 2014 17:47:31 -0500
> Subject: [PATCH] utils: Allow wrap-program to be called multiple times.
>
> * guix/build/utils.scm (wrap-program): Multiple invocations of
>   wrap-program for the same file create successive wrappers.
> ---
>  guix/build/utils.scm |   26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/guix/build/utils.scm b/guix/build/utils.scm
> index 2f3dc9c..d4435b4 100644
> --- a/guix/build/utils.scm
> +++ b/guix/build/utils.scm
> @@ -711,8 +711,24 @@ contents:
>  This is useful for scripts that expect particular programs to be in $PATH, for
>  programs that expect particular shared libraries to be in $LD_LIBRARY_PATH, or
>  modules in $GUILE_LOAD_PATH, etc."
> -  (let ((prog-real (string-append (dirname prog) "/." (basename prog) "-real"))
> -        (prog-tmp  (string-append (dirname prog) "/." (basename prog) "-tmp")))
> +  (define (wrapper-path num)
> +    (format #f "~a/.~a-wrap-~2'0d" (dirname prog) (basename prog) num))

Make it ‘wrapper-file-name’ (in GNU, “path” means “search path”.)

> +  (let* ((current-wrappers
> +          (find-files (dirname prog)
> +                      (string-append "\\." (basename prog) "-wrap-.*")))
> +         (wrapper-num (if (null? current-wrappers)
> +                          0
> +                          (string->number
> +                           (string-take-right (last current-wrappers) 2))))

These two could be factorized as a local ‘next-wrapper-number’
procedure.

For local variables, it’s better to use shorter names, such as
‘wrappers’ and ‘number’ here.

> +         (wrapper-tgt (if (zero? wrapper-num)
> +                          (let ((prog-real (string-append
> +                                            (dirname prog) "/."
> +                                            (basename prog) "-real")))
> +                            (copy-file prog prog-real)
> +                            prog-real)
> +                          (wrapper-path wrapper-num)))

Make it a ‘wrapper-target’ local procedure, and change ‘wrapper-tgt’ to
‘target’.

It looks OK.  It would be ideal if a test in tests/build-utils.scm made
sure that ‘wrap-program’ uses the right file names when called multiple
times, but I won’t object if the patch doesn’t have it.

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-11 13:10 ` Ludovic Courtès
@ 2014-09-13  6:12   ` Eric Bavier
  2014-09-13 12:20     ` Ludovic Courtès
  2014-09-14  4:05     ` mhw
  0 siblings, 2 replies; 10+ messages in thread
From: Eric Bavier @ 2014-09-13  6:12 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

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


Ludovic Courtès writes:

> Eric Bavier <ericbavier@gmail.com> skribis:
>
>> Currently, if (@ (guix build utils) wrap-program) is called multiple
>> times for the same file, the original file ends up being overwritten.
>
> OK, you’ve convinced me that this improvement is welcome.

An updated patch is attached.  I changed some of the wording in the
wrap-program docstring to bring it in a bit more in line with the new
behavior.  Let me know if there should be any more adjustments there.  I
also took the liberty of changing "/nix" to "/gnu".  ;)

> It would be ideal if a test in tests/build-utils.scm made sure that
> ‘wrap-program’ uses the right file names when called multiple times,
> but I won’t object if the patch doesn’t have it.

See the new test included in this patch.  Rather than checking for the
file outputs of wrap-program, it checks for correct behavior of the
wrapped program.  I believe this is more consistent with how
wrap-program is used, and doesn't tie the test to the implementation.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-utils-Allow-wrap-program-to-be-called-multiple-times.patch --]
[-- Type: text/x-diff, Size: 7271 bytes --]

From 1b09db0a80d94d3a4c798cc6ee811891b34153e1 Mon Sep 17 00:00:00 2001
From: Eric Bavier <bavier@member.fsf.org>
Date: Sat, 13 Sep 2014 01:05:03 -0500
Subject: [PATCH] utils: Allow wrap-program to be called multiple times.

* guix/build/utils.scm (wrap-program): Multiple invocations of
  wrap-program for the same file create successive wrappers.  Adjust
  docstring.
* tests/build-utils.scm: Test new wrap-program behavior.
  (%store): New variable.
---
 guix/build/utils.scm  |   44 ++++++++++++++++++++++++++----------
 tests/build-utils.scm |   60 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 13 deletions(-)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index d169053..7257b30 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -687,8 +687,7 @@ known as `nuke-refs' in Nixpkgs."
                              result))))))
 
 (define* (wrap-program prog #:rest vars)
-  "Rename PROG to .PROG-real and make PROG a wrapper.  VARS should look like
-this:
+  "Make a wrapper for PROG.  VARS should look like this:
 
   '(VARIABLE DELIMITER POSITION LIST-OF-DIRECTORIES)
 
@@ -697,23 +696,44 @@ where DELIMITER is optional.  ':' will be used if DELIMITER is not given.
 For example, this command:
 
   (wrap-program \"foo\"
-                '(\"PATH\" \":\" = (\"/nix/.../bar/bin\"))
-                '(\"CERT_PATH\" suffix (\"/nix/.../baz/certs\"
+                '(\"PATH\" \":\" = (\"/gnu/.../bar/bin\"))
+                '(\"CERT_PATH\" suffix (\"/gnu/.../baz/certs\"
                                         \"/qux/certs\")))
 
 will copy 'foo' to '.foo-real' and create the file 'foo' with the following
 contents:
 
   #!location/of/bin/bash
-  export PATH=\"/nix/.../bar/bin\"
-  export CERT_PATH=\"$CERT_PATH${CERT_PATH:+:}/nix/.../baz/certs:/qux/certs\"
+  export PATH=\"/gnu/.../bar/bin\"
+  export CERT_PATH=\"$CERT_PATH${CERT_PATH:+:}/gnu/.../baz/certs:/qux/certs\"
   exec location/of/.foo-real
 
 This is useful for scripts that expect particular programs to be in $PATH, for
 programs that expect particular shared libraries to be in $LD_LIBRARY_PATH, or
-modules in $GUILE_LOAD_PATH, etc."
-  (let ((prog-real (string-append (dirname prog) "/." (basename prog) "-real"))
-        (prog-tmp  (string-append (dirname prog) "/." (basename prog) "-tmp")))
+modules in $GUILE_LOAD_PATH, etc.
+
+If PROG has previously been wrapped by wrap-program the wrapper will point to
+the previous wrapper."
+  (define (wrapper-file-name number)
+    (format #f "~a/.~a-wrap-~2'0d" (dirname prog) (basename prog) number))
+  (define (next-wrapper-number)
+    (let ((wrappers
+           (find-files (dirname prog)
+                       (string-append "\\." (basename prog) "-wrap-.*"))))
+      (if (null? wrappers)
+          0
+          (string->number (string-take-right (last wrappers) 2)))))
+  (define (wrapper-target number)
+    (if (zero? number)
+        (let ((prog-real (string-append (dirname prog) "/."
+                                        (basename prog) "-real")))
+          (copy-file prog prog-real)
+          prog-real)
+        (wrapper-file-name number)))
+  (let* ((number   (next-wrapper-number))
+         (target   (wrapper-target number))
+         (wrapper  (wrapper-file-name (1+ number)))
+         (prog-tmp (string-append target "-tmp")))
     (define (export-variable lst)
       ;; Return a string that exports an environment variable.
       (match lst
@@ -736,8 +756,6 @@ modules in $GUILE_LOAD_PATH, etc."
          (format #f "export ~a=\"$~a${~a:+:}~a\""
                  var var var (string-join rest ":")))))
 
-    (copy-file prog prog-real)
-
     (with-output-to-file prog-tmp
       (lambda ()
         (format #t
@@ -745,9 +763,11 @@ modules in $GUILE_LOAD_PATH, etc."
                 (which "bash")
                 (string-join (map export-variable vars)
                              "\n")
-                (canonicalize-path prog-real))))
+                (canonicalize-path target))))
 
     (chmod prog-tmp #o755)
+    (rename-file prog-tmp wrapper)
+    (symlink wrapper prog-tmp)
     (rename-file prog-tmp prog)))
 
 ;;; Local Variables:
diff --git a/tests/build-utils.scm b/tests/build-utils.scm
index e94f04b..b82cd27 100644
--- a/tests/build-utils.scm
+++ b/tests/build-utils.scm
@@ -18,9 +18,27 @@
 
 
 (define-module (test-build-utils)
+  #:use-module (guix store)
+  #:use-module (guix derivations)
   #:use-module (guix build utils)
-  #:use-module (srfi srfi-64))
+  #:use-module (guix packages)
+  #:use-module (guix build-system)
+  #:use-module (guix build-system trivial)
+  #:use-module (gnu packages)
+  #:use-module (gnu packages bootstrap)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-64)
+  #:use-module (rnrs io ports)
+  #:use-module (ice-9 popen))
 
+(define %store
+  (false-if-exception (open-connection)))
+
+(when %store
+  ;; Make sure we build everything by ourselves.
+  (set-build-options %store #:use-substitutes? #f))
+
+\f
 (test-begin "build-utils")
 
 (test-equal "alist-cons-before"
@@ -80,6 +98,46 @@
                           port
                           cons)))))
 
+(test-assert "wrap-program, one input, multiple calls"
+  (let* ((p (package
+              (name "test-wrap-program") (version "0") (source #f)
+              (synopsis #f) (description #f) (license #f) (home-page #f)
+              (build-system trivial-build-system)
+              (arguments
+               `(#:guile ,%bootstrap-guile
+                 #:modules ((guix build utils))
+                 #:builder
+                 (let* ((out  (assoc-ref %outputs "out"))
+                        (bash (assoc-ref %build-inputs "bash"))
+                        (foo  (string-append out "/foo")))
+                   (begin
+                     (use-modules (guix build utils))
+                     (mkdir out)
+                     (call-with-output-file foo
+                       (lambda (p)
+                         (format p
+                                 "#!~a~%echo \"${GUIX_FOO} ${GUIX_BAR}\"~%"
+                                 bash)))
+                     (chmod foo #o777)
+                     ;; wrap-program uses `which' to find bash for the wrapper
+                     ;; shebang, but it can't know about the bootstrap bash in
+                     ;; the store, since it's not named "bash".  Help it out a
+                     ;; bit by providing a symlink it this package's output.
+                     (symlink bash (string-append out "/bash"))
+                     (setenv "PATH" out)
+                     (wrap-program foo `("GUIX_FOO" prefix ("hello")))
+                     (wrap-program foo `("GUIX_BAR" prefix ("world")))
+                     #t))))
+              (inputs `(("bash" ,(search-bootstrap-binary "bash"
+                                                          (%current-system)))))))
+         (d (package-derivation %store p)))
+    (and (build-derivations %store (pk 'drv d (list d)))
+         (let* ((p    (derivation->output-path d))
+                (foo  (string-append p "/foo"))
+                (pipe (open-input-pipe foo))
+                (str  (get-string-all pipe)))
+           (equal? str "hello world\n")))))
+
 (test-end)
 
 \f
-- 
1.7.9.5


[-- Attachment #3: Type: text/plain, Size: 133 bytes --]


-- 
Eric Bavier

Please avoid sending me Word or PowerPoint attachments.
See http://www.gnu.org/philosophy/no-word-attachments.html

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-13  6:12   ` Eric Bavier
@ 2014-09-13 12:20     ` Ludovic Courtès
  2014-09-14  4:05     ` mhw
  1 sibling, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2014-09-13 12:20 UTC (permalink / raw)
  To: Eric Bavier; +Cc: guix-devel

Eric Bavier <ericbavier@gmail.com> skribis:

> An updated patch is attached.  I changed some of the wording in the
> wrap-program docstring to bring it in a bit more in line with the new
> behavior.  Let me know if there should be any more adjustments there.  I
> also took the liberty of changing "/nix" to "/gnu".  ;)

Good.  :-)

>> It would be ideal if a test in tests/build-utils.scm made sure that
>> ‘wrap-program’ uses the right file names when called multiple times,
>> but I won’t object if the patch doesn’t have it.
>
> See the new test included in this patch.  Rather than checking for the
> file outputs of wrap-program, it checks for correct behavior of the
> wrapped program.  I believe this is more consistent with how
> wrap-program is used, and doesn't tie the test to the implementation.

Right, good idea.

> From 1b09db0a80d94d3a4c798cc6ee811891b34153e1 Mon Sep 17 00:00:00 2001
> From: Eric Bavier <bavier@member.fsf.org>
> Date: Sat, 13 Sep 2014 01:05:03 -0500
> Subject: [PATCH] utils: Allow wrap-program to be called multiple times.
>
> * guix/build/utils.scm (wrap-program): Multiple invocations of
>   wrap-program for the same file create successive wrappers.  Adjust
>   docstring.
> * tests/build-utils.scm: Test new wrap-program behavior.
>   (%store): New variable.

Looks good to me.  One last thing:

>  (define-module (test-build-utils)
> +  #:use-module (guix store)
> +  #:use-module (guix derivations)
>    #:use-module (guix build utils)
> -  #:use-module (srfi srfi-64))
> +  #:use-module (guix packages)
> +  #:use-module (guix build-system)
> +  #:use-module (guix build-system trivial)
> +  #:use-module (gnu packages)
> +  #:use-module (gnu packages bootstrap)
> +  #:use-module (srfi srfi-34)
> +  #:use-module (srfi srfi-64)
> +  #:use-module (rnrs io ports)
> +  #:use-module (ice-9 popen))
>  
> +(define %store
> +  (false-if-exception (open-connection)))
> +
> +(when %store
> +  ;; Make sure we build everything by ourselves.
> +  (set-build-options %store #:use-substitutes? #f))

These two forms can be replaced with:

  (define %store
    (open-connection-for-tests))

with the addition of #:use-module (guix tests).

OK to commit with this change.

Thank you!

Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-13  6:12   ` Eric Bavier
  2014-09-13 12:20     ` Ludovic Courtès
@ 2014-09-14  4:05     ` mhw
  2014-09-14 14:27       ` Ludovic Courtès
  1 sibling, 1 reply; 10+ messages in thread
From: mhw @ 2014-09-14  4:05 UTC (permalink / raw)
  To: Eric Bavier; +Cc: guix-devel

Eric Bavier <ericbavier@gmail.com> writes:

> diff --git a/guix/build/utils.scm b/guix/build/utils.scm
> index d169053..7257b30 100644
> --- a/guix/build/utils.scm
> +++ b/guix/build/utils.scm
> @@ -687,8 +687,7 @@ known as `nuke-refs' in Nixpkgs."
>                               result))))))
>  
>  (define* (wrap-program prog #:rest vars)

[... docstring changes ...]

> +  (define (wrapper-file-name number)
> +    (format #f "~a/.~a-wrap-~2'0d" (dirname prog) (basename prog) number))

When compiling core-updates, I now see the following warning:

--8<---------------cut here---------------start------------->8---
LC_ALL=C							\
./pre-inst-env					\
/home/mhw/.guix-profile/bin/guild compile -L "." -L "."	\
  -Wformat -Wunbound-variable -Warity-mismatch			\
  --target="i686-pc-linux-gnu"						\
  -o "guix/build/utils.go" "guix/build/utils.scm"
guix/build/utils.scm:718:4: warning: "~a/.~a-wrap-~2'0d": unsupported format option ~2, use (ice-9 format) instead
wrote `guix/build/utils.go'
--8<---------------cut here---------------end--------------->8---

Have you verified that this call to 'format' works as expected in all
the relevant cases?

      Thanks,
        Mark

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-14  4:05     ` mhw
@ 2014-09-14 14:27       ` Ludovic Courtès
  2014-09-14 15:16         ` Eric Bavier
  2014-09-14 15:58         ` Mark H Weaver
  0 siblings, 2 replies; 10+ messages in thread
From: Ludovic Courtès @ 2014-09-14 14:27 UTC (permalink / raw)
  To: mhw; +Cc: guix-devel

mhw@netris.org skribis:

> When compiling core-updates, I now see the following warning:
>
> LC_ALL=C							\
> ./pre-inst-env					\
> /home/mhw/.guix-profile/bin/guild compile -L "." -L "."	\
>   -Wformat -Wunbound-variable -Warity-mismatch			\
>   --target="i686-pc-linux-gnu"						\
>   -o "guix/build/utils.go" "guix/build/utils.scm"
> guix/build/utils.scm:718:4: warning: "~a/.~a-wrap-~2'0d": unsupported format option ~2, use (ice-9 format) instead
> wrote `guix/build/utils.go'

Oops, this needs to be fixed, indeed.

> Have you verified that this call to 'format' works as expected in all
> the relevant cases?

It could be that it works in practice, because (ice-9 format) does this
crazy thing:

  (module-set! the-root-module 'format format)

(-Wformat is purposefully stricter, because this crazy thing should
vanish eventually.)

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-14 14:27       ` Ludovic Courtès
@ 2014-09-14 15:16         ` Eric Bavier
  2014-09-14 15:58         ` Mark H Weaver
  1 sibling, 0 replies; 10+ messages in thread
From: Eric Bavier @ 2014-09-14 15:16 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel


Ludovic Courtès writes:

> mhw@netris.org skribis:
>
>> When compiling core-updates, I now see the following warning:
>>
>> LC_ALL=C							\
>> ./pre-inst-env					\
>> /home/mhw/.guix-profile/bin/guild compile -L "." -L "."	\
>>   -Wformat -Wunbound-variable -Warity-mismatch			\
>>   --target="i686-pc-linux-gnu"						\
>>   -o "guix/build/utils.go" "guix/build/utils.scm"
>> guix/build/utils.scm:718:4: warning: "~a/.~a-wrap-~2'0d": unsupported format option ~2, use (ice-9 format) instead
>> wrote `guix/build/utils.go'
>
> Oops, this needs to be fixed, indeed.

Sorry about that. :\

>> Have you verified that this call to 'format' works as expected in all
>> the relevant cases?
>
> It could be that it works in practice, because (ice-9 format) does this
> crazy thing:

From what I've seen, it works as expected.  But if adding (ice-9 format)
to the loaded modules doesn't cause any other issues, it might be a good
idea.

-- 
Eric Bavier

Please avoid sending me Word or PowerPoint attachments.
See http://www.gnu.org/philosophy/no-word-attachments.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] utils: Allow wrap-program to be called multiple times.
  2014-09-14 14:27       ` Ludovic Courtès
  2014-09-14 15:16         ` Eric Bavier
@ 2014-09-14 15:58         ` Mark H Weaver
  1 sibling, 0 replies; 10+ messages in thread
From: Mark H Weaver @ 2014-09-14 15:58 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

ludo@gnu.org (Ludovic Courtès) writes:

> mhw@netris.org skribis:
>
>> When compiling core-updates, I now see the following warning:
>>
>> LC_ALL=C							\
>> ./pre-inst-env					\
>> /home/mhw/.guix-profile/bin/guild compile -L "." -L "."	\
>>   -Wformat -Wunbound-variable -Warity-mismatch			\
>>   --target="i686-pc-linux-gnu"						\
>>   -o "guix/build/utils.go" "guix/build/utils.scm"
>> guix/build/utils.scm:718:4: warning: "~a/.~a-wrap-~2'0d": unsupported format option ~2, use (ice-9 format) instead
>> wrote `guix/build/utils.go'
>
> Oops, this needs to be fixed, indeed.

Okay, I pushed a fix to core-updates.

>> Have you verified that this call to 'format' works as expected in all
>> the relevant cases?
>
> It could be that it works in practice, because (ice-9 format) does this
> crazy thing:
>
>   (module-set! the-root-module 'format format)
>
> (-Wformat is purposefully stricter, because this crazy thing should
> vanish eventually.)

Indeed, maybe we should consider fixing that for 2.2.

    Thanks,
      Mark

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-09-14 15:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-09 22:56 [PATCH] utils: Allow wrap-program to be called multiple times Eric Bavier
2014-09-10 13:40 ` Ludovic Courtès
2014-09-10 19:16   ` Eric Bavier
2014-09-11 13:10 ` Ludovic Courtès
2014-09-13  6:12   ` Eric Bavier
2014-09-13 12:20     ` Ludovic Courtès
2014-09-14  4:05     ` mhw
2014-09-14 14:27       ` Ludovic Courtès
2014-09-14 15:16         ` Eric Bavier
2014-09-14 15:58         ` Mark H Weaver

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).