Skip to content

Commit 9c93ac3

Browse files
basbossinkisaacs
basbossink
authored and
isaacs
committed
fix: Handle environment variables properly
- Adapt the regular expression used to match the first line. - Create a simple module that can convert shell style variable declarations: NODE_PATH=./lib:$NODE_PATH to the equivalent batch syntax: @set=NODE_PATH=./lib:%NODE_PATH%. Furthermore the structure of the generated shim now looks like this: @SETLOCAL <variable declarations> <original content> @endlocal - Note that the new segments are only added to the file if there were any variable declarations. - The generated shell script carries over the captured variable declaration as is. - Add some extra tests to validate the behavior. - Remove some unnecessary white-space in the generated shim. PR-URL: #2 Fix: npm/npm#3380 Credit: @basbossink Close: #2 Reviewed-by: @isaacs
1 parent 3bf6d4a commit 9c93ac3

File tree

5 files changed

+146
-17
lines changed

5 files changed

+146
-17
lines changed

‎index.js

+19-9
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ var fs = require("graceful-fs")
1515

1616
var mkdir = require("mkdirp")
1717
, path = require("path")
18-
, shebangExpr = /^#\!\s*(?:\/usr\/bin\/env)?\s*([^ \t]+)(.*)$/
18+
, toBatchSyntax = require("./lib/to-batch-syntax")
19+
, shebangExpr = /^#\!\s*(?:\/usr\/bin\/env)?\s*([^ \t]+=[^ \t]+\s+)*\s*([^ \t]+)(.*)$/
1920

2021
function cmdShimIfExists (from, to, cb) {
2122
fs.stat(from, function (er) {
@@ -63,22 +64,25 @@ function writeShim (from, to, cb) {
6364
if (er) return writeShim_(from, to, null, null, cb)
6465
var firstLine = data.trim().split(/\r*\n/)[0]
6566
, shebang = firstLine.match(shebangExpr)
66-
if (!shebang) return writeShim_(from, to, null, null, cb)
67-
var prog = shebang[1]
68-
, args = shebang[2] || ""
69-
return writeShim_(from, to, prog, args, cb)
67+
if (!shebang) return writeShim_(from, to, null, null, null, cb)
68+
var vars = shebang[1] || ""
69+
, prog = shebang[2]
70+
, args = shebang[3] || ""
71+
return writeShim_(from, to, prog, args, vars, cb)
7072
})
7173
})
7274
}
7375

74-
function writeShim_ (from, to, prog, args, cb) {
76+
77+
function writeShim_ (from, to, prog, args, variables, cb) {
7578
var shTarget = path.relative(path.dirname(to), from)
7679
, target = shTarget.split("/").join("\\")
7780
, longProg
7881
, shProg = prog && prog.split("\\").join("/")
7982
, shLongProg
8083
shTarget = shTarget.split("\\").join("/")
8184
args = args || ""
85+
variables = variables || ""
8286
if (!prog) {
8387
prog = "\"%~dp0\\" + target + "\""
8488
shProg = "\"$basedir/" + shTarget + "\""
@@ -101,13 +105,19 @@ function writeShim_ (from, to, prog, args, cb) {
101105
// )
102106
var cmd
103107
if (longProg) {
104-
cmd = "@IF EXIST " + longProg + " (\r\n"
108+
shLongProg = shLongProg.trim();
109+
args = args.trim();
110+
var variableDeclarationsAsBatch = toBatchSyntax.convertToSetCommands(variables) || "";
111+
cmd = ((variableDeclarationsAsBatch.length > 0) ? ("@SETLOCAL\r\n"
112+
+ variableDeclarationsAsBatch) : "")
113+
+ "@IF EXIST " + longProg + " (\r\n"
105114
+ " " + longProg + " " + args + " " + target + " %*\r\n"
106115
+ ") ELSE (\r\n"
107116
+ " @SETLOCAL\r\n"
108117
+ " @SET PATHEXT=%PATHEXT:;.JS;=;%\r\n"
109118
+ " " + prog + " " + args + " " + target + " %*\r\n"
110119
+ ")"
120+
+ ((variableDeclarationsAsBatch.length > 0) ? "\r\n@ENDLOCAL" : "")
111121
} else {
112122
cmd = "@" + prog + " " + args + " " + target + " %*\r\n"
113123
}
@@ -141,10 +151,10 @@ function writeShim_ (from, to, prog, args, cb) {
141151

142152
sh = sh
143153
+ "if [ -x "+shLongProg+" ]; then\n"
144-
+ " " + shLongProg + " " + args + " " + shTarget + " \"$@\"\n"
154+
+ " " + variables + shLongProg + " " + args + " " + shTarget + " \"$@\"\n"
145155
+ " ret=$?\n"
146156
+ "else \n"
147-
+ " " + shProg + " " + args + " " + shTarget + " \"$@\"\n"
157+
+ " " + variables + shProg + " " + args + " " + shTarget + " \"$@\"\n"
148158
+ " ret=$?\n"
149159
+ "fi\n"
150160
+ "exit $ret\n"

‎lib/to-batch-syntax.js

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
exports.replaceDollarWithPercentPair = replaceDollarWithPercentPair
2+
exports.convertToSetCommand = convertToSetCommand
3+
exports.convertToSetCommands = convertToSetCommands
4+
5+
function convertToSetCommand(key, value) {
6+
var line = ""
7+
key = key || ""
8+
key = key.trim()
9+
value = value || ""
10+
value = value.trim()
11+
if(key && value && value.length > 0) {
12+
line = "@SET " + key + "=" + replaceDollarWithPercentPair(value) + "\r\n"
13+
}
14+
return line
15+
}
16+
17+
function extractVariableValuePairs(declarations) {
18+
var pairs = {}
19+
declarations.map(function(declaration) {
20+
var split = declaration.split("=")
21+
pairs[split[0]]=split[1]
22+
})
23+
return pairs
24+
}
25+
26+
function convertToSetCommands(variableString) {
27+
var variableValuePairs = extractVariableValuePairs(variableString.split(" "))
28+
var variableDeclarationsAsBatch = ""
29+
Object.keys(variableValuePairs).forEach(function (key) {
30+
variableDeclarationsAsBatch += convertToSetCommand(key, variableValuePairs[key])
31+
})
32+
return variableDeclarationsAsBatch
33+
}
34+
35+
function replaceDollarWithPercentPair(value) {
36+
var dollarExpressions = /\$\{?([^\$@#\?\- \t{}:]+)\}?/g
37+
var result = ""
38+
var startIndex = 0
39+
value = value || ""
40+
do {
41+
var match = dollarExpressions.exec(value)
42+
if(match) {
43+
var betweenMatches = value.substring(startIndex, match.index) || ""
44+
result += betweenMatches + "%" + match[1] + "%"
45+
startIndex = dollarExpressions.lastIndex
46+
}
47+
} while (dollarExpressions.lastIndex > 0)
48+
result += value.substr(startIndex)
49+
return result
50+
}
51+
52+

‎test/00-setup.js

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ var froms = {
88
'from.exe': 'exe',
99
'from.env': '#!/usr/bin/env node\nconsole.log(/hi/)\n',
1010
'from.env.args': '#!/usr/bin/env node --expose_gc\ngc()\n',
11+
'from.env.variables': '#!/usr/bin/env NODE_PATH=./lib:$NODE_PATH node',
1112
'from.sh': '#!/usr/bin/sh\necho hi\n',
1213
'from.sh.args': '#!/usr/bin/sh -x\necho hi\n'
1314
}

‎test/basic.js

+49-8
Original file line numberDiff line numberDiff line change
@@ -76,26 +76,67 @@ test('env shebang with args', function (t) {
7676
"\nesac"+
7777
"\n"+
7878
"\nif [ -x \"$basedir/node\" ]; then"+
79-
"\n \"$basedir/node\" --expose_gc \"$basedir/from.env.args\" \"$@\""+
79+
"\n \"$basedir/node\" --expose_gc \"$basedir/from.env.args\" \"$@\""+
8080
"\n ret=$?"+
8181
"\nelse "+
82-
"\n node --expose_gc \"$basedir/from.env.args\" \"$@\""+
82+
"\n node --expose_gc \"$basedir/from.env.args\" \"$@\""+
8383
"\n ret=$?"+
8484
"\nfi"+
8585
"\nexit $ret"+
8686
"\n")
8787
t.equal(fs.readFileSync(to + '.cmd', 'utf8'),
8888
"@IF EXIST \"%~dp0\\node.exe\" (\r"+
89-
"\n \"%~dp0\\node.exe\" --expose_gc \"%~dp0\\from.env.args\" %*\r"+
89+
"\n \"%~dp0\\node.exe\" --expose_gc \"%~dp0\\from.env.args\" %*\r"+
9090
"\n) ELSE (\r"+
9191
"\n @SETLOCAL\r"+
9292
"\n @SET PATHEXT=%PATHEXT:;.JS;=;%\r"+
93-
"\n node --expose_gc \"%~dp0\\from.env.args\" %*\r"+
93+
"\n node --expose_gc \"%~dp0\\from.env.args\" %*\r"+
9494
"\n)")
9595
t.end()
9696
})
9797
})
9898

99+
test('env shebang with variables', function (t) {
100+
var from = path.resolve(fixtures, 'from.env.variables')
101+
var to = path.resolve(fixtures, 'env.variables.shim')
102+
cmdShim(from, to, function(er) {
103+
if (er)
104+
throw er
105+
console.error('%j', fs.readFileSync(to, 'utf8'))
106+
console.error('%j', fs.readFileSync(to + '.cmd', 'utf8'))
107+
108+
t.equal(fs.readFileSync(to, 'utf8'),
109+
"#!/bin/sh"+
110+
"\nbasedir=$(dirname \"$(echo \"$0\" | sed -e 's,\\\\,/,g')\")" +
111+
"\n"+
112+
"\ncase `uname` in"+
113+
"\n *CYGWIN*) basedir=`cygpath -w \"$basedir\"`;;"+
114+
"\nesac"+
115+
"\n"+
116+
"\nif [ -x \"$basedir/node\" ]; then"+
117+
"\n NODE_PATH=./lib:$NODE_PATH \"$basedir/node\" \"$basedir/from.env.variables\" \"$@\""+
118+
"\n ret=$?"+
119+
"\nelse "+
120+
"\n NODE_PATH=./lib:$NODE_PATH node \"$basedir/from.env.variables\" \"$@\""+
121+
"\n ret=$?"+
122+
"\nfi"+
123+
"\nexit $ret"+
124+
"\n")
125+
t.equal(fs.readFileSync(to + '.cmd', 'utf8'),
126+
"@SETLOCAL\r"+
127+
"\n@SET NODE_PATH=./lib:%NODE_PATH%\r"+
128+
"\n@IF EXIST \"%~dp0\\node.exe\" (\r"+
129+
"\n \"%~dp0\\node.exe\" \"%~dp0\\from.env.variables\" %*\r"+
130+
"\n) ELSE (\r"+
131+
"\n @SETLOCAL\r" +
132+
"\n @SET PATHEXT=%PATHEXT:;.JS;=;%\r" +
133+
"\n node \"%~dp0\\from.env.variables\" %*\r"+
134+
"\n)\r"+
135+
"\n@ENDLOCAL")
136+
t.end()
137+
})
138+
})
139+
99140
test('explicit shebang', function (t) {
100141
var from = path.resolve(fixtures, 'from.sh')
101142
var to = path.resolve(fixtures, 'sh.shim')
@@ -153,22 +194,22 @@ test('explicit shebang with args', function (t) {
153194
"\nesac" +
154195
"\n" +
155196
"\nif [ -x \"$basedir//usr/bin/sh\" ]; then" +
156-
"\n \"$basedir//usr/bin/sh\" -x \"$basedir/from.sh.args\" \"$@\"" +
197+
"\n \"$basedir//usr/bin/sh\" -x \"$basedir/from.sh.args\" \"$@\"" +
157198
"\n ret=$?" +
158199
"\nelse " +
159-
"\n /usr/bin/sh -x \"$basedir/from.sh.args\" \"$@\"" +
200+
"\n /usr/bin/sh -x \"$basedir/from.sh.args\" \"$@\"" +
160201
"\n ret=$?" +
161202
"\nfi" +
162203
"\nexit $ret" +
163204
"\n")
164205

165206
t.equal(fs.readFileSync(to + '.cmd', 'utf8'),
166207
"@IF EXIST \"%~dp0\\/usr/bin/sh.exe\" (\r" +
167-
"\n \"%~dp0\\/usr/bin/sh.exe\" -x \"%~dp0\\from.sh.args\" %*\r" +
208+
"\n \"%~dp0\\/usr/bin/sh.exe\" -x \"%~dp0\\from.sh.args\" %*\r" +
168209
"\n) ELSE (\r" +
169210
"\n @SETLOCAL\r"+
170211
"\n @SET PATHEXT=%PATHEXT:;.JS;=;%\r"+
171-
"\n /usr/bin/sh -x \"%~dp0\\from.sh.args\" %*\r" +
212+
"\n /usr/bin/sh -x \"%~dp0\\from.sh.args\" %*\r" +
172213
"\n)")
173214
t.end()
174215
})

‎test/to-batch-syntax-tests.js

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
var test = require('tap').test
2+
var toBatchSyntax = require('../lib/to-batch-syntax')
3+
4+
test('replace $ expressions with % pair', function (t) {
5+
var assertReplacement = function(string, expected) {
6+
t.equal(toBatchSyntax.replaceDollarWithPercentPair(string), expected)
7+
}
8+
assertReplacement("$A", "%A%")
9+
assertReplacement("$A:$B", "%A%:%B%")
10+
assertReplacement("$A bla", "%A% bla")
11+
assertReplacement("${A}bla", "%A%bla")
12+
assertReplacement("$A $bla bla", "%A% %bla% bla")
13+
assertReplacement("${A}bla ${bla}bla", "%A%bla %bla%bla")
14+
assertReplacement("./lib:$NODE_PATH", "./lib:%NODE_PATH%")
15+
t.end()
16+
})
17+
18+
test('convert variable declaration to set command', function(t) {
19+
t.equal(toBatchSyntax.convertToSetCommand("A",".lib:$A "), "@SET A=.lib:%A%\r\n")
20+
t.equal(toBatchSyntax.convertToSetCommand("", ""), "")
21+
t.equal(toBatchSyntax.convertToSetCommand(" ", ""), "")
22+
t.equal(toBatchSyntax.convertToSetCommand(" ", " "), "")
23+
t.equal(toBatchSyntax.convertToSetCommand(" ou", " ou "), "@SET ou=ou\r\n")
24+
t.end()
25+
})

0 commit comments

Comments
 (0)