unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#40039: 'wrap-script' introduces spurious argument
@ 2020-03-12 14:26 Ludovic Courtès
  2020-03-22  0:53 ` Brendan Tildesley
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ludovic Courtès @ 2020-03-12 14:26 UTC (permalink / raw)
  To: bug-Guix

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

Hello,

I have a script that starts with:

--8<---------------cut here---------------start------------->8---
#!/gnu/store/29jhbbg1hf557x8j53f9sxd9imlmf02a-bash-minimal-5.0.7/bin/bash
# 
--8<---------------cut here---------------end--------------->8---

When I call ‘wrap-script’ on it, it leads to:

--8<---------------cut here---------------start------------->8---
#!/gnu/store/0awhym5h0m890n0wq87y0dxznh14rk88-guile-next-3.0.1/bin/guile --no-auto-compile
#!#; Guix wrapper
#\-(begin (setenv "PATH" "/gnu/store/9kzrrccpzl6i1sfwb0drb00gi2gwk0x0-coreutils-8.31/bin"))
#\-(let ((cl (command-line))) (apply execl "/gnu/store/29jhbbg1hf557x8j53f9sxd9imlmf02a-bash-minimal-5.0.7/bin/bash" (car cl) (cons (car cl) (append (quote ("")) cl))))
#!/gnu/store/29jhbbg1hf557x8j53f9sxd9imlmf02a-bash-minimal-5.0.7/bin/bash
# 
--8<---------------cut here---------------end--------------->8---

The expression (append '("") cl) is incorrect: the empty string
shouldn’t be added here.

I think one way to fix it is:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 786 bytes --]

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index b8be73ead4..f9698773c3 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -1295,7 +1295,8 @@ not supported."
                                              (car cl)
                                              (cons (car cl)
                                                    (append
-                                                    ',(string-split args #\space)
+                                                    ',(string-tokenize args
+                                                                       char-set:graphic)
                                                     cl))))))
                    (template (string-append prog ".XXXXXX"))
                    (out      (mkstemp! template))

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


Ludo’.

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

* bug#40039: 'wrap-script' introduces spurious argument
  2020-03-12 14:26 bug#40039: 'wrap-script' introduces spurious argument Ludovic Courtès
@ 2020-03-22  0:53 ` Brendan Tildesley
  2020-03-22 10:27   ` Ricardo Wurmus
  2021-04-29 15:23 ` Brendan Tildesley via Bug reports for GNU Guix
  2021-09-09 19:02 ` bug#40039: (No Subject) Attila Lendvai
  2 siblings, 1 reply; 8+ messages in thread
From: Brendan Tildesley @ 2020-03-22  0:53 UTC (permalink / raw)
  To: 40039

It appears the repeated (car cl) results in the executable path getting 
sent to it's self as the first argument. I'm not sure how things managed 
to work up until now? I tested the following change and it fixed the one 
case I was using, but am not sure it is correct. why was the cons (car 
cl) there in the first place?


diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index b8be73ead4..9610f39d71 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -946,7 +946,7 @@ FILE are kept unchanged."
                                                    "patch-shebang: ~a: 
changing `~a' to `~a'~%"
                                                    file (string-append 
interp " " arg1) bin)
                                            (patch p bin rest))
-                                      (begin
+                                      (begin
                                          (format (current-error-port)
                                                  "patch-shebang: ~a: 
changing `~a' to `~a'~%"
                                                  file interp bin)
@@ -1292,11 +1292,10 @@ not supported."
                                                         (_ vars))))
                                     `(let ((cl (command-line)))
                                        (apply execl ,interpreter
-                                             (car cl)
-                                             (cons (car cl)
-                                                   (append
- ',(string-split args #\space)
-                                                    cl))))))
+                                             (car cl) ;; The program.
+                                             (append
+                                              ',(string-tokenize args 
#\space)
+                                              cl)))))
                     (template (string-append prog ".XXXXXX"))
                     (out      (mkstemp! template))
                     (st       (stat prog))

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

* bug#40039: 'wrap-script' introduces spurious argument
  2020-03-22  0:53 ` Brendan Tildesley
@ 2020-03-22 10:27   ` Ricardo Wurmus
  2020-03-22 11:42     ` Brendan Tildesley
  2020-09-13  2:35     ` Brendan Tildesley
  0 siblings, 2 replies; 8+ messages in thread
From: Ricardo Wurmus @ 2020-03-22 10:27 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: 40039


Brendan Tildesley <brendan.tildesley@gmail.com> writes:

> It appears the repeated (car cl) results in the executable path
> getting sent to it's self as the first argument. I'm not sure how
> things managed to work up until now? I tested the following change and
> it fixed the one case I was using, but am not sure it is correct. why
> was the cons (car cl) there in the first place?

See the documentation of execl:

 -- Scheme Procedure: execl filename arg ...
 -- C Function: scm_execl (filename, args)
     Executes the file named by FILENAME as a new process image.  The
     remaining arguments are supplied to the process; from a C program
     they are accessible as the ‘argv’ argument to ‘main’.
     Conventionally the first ARG is the same as FILENAME.  All
     arguments must be strings.

     If ARG is missing, FILENAME is executed with a null argument list,
     which may have system-dependent side-effects.

     This procedure is currently implemented using the ‘execv’ system
     call, but we call it ‘execl’ because of its Scheme calling
     interface.

-- 
Ricardo

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

* bug#40039: 'wrap-script' introduces spurious argument
  2020-03-22 10:27   ` Ricardo Wurmus
@ 2020-03-22 11:42     ` Brendan Tildesley
  2020-09-13  2:35     ` Brendan Tildesley
  1 sibling, 0 replies; 8+ messages in thread
From: Brendan Tildesley @ 2020-03-22 11:42 UTC (permalink / raw)
  To: 40039, Ludovic Courtès

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

I'm currently packaging libratbag which provides the cli interface 
ratbagctl. when launched without arguments, it normally the current devices

######################
## with wrap-program (correct):
b@ui ~/guix [env]$ ratbagctl
warbling-mara:       Logitech G102 Prodigy Gaming Mouse

b@ui ~/guix [env]$ head `which ratbagctl`
#!/gnu/store/29jhbbg1hf557x8j53f9sxd9imlmf02a-bash-minimal-5.0.7/bin/bash
export 
PYTHONPATH="/gnu/store/88v8lvs02sdqgzv7w96g19fvh8hffzzp-libratbag-0.13/lib/python3.7/site-packages/:/gnu/store/h4jkr0qg86zjn1kq6iq8v33pcadj9r13-python-evdev-1.3.0/lib/python3.7/site-packages/:/gnu/store/z5fdc5aa9hs4c3p79ajzgxhazv7702y8-python-pygobject-3.28.3/lib/python3.7/site-packages/"
exec -a "$0" 
"/gnu/store/88v8lvs02sdqgzv7w96g19fvh8hffzzp-libratbag-0.13/bin/.ratbagctl-real" 
"$@"


######################
## with wrap-script:
b@ui ~/guix [env]$ ratbagctl
usage: /gnu/store/754ylqjs68va7rswr3fscwa0kyazsbjq-profile/bin/ratbagctl 
<device>
        {info,name,profile,resolution,dpi,rate,button,led} ...
/gnu/store/754ylqjs68va7rswr3fscwa0kyazsbjq-profile/bin/ratbagctl 
<device>: error: invalid choice: 
'/gnu/store/754ylqjs68va7rswr3fscwa0kyazsbjq-profile/bin/ratbagctl' 
(choose from 'info', 'name', 'profile', 'resolution', 'dpi', 'rate', 
'button', 'led')

b@ui ~/guix [env]$ ratbagctl list
/gnu/store/754ylqjs68va7rswr3fscwa0kyazsbjq-profile/bin/ratbagctl 
<device>: error: invalid choice: 
'/gnu/store/754ylqjs68va7rswr3fscwa0kyazsbjq-profile/bin/ratbagctl' 
(choose from 'info', 'name', 'profile', 'resolution', 'dpi', 'rate', 
'button', 'led')
b@ui ~/guix [env]$ ratbagctl info rastkasnts atkatkaf ftbaontb 
aesbtabtowf ~5qawylftarsnvbawlpfyq
usage: /gnu/store/754ylqjs68va7rswr3fscwa0kyazsbjq-profile/bin/ratbagctl 
<device>
        {info,name,profile,resolution,dpi,rate,button,led} ...
/gnu/store/754ylqjs68va7rswr3fscwa0kyazsbjq-profile/bin/ratbagctl 
<device>: error: invalid choice: 
'/gnu/store/754ylqjs68va7rswr3fscwa0kyazsbjq-profile/bin/ratbagctl' 
(choose from 'info', 'name', 'profile', 'resolution', 'dpi', 'rate', 
'button', 'led')
b@ui ~/guix [env]$

b@ui ~/guix [env]$ head `which ratbagctl`
#!/gnu/store/0awhym5h0m890n0wq87y0dxznh14rk88-guile-next-3.0.1/bin/guile 
--no-auto-compile
#!#; Guix wrapper
#\-(begin (setenv "PYTHONPATH" 
"/gnu/store/1hcpppqc6268siki64vs1ygq1qsr8w35-libratbag-0.13/lib/python3.7/site-packages/:/gnu/store/h4jkr0qg86zjn1kq6iq8v33pcadj9r13-python-evdev-1.3.0/lib/python3.7/site-packages/:/gnu/store/z5fdc5aa9hs4c3p79ajzgxhazv7702y8-python-pygobject-3.28.3/lib/python3.7/site-packages/"))
#\-(let ((cl (command-line))) (apply execl 
"/gnu/store/608bvypsh90c58apvd2cgg3m9l2pwjqn-python-3.7.4/bin/python3" 
(car cl) (cons (car cl) (append (quote ("")) cl))))
#!/gnu/store/608bvypsh90c58apvd2cgg3m9l2pwjqn-python-3.7.4/bin/python3

####################

Here I make a copy of the guix build utils module and modify the 
wrap-script, removing #\space as suggested, so it defaults to 
char-set:graphic. This results in the above guile wrapper chaging to:

#\-(let ((cl (command-line))) (apply execl 
"/gnu/store/608bvypsh90c58apvd2cgg3m9l2pwjqn-python-3.7.4/bin/python3" 
(car cl) (cons (car cl) (append (quote ()) cl))))

The behaviour changes somewhat. Now running ratbagctl prints the 
commants list, which is what should happen when the wrong arguments are 
provided, e.g., `ratbagctl qwfqwfqf`


b@ui ~/guix [env]$ ratbagctl
usage: ratbagctl [OPTIONS] list
        ratbagctl [OPTIONS] <device> {COMMAND} ...


b@ui ~/guix [env]$ ratbagctl list
usage: /gnu/store/fgl1w0lh1pzqg8j4gi8j1yi29aa122ja-profile/bin/ratbagctl 
<device>
        {info,name,profile,resolution,dpi,rate,button,led} ...
/gnu/store/fgl1w0lh1pzqg8j4gi8j1yi29aa122ja-profile/bin/ratbagctl 
<device>: error: invalid choice: 'list' (choose from 'info', 'name', 
'profile', 'resolution', 'dpi', 'rate', 'button', 'led')


























[-- Attachment #2: 0001-WRAPSCRIPT.patch --]
[-- Type: text/x-patch, Size: 14218 bytes --]

From 3b5db2c77598961b0b60c901a9bbed8f0524a93f Mon Sep 17 00:00:00 2001
From: Brendan Tildesley <mail@brendan.scot>
Date: Sun, 22 Mar 2020 22:40:18 +1100
Subject: [PATCH] WRAPSCRIPT

---
 gnu/packages/gnome.scm | 131 ++++++++++++++++++++++++++++++++-
 my-wrap.scm            | 162 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 292 insertions(+), 1 deletion(-)
 create mode 100644 my-wrap.scm

diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index a08cd00d72..d614677214 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -27,7 +27,7 @@
 ;;; Copyright © 2017, 2018 nee <nee-git@hidamari.blue>
 ;;; Copyright © 2017 Chris Marusich <cmmarusich@gmail.com>
 ;;; Copyright © 2017 Mohammed Sadiq <sadiq@sadiqpk.org>
-;;; Copyright © 2017 Brendan Tildesley <mail@brendan.scot>
+;;; Copyright © 2017, 2020 Brendan Tildesley <mail@brendan.scot>
 ;;; Copyright © 2017, 2018 Rutger Helling <rhelling@mykolab.com>
 ;;; Copyright © 2018 Jovany Leandro G.C <bit4bit@riseup.net>
 ;;; Copyright © 2018 Vasile Dumitrascu <va511e@yahoo.com>
@@ -158,12 +158,14 @@
   #:use-module (gnu packages spice)
   #:use-module (gnu packages sqlite)
   #:use-module (gnu packages ssh)
+  #:use-module (gnu packages swig)
   #:use-module (gnu packages tex)
   #:use-module (gnu packages time)
   #:use-module (gnu packages tls)
   #:use-module (gnu packages version-control)
   #:use-module (gnu packages video)
   #:use-module (gnu packages virtualization)
+  #:use-module (gnu packages valgrind)
   #:use-module (gnu packages vpn)
   #:use-module (gnu packages web)
   #:use-module (gnu packages webkit)
@@ -186,6 +188,10 @@
   #:use-module (guix utils)
   #:use-module (guix gexp)
   #:use-module (guix monads)
+  #:use-module (guix)
+
+  #:use-module (my-wrap)
+
   #:use-module (guix store)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1))
@@ -9956,3 +9962,126 @@ manage remote and virtual systems.")
               license:cc-by2.0
               ;; For all others.
               license:lgpl2.0+))))
+
+
+(define-public python-evdev
+  (package
+    (name "python-evdev")
+    (version "1.3.0")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (pypi-uri "evdev" version))
+       (sha256
+        (base32
+         "0kb3636yaw9l8xi8s184w0r0n9ic5dw3b8hx048jf9fpzss4kimi"))))
+    (build-system python-build-system)
+    (native-inputs
+     `(("kernel-headers" ,linux-libre-headers)))
+    (arguments
+     `(#:tests? #f
+       #:phases
+       (modify-phases %standard-phases
+         (add-before 'build 'patch-patch
+           (lambda* (#:key inputs #:allow-other-keys)
+             (substitute* "setup.py"
+               (("/usr/include/linux")
+                (string-append
+                 (assoc-ref inputs "kernel-headers") "/include/linux")))
+             #t)))))
+    (home-page
+     "https://github.com/gvalkov/python-evdev")
+    (synopsis
+     "Bindings to the Linux input handling subsystem")
+    (description
+     "Bindings to the Linux input handling subsystem")
+    (license license:lgpl2.1)))
+
+(define-public libratbag
+  (package
+    (name "libratbag")
+    (version "0.13")
+    (source (origin
+              (method url-fetch)
+              (uri
+               (string-append
+                "https://github.com/libratbag/libratbag/archive/v" version ".tar.gz"))
+              (file-name (string-append name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                "1j8ryzrljqcp0c1wqzzpgr5fqdmwqr5z99avkxwfs7kqwm8ii9xh"))))
+    (build-system meson-build-system)
+    (native-inputs `(("pkg-config" ,pkg-config)
+                     ("check" ,check)
+                     ("swing" ,swig)
+                     ("valgrind" ,valgrind)))
+    (inputs `(("guile" ,guile-3.0)   ;;; for wrap-script
+              ("glib" ,glib)
+              ("json-glib" ,json-glib)
+              ("libevdev" ,libevdev)
+              ("udev" ,eudev)
+                                        ;("gobject-introspection" ,gobject-introspection)
+              ("python-pygobject" ,python-pygobject)
+              ("python-evdev" ,python-evdev)
+              ("libsystemd" ,elogind)
+
+              ))
+    (arguments
+     `(#:imported-modules ((my-wrap)
+                           (guix build meson-build-system)
+                           (guix build gnu-build-system)
+                           (guix build utils)
+                           (guix build glib-or-gtk-build-system)
+                           (guix build gremlin)
+                           (guix elf))
+       ;;#:modules ((wrap))
+       #:configure-flags
+       (list "-Dsystemd=false"
+             "-Dlogind-provider=elogind")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'install 'wrap
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+
+             (use-modules (my-wrap))
+
+
+             (let*
+                 ((out (assoc-ref outputs "out" ))
+                  (site "/lib/python3.7/site-packages/")
+                  (out-site (string-append (assoc-ref outputs "out" ) site))
+                  (evdev (string-append (assoc-ref inputs "python-evdev") site))
+                  (pygo (string-append (assoc-ref inputs "python-pygobject") site))
+
+                  (python-wrap
+                   `("PYTHONPATH" = (,out-site ,evdev ,pygo)))
+
+                  (gi-wrap ;; wraps json-glibs girepository directory. doesnt seem to matter at all??
+                   `("GI_TYPELIB_PATH" = (,(getenv "GI_TYPELIB_PATH")))))
+
+               ;; Do we even need to wrap the daemon?
+               ;; (wrap-program (string-append out "/bin/" "ratbagd")
+               ;;   python-wrap ;; gi-wrap
+               ;;   )
+               ;; TODO: switch to wrap-script when it's fixed
+               (wrap-script** (string-append out "/bin/" "ratbagctl")
+                              python-wrap ;; gi-wrap
+                              )
+
+               #t))))))
+    (home-page "https://github.com/libratbag/libratbag")
+    (synopsis "DBus daemon for configuring gaming mice")
+    (description "libratbag provides ratbagd, a DBus daemon to configure input
+devices, mainly gaming mice.  The daemon provides a generic way to access the
+various features exposed by these mice and abstracts away hardware-specific
+and kernel-specific quirks. There is also the ratbagctl command line interface
+for configuring devices.
+
+libratbag currently supports devices from Logitech, Etekcity, GSkill, Roccat,
+Steelseries.
+
+ratbagd can be enabled by adding the following service to your
+operating-system definition:
+(simple-service 'ratbagd dbus-root-service-type (list libratbag))
+")
+    (license license:expat)))
diff --git a/my-wrap.scm b/my-wrap.scm
new file mode 100644
index 0000000000..f47ea210f6
--- /dev/null
+++ b/my-wrap.scm
@@ -0,0 +1,162 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013 Andreas Enge <andreas@enge.fr>
+;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2015, 2018 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
+;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado@elephly.net>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (my-wrap)
+  #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
+  #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-60)
+  #:use-module (ice-9 ftw)
+  #:use-module (ice-9 match)
+  #:use-module (ice-9 regex)
+  #:use-module (ice-9 rdelim)
+  #:use-module (ice-9 format)
+  #:use-module (ice-9 threads)
+  #:use-module (rnrs bytevectors)
+  #:use-module (rnrs io ports)
+  #:use-module (guix build utils)
+  #:export ( wrap-script**))
+
+\f
+;;;
+;;; Guile 2.0 compatibility later.
+;;;
+
+;; The bootstrap Guile is Guile 2.0, so provide a compatibility layer.
+
+(define wrap-script**
+  (let ((interpreter-regex
+         (make-regexp
+          (string-append "^#! ?(/[^ ]+/bin/("
+                         (string-join '("python[^ ]*"
+                                        "Rscript"
+                                        "perl"
+                                        "ruby"
+                                        "bash"
+                                        "sh") "|")
+                         "))( ?.*)")))
+        (coding-line-regex
+         (make-regexp
+          ".*#.*coding[=:][[:space:]]*([-a-zA-Z_0-9.]+)")))
+    (lambda* (prog #:key (guile (which "guile")) #:rest vars)
+      "Wrap the script PROG such that VARS are set first.  The format of VARS
+is the same as in the WRAP-PROGRAM procedure.  This procedure differs from
+WRAP-PROGRAM in that it does not create a separate shell script.  Instead,
+PROG is modified directly by prepending a Guile script, which is interpreted
+as a comment in the script's language.
+
+Special encoding comments as supported by Python are recreated on the second
+line.
+
+Note that this procedure can only be used once per file as Guile scripts are
+not supported."
+      (define update-env
+        (match-lambda
+          ((var sep '= rest)
+           `(setenv ,var ,(string-join rest sep)))
+          ((var sep 'prefix rest)
+           `(let ((current (getenv ,var)))
+              (setenv ,var (if current
+                               (string-append ,(string-join rest sep)
+                                              ,sep current)
+                               ,(string-join rest sep)))))
+          ((var sep 'suffix rest)
+           `(let ((current (getenv ,var)))
+              (setenv ,var (if current
+                               (string-append current ,sep
+                                              ,(string-join rest sep))
+                               ,(string-join rest sep)))))
+          ((var '= rest)
+           `(setenv ,var ,(string-join rest ":")))
+          ((var 'prefix rest)
+           `(let ((current (getenv ,var)))
+              (setenv ,var (if current
+                               (string-append ,(string-join rest ":")
+                                              ":" current)
+                               ,(string-join rest ":")))))
+          ((var 'suffix rest)
+           `(let ((current (getenv ,var)))
+              (setenv ,var (if current
+                               (string-append current ":"
+                                              ,(string-join rest ":"))
+                               ,(string-join rest ":")))))))
+      (let-values (((interpreter args coding-line)
+                    (call-with-ascii-input-file prog
+                      (lambda (p)
+                        (let ((first-match
+                               (false-if-exception
+                                (regexp-exec interpreter-regex (read-line p)))))
+                          (values (and first-match (match:substring first-match 1))
+                                  (and first-match (match:substring first-match 3))
+                                  (false-if-exception
+                                   (and=> (regexp-exec coding-line-regex (read-line p))
+                                          (lambda (m) (match:substring m 0))))))))))
+        (if interpreter
+            (let* ((header (format #f "\
+#!~a --no-auto-compile
+#!#; ~a
+#\\-~s
+#\\-~s
+"
+                                   guile
+                                   (or coding-line "Guix wrapper")
+                                   (cons 'begin (map update-env
+                                                     (match vars
+                                                       ((#:guile _ . vars) vars)
+                                                       (_ vars))))
+                                   `(let ((cl (command-line)))
+                                      (apply execl ,interpreter
+                                             (car cl)
+                                             (cons (car cl)
+                                                   (append
+                                                    ',(string-tokenize args)
+                                                    cl))))))
+                   (template (string-append prog ".XXXXXX"))
+                   (out      (mkstemp! template))
+                   (st       (stat prog))
+                   (mode     (stat:mode st)))
+              (with-throw-handler #t
+                (lambda ()
+                  (call-with-ascii-input-file prog
+                    (lambda (p)
+                      (format out header)
+                      (dump-port p out)
+                      (close out)
+                      (chmod template mode)
+                      (rename-file template prog)
+                      (set-file-time prog st))))
+                (lambda (key . args)
+                  (format (current-error-port)
+                          "wrap-script: ~a: error: ~a ~s~%"
+                          prog key args)
+                  (false-if-exception (delete-file template))
+                  (raise (condition
+                          (&wrap-error (program prog)
+                                       (type key))))
+                  #f)))
+            (raise (condition
+                    (&wrap-error (program prog)
+                                 (type 'no-interpreter-found)))))))))
+\f
-- 
2.25.1


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

* bug#40039: 'wrap-script' introduces spurious argument
  2020-03-22 10:27   ` Ricardo Wurmus
  2020-03-22 11:42     ` Brendan Tildesley
@ 2020-09-13  2:35     ` Brendan Tildesley
  2020-09-13 12:23       ` Ricardo Wurmus
  1 sibling, 1 reply; 8+ messages in thread
From: Brendan Tildesley @ 2020-09-13  2:35 UTC (permalink / raw)
  To: Ricardo Wurmus, Ludovic Courtès; +Cc: 40039

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

Hi Ricardo, Ludovic... I was wondering if we could revisit and fix this. 
I believe there is more than 1 bug here.

Suppose we have the script test.sh

    #!/run/current-system/profile/bin/bash
    echo "$@"

./test.sh returns:

hi

Wrapping with wrap-script returns

  ./test.sh hi

(notice the extract space at the start.)

With Ludovic's change to char-set:graphicY

.test.sh hi

The leading space is gone at least. Finally, after removing one of the 
(car cl)'s, if finally just returns hi.


[-- Attachment #2: Type: text/html, Size: 790 bytes --]

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

* bug#40039: 'wrap-script' introduces spurious argument
  2020-09-13  2:35     ` Brendan Tildesley
@ 2020-09-13 12:23       ` Ricardo Wurmus
  0 siblings, 0 replies; 8+ messages in thread
From: Ricardo Wurmus @ 2020-09-13 12:23 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: 40039


Brendan Tildesley <brendan.tildesley@gmail.com> writes:

> Hi Ricardo, Ludovic... I was wondering if we could revisit and fix
> this.

Yes, let’s try to fix this.  I think it would be good to have a bunch of
automated tests that we can work with to validate the feature even in
somewhat obscure circumstances.

It’s been a while since I originally wrote the code, so some decisions
are no longer obvious to me, but I’ll try to familiarize myself with it
once again.

-- 
Ricardo




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

* bug#40039: 'wrap-script' introduces spurious argument
  2020-03-12 14:26 bug#40039: 'wrap-script' introduces spurious argument Ludovic Courtès
  2020-03-22  0:53 ` Brendan Tildesley
@ 2021-04-29 15:23 ` Brendan Tildesley via Bug reports for GNU Guix
  2021-09-09 19:02 ` bug#40039: (No Subject) Attila Lendvai
  2 siblings, 0 replies; 8+ messages in thread
From: Brendan Tildesley via Bug reports for GNU Guix @ 2021-04-29 15:23 UTC (permalink / raw)
  To: 40039@debbugs.gnu.org; +Cc: Ricardo Wurmus, Ludovic Courtès


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

Same patch as before, but with a test case. Have a play with it and see if it makes sense.

[-- Attachment #1.2: Type: text/html, Size: 241 bytes --]

[-- Attachment #2: 0001-utils-Fix-wrap-script-argument-handling.patch --]
[-- Type: text/x-patch, Size: 5484 bytes --]

From 21d5f4e01be7f9b86649f4176f53e14854b58d53 Mon Sep 17 00:00:00 2001
From: Brendan Tildesley <mail@brendan.scot>
Date: Thu, 29 Apr 2021 20:33:08 +1000
Subject: [PATCH] utils: Fix wrap-script argument handling.

* guix/build/utils.scm (wrap-script):
Don't add (car cl) one too many times, cl its self contains it's car.
Split the aguments string with string-tokenize to avoid leaving an empty
string argument when there should be none. These two bugs seemed to
be partially cancelling each other out so that scripts still worked when
ran with no arguments.

* tests/build-utils.scm: Adjust wrap-script to above changes.
Add two tests to ensure the command line arguments appear identical to a
script and its wrapped version.
---
 guix/build/utils.scm  |  8 +++----
 tests/build-utils.scm | 55 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 419c10195b..cc2a020fbf 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2015, 2018 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Brendan Tildesley <mail@brendan.scot>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1295,10 +1296,9 @@ not supported."
                                    `(let ((cl (command-line)))
                                       (apply execl ,interpreter
                                              (car cl)
-                                             (cons (car cl)
-                                                   (append
-                                                    ',(string-split args #\space)
-                                                    cl))))))
+                                             (append
+                                              ',(string-tokenize args char-set:graphic)
+                                              cl)))))
                    (template (string-append prog ".XXXXXX"))
                    (out      (mkstemp! template))
                    (st       (stat prog))
diff --git a/tests/build-utils.scm b/tests/build-utils.scm
index 654b480ed9..f3f31deaf6 100644
--- a/tests/build-utils.scm
+++ b/tests/build-utils.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2015, 2016, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Brendan Tildesley <mail@brendan.scot>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -165,9 +166,7 @@ echo hello world"))
                                    "/some/path:/some/other/path"))))
              '(let ((cl (command-line)))
                 (apply execl "/anything/cabbage-bash-1.2.3/bin/sh"
-                       (car cl)
-                       (cons (car cl)
-                             (append '("") cl)))))
+                       (car cl) cl)))
      script-contents)
     (call-with-temporary-directory
      (lambda (directory)
@@ -206,8 +205,7 @@ print('hello world')"))
              `(let ((cl (command-line)))
                 (apply execl "/anything/cabbage-bash-1.2.3/bin/python3"
                        (car cl)
-                       (cons (car cl)
-                             (append '("" "-and" "-args") cl)))))
+                       (append '("-and" "-args") cl))))
      script-contents)
     (call-with-temporary-directory
      (lambda (directory)
@@ -241,4 +239,51 @@ print('hello world')"))
                                            "/some/other/path")))
          #f)))))
 
+(define (arg-test bash-args)
+  (call-with-temporary-directory
+   (lambda (directory)
+     (let ((script-file-name (string-append directory "/bash-test.sh")))
+       (call-with-output-file script-file-name
+         (lambda (port)
+           (display (string-append "\
+#!" (which "bash") bash-args "
+echo \"$#$0$*${A}\"")
+                    port)))
+
+       (display "Unwrapped script contents:\n")
+       (call-with-input-file script-file-name
+         (lambda (port) (display (get-string-all port))))
+       (newline) (newline)
+       (chmod script-file-name #o777)
+       (setenv "A" "A")
+       (let* ((run-script (lambda _
+                            (open-pipe*
+                             OPEN_READ
+                             script-file-name "1" "2" "3 3" "4")))
+              (pipe (run-script))
+              (unwrapped-output (get-string-all pipe)))
+         (close-pipe pipe)
+
+         (wrap-script script-file-name `("A" = ("A\nA")))
+
+         (display "Wrapped script contents:\n")
+         (call-with-input-file script-file-name
+           (lambda (port) (display (get-string-all port))))
+         (newline) (newline)
+
+         (let* ((pipe (run-script))
+                (wrapped-output (get-string-all pipe)))
+           (close-pipe pipe)
+           (display "./bash-test.sh 1 2 3\\ 3 4 # Output:\n")
+           (display unwrapped-output) (newline)
+           (display "./bash-test.sh 1 2 3\\ 3 4 # Output (wrapped):\n")
+           (display wrapped-output) (newline)
+           (string=? (string-append unwrapped-output "A\n") wrapped-output)))))))
+
+(test-assert "wrap-script, argument handling"
+  (arg-test ""))
+
+(test-assert "wrap-script, argument handling, bash --norc"
+  (arg-test " --norc"))
+
 (test-end)
-- 
2.31.1


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

* bug#40039: (No Subject)
  2020-03-12 14:26 bug#40039: 'wrap-script' introduces spurious argument Ludovic Courtès
  2020-03-22  0:53 ` Brendan Tildesley
  2021-04-29 15:23 ` Brendan Tildesley via Bug reports for GNU Guix
@ 2021-09-09 19:02 ` Attila Lendvai
  2 siblings, 0 replies; 8+ messages in thread
From: Attila Lendvai @ 2021-09-09 19:02 UTC (permalink / raw)
  To: 40039@debbugs.gnu.org

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

any ETA on this fix? i think i need it.

i tried to add a `wrap-script-or-program` to `python-build-system`: attempt to use `wrap-scipt`, and in case of a `no-interpreter-found` error fall back to `wrap-program`. (motivated by trezor-agent relying on sys.argv[0], which is ruined by `wrap-program`)

the result of my change was that building `serf` dies, and i suspect that because of this bug.

the output:

command "scons" "-j" "8" "APR=/gnu/store/gxq63qb00yn11vv875n7l9fffs2gmgxp-apr-1.6.5" "APU=/gnu/store/gl2wzkld26ry7xainbbbwgrz493925xn-apr-util-1.6.1" "OPENSSL=/gnu/store/ixm51m1jcfw4rhvwnd690szfv2w3pcsm-openssl-1.1.1j" "ZLIB=/gnu/store/rykm237xkmq7rl1p0nwass01p090p88x-zlib-1.2.11" "PREFIX=/gnu/store/pgs8b7410lap9ax68wbq2j5kyhhb3kwf-serf-1.3.9" failed with status 127

note that after my change `scons` is wrapped as a script, not as a program.

i tried to apply this fix, and retry, but it seems to cause the rebuilding of the entire world -- and i'm on a laptop... :)

so, i think i'll just wait until this fix reaches main, and i'll retry after that.

- attila
PGP: 5D5F 45C7 DFCD 0A39

[-- Attachment #2: Type: text/html, Size: 1479 bytes --]

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

end of thread, other threads:[~2021-09-09 19:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-12 14:26 bug#40039: 'wrap-script' introduces spurious argument Ludovic Courtès
2020-03-22  0:53 ` Brendan Tildesley
2020-03-22 10:27   ` Ricardo Wurmus
2020-03-22 11:42     ` Brendan Tildesley
2020-09-13  2:35     ` Brendan Tildesley
2020-09-13 12:23       ` Ricardo Wurmus
2021-04-29 15:23 ` Brendan Tildesley via Bug reports for GNU Guix
2021-09-09 19:02 ` bug#40039: (No Subject) Attila Lendvai

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