Skip to content

Commit

Permalink
fix: Handle environment variables properly
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
basbossink authored and isaacs committed Aug 12, 2019
1 parent 3bf6d4a commit 9c93ac3
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 17 deletions.
28 changes: 19 additions & 9 deletions index.js
Expand Up @@ -15,7 +15,8 @@ var fs = require("graceful-fs")

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

function cmdShimIfExists (from, to, cb) {
fs.stat(from, function (er) {
Expand Down Expand Up @@ -63,22 +64,25 @@ function writeShim (from, to, cb) {
if (er) return writeShim_(from, to, null, null, cb)
var firstLine = data.trim().split(/\r*\n/)[0]
, shebang = firstLine.match(shebangExpr)
if (!shebang) return writeShim_(from, to, null, null, cb)
var prog = shebang[1]
, args = shebang[2] || ""
return writeShim_(from, to, prog, args, cb)
if (!shebang) return writeShim_(from, to, null, null, null, cb)
var vars = shebang[1] || ""
, prog = shebang[2]
, args = shebang[3] || ""
return writeShim_(from, to, prog, args, vars, cb)
})
})
}

function writeShim_ (from, to, prog, args, cb) {

function writeShim_ (from, to, prog, args, variables, cb) {
var shTarget = path.relative(path.dirname(to), from)
, target = shTarget.split("/").join("\\")
, longProg
, shProg = prog && prog.split("\\").join("/")
, shLongProg
shTarget = shTarget.split("\\").join("/")
args = args || ""
variables = variables || ""
if (!prog) {
prog = "\"%~dp0\\" + target + "\""
shProg = "\"$basedir/" + shTarget + "\""
Expand All @@ -101,13 +105,19 @@ function writeShim_ (from, to, prog, args, cb) {
// )
var cmd
if (longProg) {
cmd = "@IF EXIST " + longProg + " (\r\n"
shLongProg = shLongProg.trim();
args = args.trim();
var variableDeclarationsAsBatch = toBatchSyntax.convertToSetCommands(variables) || "";
cmd = ((variableDeclarationsAsBatch.length > 0) ? ("@SETLOCAL\r\n"
+ variableDeclarationsAsBatch) : "")
+ "@IF EXIST " + longProg + " (\r\n"
+ " " + longProg + " " + args + " " + target + " %*\r\n"
+ ") ELSE (\r\n"
+ " @SETLOCAL\r\n"
+ " @SET PATHEXT=%PATHEXT:;.JS;=;%\r\n"
+ " " + prog + " " + args + " " + target + " %*\r\n"
+ ")"
+ ((variableDeclarationsAsBatch.length > 0) ? "\r\n@ENDLOCAL" : "")
} else {
cmd = "@" + prog + " " + args + " " + target + " %*\r\n"
}
Expand Down Expand Up @@ -141,10 +151,10 @@ function writeShim_ (from, to, prog, args, cb) {

sh = sh
+ "if [ -x "+shLongProg+" ]; then\n"
+ " " + shLongProg + " " + args + " " + shTarget + " \"$@\"\n"
+ " " + variables + shLongProg + " " + args + " " + shTarget + " \"$@\"\n"
+ " ret=$?\n"
+ "else \n"
+ " " + shProg + " " + args + " " + shTarget + " \"$@\"\n"
+ " " + variables + shProg + " " + args + " " + shTarget + " \"$@\"\n"
+ " ret=$?\n"
+ "fi\n"
+ "exit $ret\n"
Expand Down
52 changes: 52 additions & 0 deletions lib/to-batch-syntax.js
@@ -0,0 +1,52 @@
exports.replaceDollarWithPercentPair = replaceDollarWithPercentPair
exports.convertToSetCommand = convertToSetCommand
exports.convertToSetCommands = convertToSetCommands

function convertToSetCommand(key, value) {
var line = ""
key = key || ""
key = key.trim()
value = value || ""
value = value.trim()
if(key && value && value.length > 0) {
line = "@SET " + key + "=" + replaceDollarWithPercentPair(value) + "\r\n"
}
return line
}

function extractVariableValuePairs(declarations) {
var pairs = {}
declarations.map(function(declaration) {
var split = declaration.split("=")
pairs[split[0]]=split[1]
})
return pairs
}

function convertToSetCommands(variableString) {
var variableValuePairs = extractVariableValuePairs(variableString.split(" "))
var variableDeclarationsAsBatch = ""
Object.keys(variableValuePairs).forEach(function (key) {
variableDeclarationsAsBatch += convertToSetCommand(key, variableValuePairs[key])
})
return variableDeclarationsAsBatch
}

function replaceDollarWithPercentPair(value) {
var dollarExpressions = /\$\{?([^\$@#\?\- \t{}:]+)\}?/g
var result = ""
var startIndex = 0
value = value || ""
do {
var match = dollarExpressions.exec(value)
if(match) {
var betweenMatches = value.substring(startIndex, match.index) || ""
result += betweenMatches + "%" + match[1] + "%"
startIndex = dollarExpressions.lastIndex
}
} while (dollarExpressions.lastIndex > 0)
result += value.substr(startIndex)
return result
}


1 change: 1 addition & 0 deletions test/00-setup.js
Expand Up @@ -8,6 +8,7 @@ var froms = {
'from.exe': 'exe',
'from.env': '#!/usr/bin/env node\nconsole.log(/hi/)\n',
'from.env.args': '#!/usr/bin/env node --expose_gc\ngc()\n',
'from.env.variables': '#!/usr/bin/env NODE_PATH=./lib:$NODE_PATH node',
'from.sh': '#!/usr/bin/sh\necho hi\n',
'from.sh.args': '#!/usr/bin/sh -x\necho hi\n'
}
Expand Down
57 changes: 49 additions & 8 deletions test/basic.js
Expand Up @@ -76,26 +76,67 @@ test('env shebang with args', function (t) {
"\nesac"+
"\n"+
"\nif [ -x \"$basedir/node\" ]; then"+
"\n \"$basedir/node\" --expose_gc \"$basedir/from.env.args\" \"$@\""+
"\n \"$basedir/node\" --expose_gc \"$basedir/from.env.args\" \"$@\""+
"\n ret=$?"+
"\nelse "+
"\n node --expose_gc \"$basedir/from.env.args\" \"$@\""+
"\n node --expose_gc \"$basedir/from.env.args\" \"$@\""+
"\n ret=$?"+
"\nfi"+
"\nexit $ret"+
"\n")
t.equal(fs.readFileSync(to + '.cmd', 'utf8'),
"@IF EXIST \"%~dp0\\node.exe\" (\r"+
"\n \"%~dp0\\node.exe\" --expose_gc \"%~dp0\\from.env.args\" %*\r"+
"\n \"%~dp0\\node.exe\" --expose_gc \"%~dp0\\from.env.args\" %*\r"+
"\n) ELSE (\r"+
"\n @SETLOCAL\r"+
"\n @SET PATHEXT=%PATHEXT:;.JS;=;%\r"+
"\n node --expose_gc \"%~dp0\\from.env.args\" %*\r"+
"\n node --expose_gc \"%~dp0\\from.env.args\" %*\r"+
"\n)")
t.end()
})
})

test('env shebang with variables', function (t) {
var from = path.resolve(fixtures, 'from.env.variables')
var to = path.resolve(fixtures, 'env.variables.shim')
cmdShim(from, to, function(er) {
if (er)
throw er
console.error('%j', fs.readFileSync(to, 'utf8'))
console.error('%j', fs.readFileSync(to + '.cmd', 'utf8'))

t.equal(fs.readFileSync(to, 'utf8'),
"#!/bin/sh"+
"\nbasedir=$(dirname \"$(echo \"$0\" | sed -e 's,\\\\,/,g')\")" +
"\n"+
"\ncase `uname` in"+
"\n *CYGWIN*) basedir=`cygpath -w \"$basedir\"`;;"+
"\nesac"+
"\n"+
"\nif [ -x \"$basedir/node\" ]; then"+
"\n NODE_PATH=./lib:$NODE_PATH \"$basedir/node\" \"$basedir/from.env.variables\" \"$@\""+
"\n ret=$?"+
"\nelse "+
"\n NODE_PATH=./lib:$NODE_PATH node \"$basedir/from.env.variables\" \"$@\""+
"\n ret=$?"+
"\nfi"+
"\nexit $ret"+
"\n")
t.equal(fs.readFileSync(to + '.cmd', 'utf8'),
"@SETLOCAL\r"+
"\n@SET NODE_PATH=./lib:%NODE_PATH%\r"+
"\n@IF EXIST \"%~dp0\\node.exe\" (\r"+
"\n \"%~dp0\\node.exe\" \"%~dp0\\from.env.variables\" %*\r"+
"\n) ELSE (\r"+
"\n @SETLOCAL\r" +
"\n @SET PATHEXT=%PATHEXT:;.JS;=;%\r" +
"\n node \"%~dp0\\from.env.variables\" %*\r"+
"\n)\r"+
"\n@ENDLOCAL")
t.end()
})
})

test('explicit shebang', function (t) {
var from = path.resolve(fixtures, 'from.sh')
var to = path.resolve(fixtures, 'sh.shim')
Expand Down Expand Up @@ -153,22 +194,22 @@ test('explicit shebang with args', function (t) {
"\nesac" +
"\n" +
"\nif [ -x \"$basedir//usr/bin/sh\" ]; then" +
"\n \"$basedir//usr/bin/sh\" -x \"$basedir/from.sh.args\" \"$@\"" +
"\n \"$basedir//usr/bin/sh\" -x \"$basedir/from.sh.args\" \"$@\"" +
"\n ret=$?" +
"\nelse " +
"\n /usr/bin/sh -x \"$basedir/from.sh.args\" \"$@\"" +
"\n /usr/bin/sh -x \"$basedir/from.sh.args\" \"$@\"" +
"\n ret=$?" +
"\nfi" +
"\nexit $ret" +
"\n")

t.equal(fs.readFileSync(to + '.cmd', 'utf8'),
"@IF EXIST \"%~dp0\\/usr/bin/sh.exe\" (\r" +
"\n \"%~dp0\\/usr/bin/sh.exe\" -x \"%~dp0\\from.sh.args\" %*\r" +
"\n \"%~dp0\\/usr/bin/sh.exe\" -x \"%~dp0\\from.sh.args\" %*\r" +
"\n) ELSE (\r" +
"\n @SETLOCAL\r"+
"\n @SET PATHEXT=%PATHEXT:;.JS;=;%\r"+
"\n /usr/bin/sh -x \"%~dp0\\from.sh.args\" %*\r" +
"\n /usr/bin/sh -x \"%~dp0\\from.sh.args\" %*\r" +
"\n)")
t.end()
})
Expand Down
25 changes: 25 additions & 0 deletions test/to-batch-syntax-tests.js
@@ -0,0 +1,25 @@
var test = require('tap').test
var toBatchSyntax = require('../lib/to-batch-syntax')

test('replace $ expressions with % pair', function (t) {
var assertReplacement = function(string, expected) {
t.equal(toBatchSyntax.replaceDollarWithPercentPair(string), expected)
}
assertReplacement("$A", "%A%")
assertReplacement("$A:$B", "%A%:%B%")
assertReplacement("$A bla", "%A% bla")
assertReplacement("${A}bla", "%A%bla")
assertReplacement("$A $bla bla", "%A% %bla% bla")
assertReplacement("${A}bla ${bla}bla", "%A%bla %bla%bla")
assertReplacement("./lib:$NODE_PATH", "./lib:%NODE_PATH%")
t.end()
})

test('convert variable declaration to set command', function(t) {
t.equal(toBatchSyntax.convertToSetCommand("A",".lib:$A "), "@SET A=.lib:%A%\r\n")
t.equal(toBatchSyntax.convertToSetCommand("", ""), "")
t.equal(toBatchSyntax.convertToSetCommand(" ", ""), "")
t.equal(toBatchSyntax.convertToSetCommand(" ", " "), "")
t.equal(toBatchSyntax.convertToSetCommand(" ou", " ou "), "@SET ou=ou\r\n")
t.end()
})

0 comments on commit 9c93ac3

Please sign in to comment.