From 9c93ac39e95b0d6ae852e842e4c5dba5e19687c2 Mon Sep 17 00:00:00 2001 From: Bas Bossink Date: Mon, 29 Apr 2013 02:32:07 +0200 Subject: [PATCH] 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 @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: https://github.com/npm/cmd-shim/pull/2 Fix: https://github.com/npm/npm/issues/3380 Credit: @basbossink Close: #2 Reviewed-by: @isaacs --- index.js | 28 +++++++++++------ lib/to-batch-syntax.js | 52 ++++++++++++++++++++++++++++++++ test/00-setup.js | 1 + test/basic.js | 57 ++++++++++++++++++++++++++++++----- test/to-batch-syntax-tests.js | 25 +++++++++++++++ 5 files changed, 146 insertions(+), 17 deletions(-) create mode 100644 lib/to-batch-syntax.js create mode 100644 test/to-batch-syntax-tests.js diff --git a/index.js b/index.js index 9f22e10..50ae17a 100644 --- a/index.js +++ b/index.js @@ -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) { @@ -63,15 +64,17 @@ 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 @@ -79,6 +82,7 @@ function writeShim_ (from, to, prog, args, cb) { , shLongProg shTarget = shTarget.split("\\").join("/") args = args || "" + variables = variables || "" if (!prog) { prog = "\"%~dp0\\" + target + "\"" shProg = "\"$basedir/" + shTarget + "\"" @@ -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" } @@ -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" diff --git a/lib/to-batch-syntax.js b/lib/to-batch-syntax.js new file mode 100644 index 0000000..30fda7c --- /dev/null +++ b/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 +} + + diff --git a/test/00-setup.js b/test/00-setup.js index 04ec2b2..c9a7d39 100644 --- a/test/00-setup.js +++ b/test/00-setup.js @@ -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' } diff --git a/test/basic.js b/test/basic.js index 0982315..ae1e0c5 100755 --- a/test/basic.js +++ b/test/basic.js @@ -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') @@ -153,10 +194,10 @@ 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" + @@ -164,11 +205,11 @@ test('explicit shebang with args', function (t) { 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() }) diff --git a/test/to-batch-syntax-tests.js b/test/to-batch-syntax-tests.js new file mode 100644 index 0000000..6050f92 --- /dev/null +++ b/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() +})