Opened 4 years ago

Last modified 9 months ago

#1685 assigned defect

shell-variable in egg-compile should quote environment variables

Reported by: Mario Domenech Goulart Owned by: evhan
Priority: major Milestone: someday
Component: core libraries Version: 5.2.0
Keywords: shell-variable, DESTDIR, chicken-install, egg-compile Cc:
Estimated difficulty:


Without quoting shell variables, install scripts end up with commands like

mkdir -p ${DESTDIR}'/home/mario/local/chicken-head/lib/chicken/11'

If DESTDIR} happens to contain a space, the command above will not do what it is expected to do.

Double-quoting shell variables on Unix systems is not harmful, as far as I can tell. I have no clue about Windows.

At the moment, shell-variable seems to be only applied to DESTDIR.

The following patch would improve the situation on Unix:

diff --git a/egg-compile.scm b/egg-compile.scm
index 4a72d5d0..e6be5d1d 100644
--- a/egg-compile.scm
+++ b/egg-compile.scm
@@ -1227,7 +1227,7 @@ EOF

 (define (shell-variable var platform)
   (case platform
-    ((unix) (string-append "${" var "}"))
+    ((unix) (string-append "\"${" var "}\""))
     ((windows) (string-append "%" var "%"))))

 ;; NOTE `cmd' must already be quoted for shell

Change History (6)

comment:1 Changed 4 years ago by Vasilij Schneidermann

You'll need to use double quotes on Windows as well, see for the finer details.

comment:2 Changed 3 years ago by evhan

Owner: set to evhan
Status: newassigned

This looks easy enough.

comment:3 Changed 3 years ago by Vasilij Schneidermann

Regarding the double-quoting problem, yes, it's an issue on Windows, see for my findings on the topic. A variable must be set correctly and assumed that it doesn't contain any unintended quotes.

comment:4 Changed 3 years ago by Mario Domenech Goulart

Fix for unix has been applied (f9a6dd4472bf137bdebd1613e302f1638305b1b9). We still have the issue on Windows.

comment:5 Changed 3 years ago by Mario Domenech Goulart

Milestone: 5.35.4

comment:6 Changed 9 months ago by felix winkelmann

Milestone: 5.4someday
Note: See TracTickets for help on using tickets.