unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
From: Brendan Tildesley via Bug reports for GNU Guix <bug-guix@gnu.org>
To: "40039@debbugs.gnu.org" <40039@debbugs.gnu.org>
Cc: "Ricardo Wurmus" <rekado@elephly.net>, "Ludovic Courtès" <ludo@gnu.org>
Subject: bug#40039: 'wrap-script' introduces spurious argument
Date: Thu, 29 Apr 2021 17:23:23 +0200 (CEST)	[thread overview]
Message-ID: <964865602.98225.1619709804547@office.mailbox.org> (raw)
In-Reply-To: <87k13psl82.fsf@inria.fr>


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


  parent reply	other threads:[~2021-04-29 15:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2021-09-09 19:02 ` bug#40039: (No Subject) Attila Lendvai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=964865602.98225.1619709804547@office.mailbox.org \
    --to=bug-guix@gnu.org \
    --cc=40039@debbugs.gnu.org \
    --cc=btild@mailbox.org \
    --cc=ludo@gnu.org \
    --cc=rekado@elephly.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).