From 86ddb04168eca625b381c66df57e80bd0a261185 Mon Sep 17 00:00:00 2001 From: Bas Bossink Date: Mon, 29 Apr 2013 02:32:07 +0200 Subject: [PATCH 1/2] Fix npm issue #3380. Fix npm issue #3380 by changing the following: - 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. --- index.js | 28 ++++++++++++------ test/00-setup.js | 1 + test/basic.js | 55 ++++++++++++++++++++++++++++++----- test/to-batch-syntax-tests.js | 25 ++++++++++++++++ toBatchSyntax.js | 52 +++++++++++++++++++++++++++++++++ 5 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 test/to-batch-syntax-tests.js create mode 100644 toBatchSyntax.js diff --git a/index.js b/index.js index e6a736b..e925355 100644 --- a/index.js +++ b/index.js @@ -19,7 +19,8 @@ try { var mkdir = require("mkdirp") , path = require("path") - , shebangExpr = /^#\!\s*(?:\/usr\/bin\/env)?\s*([^ \t]+)(.*)$/ + , toBatchSyntax = require("./toBatchSyntax") + , shebangExpr = /^#\!\s*(?:\/usr\/bin\/env)?\s*([^ \t]+=[^ \t]+\s+)*\s*([^ \t]+)(.*)$/ function cmdShimIfExists (from, to, cb) { fs.stat(from, function (er) { @@ -67,15 +68,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 @@ -83,6 +86,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 + "\"" @@ -103,11 +107,17 @@ 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" + " " + 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/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 59021e2..1f66dad 100755 --- a/test/basic.js +++ b/test/basic.js @@ -74,24 +74,63 @@ 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 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 \"$0\"`"+ + "\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 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') @@ -147,10 +186,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" + @@ -158,9 +197,9 @@ 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 /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..4f7c9cb --- /dev/null +++ b/test/to-batch-syntax-tests.js @@ -0,0 +1,25 @@ +var test = require('tap').test +var toBatchSyntax = require('../toBatchSyntax') + +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(); +}) diff --git a/toBatchSyntax.js b/toBatchSyntax.js new file mode 100644 index 0000000..ac01185 --- /dev/null +++ b/toBatchSyntax.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; +} + + From 32fa5bfaa3b3459dd9cc2b7fc594d9db0b370c7d Mon Sep 17 00:00:00 2001 From: Bas Bossink Date: Mon, 29 Apr 2013 19:01:49 +0200 Subject: [PATCH 2/2] Implemented suggestions by @ForbesLindesay - removed all ; - moved ./toBatchSyntax.js to bin in ./lib/to-batch-syntax.js --- index.js | 2 +- toBatchSyntax.js => lib/to-batch-syntax.js | 54 +++++++++++----------- test/to-batch-syntax-tests.js | 32 ++++++------- 3 files changed, 44 insertions(+), 44 deletions(-) rename toBatchSyntax.js => lib/to-batch-syntax.js (50%) diff --git a/index.js b/index.js index e925355..6f189a6 100644 --- a/index.js +++ b/index.js @@ -19,7 +19,7 @@ try { var mkdir = require("mkdirp") , path = require("path") - , toBatchSyntax = require("./toBatchSyntax") + , toBatchSyntax = require("./lib/to-batch-syntax") , shebangExpr = /^#\!\s*(?:\/usr\/bin\/env)?\s*([^ \t]+=[^ \t]+\s+)*\s*([^ \t]+)(.*)$/ function cmdShimIfExists (from, to, cb) { diff --git a/toBatchSyntax.js b/lib/to-batch-syntax.js similarity index 50% rename from toBatchSyntax.js rename to lib/to-batch-syntax.js index ac01185..30fda7c 100644 --- a/toBatchSyntax.js +++ b/lib/to-batch-syntax.js @@ -3,50 +3,50 @@ exports.convertToSetCommand = convertToSetCommand exports.convertToSetCommands = convertToSetCommands function convertToSetCommand(key, value) { - var line = ""; - key = key || ""; - key = key.trim(); - value = value || ""; - value = value.trim(); + 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"; + line = "@SET " + key + "=" + replaceDollarWithPercentPair(value) + "\r\n" } - return line; + return line } function extractVariableValuePairs(declarations) { - var pairs = {}; + var pairs = {} declarations.map(function(declaration) { - var split = declaration.split("="); - pairs[split[0]]=split[1]; - }); - return pairs; + var split = declaration.split("=") + pairs[split[0]]=split[1] + }) + return pairs } function convertToSetCommands(variableString) { - var variableValuePairs = extractVariableValuePairs(variableString.split(" ")); - var variableDeclarationsAsBatch = ""; + var variableValuePairs = extractVariableValuePairs(variableString.split(" ")) + var variableDeclarationsAsBatch = "" Object.keys(variableValuePairs).forEach(function (key) { - variableDeclarationsAsBatch += convertToSetCommand(key, variableValuePairs[key]); - }); - return variableDeclarationsAsBatch; + variableDeclarationsAsBatch += convertToSetCommand(key, variableValuePairs[key]) + }) + return variableDeclarationsAsBatch } function replaceDollarWithPercentPair(value) { - var dollarExpressions = /\$\{?([^\$@#\?\- \t{}:]+)\}?/g; - var result = ""; - var startIndex = 0; - value = value || ""; + var dollarExpressions = /\$\{?([^\$@#\?\- \t{}:]+)\}?/g + var result = "" + var startIndex = 0 + value = value || "" do { - var match = dollarExpressions.exec(value); + var match = dollarExpressions.exec(value) if(match) { - var betweenMatches = value.substring(startIndex, match.index) || ""; - result += betweenMatches + "%" + match[1] + "%"; - startIndex = dollarExpressions.lastIndex; + var betweenMatches = value.substring(startIndex, match.index) || "" + result += betweenMatches + "%" + match[1] + "%" + startIndex = dollarExpressions.lastIndex } - } while (dollarExpressions.lastIndex > 0); + } while (dollarExpressions.lastIndex > 0) result += value.substr(startIndex) - return result; + return result } diff --git a/test/to-batch-syntax-tests.js b/test/to-batch-syntax-tests.js index 4f7c9cb..6050f92 100644 --- a/test/to-batch-syntax-tests.js +++ b/test/to-batch-syntax-tests.js @@ -1,25 +1,25 @@ var test = require('tap').test -var toBatchSyntax = require('../toBatchSyntax') +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); + 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%"); + 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(); + 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() })