Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use execa's cross-platform shebang support in HooksRunner #819

Merged
merged 1 commit into from Nov 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 0 additions & 14 deletions integration-tests/HooksRunner.spec.js
Expand Up @@ -435,18 +435,4 @@ describe('HooksRunner', function () {
return hooksRunner.fire('CLEAN YOUR SHORTS GODDAMNIT LIKE A BIG BOY!', hookOptions);
});
});

describe('extractSheBangInterpreter', () => {
const rewire = require('rewire');
const HooksRunner = rewire('../src/hooks/HooksRunner');
const extractSheBangInterpreter = HooksRunner.__get__('extractSheBangInterpreter');

it('Test 025 : should not read uninitialized buffer contents', () => {
spyOn(require('fs'), 'readSync').and.callFake((fd, buf) => {
buf.write('#!/usr/bin/env XXX\n# foo');
return 0;
});
expect(extractSheBangInterpreter(__filename)).toBeFalsy();
});
});
});
4 changes: 1 addition & 3 deletions package.json
Expand Up @@ -31,9 +31,7 @@
"init-package-json": "^1.10.3",
"md5-file": "^4.0.0",
"pify": "^4.0.1",
"read-chunk": "^3.2.0",
"semver": "^6.3.0",
"shebang-command": "^2.0.0"
"semver": "^6.3.0"
},
"devDependencies": {
"codecov": "^3.2.0",
Expand Down
30 changes: 0 additions & 30 deletions src/hooks/HooksRunner.js
Expand Up @@ -17,18 +17,13 @@

const execa = require('execa');
const fs = require('fs-extra');
const os = require('os');
const path = require('path');
const readChunk = require('read-chunk');
const shebangCommand = require('shebang-command');

const cordovaUtil = require('../cordova/util');
const scriptsFinder = require('./scriptsFinder');
const Context = require('./Context');
const { CordovaError, events } = require('cordova-common');

const isWindows = os.platform().slice(0, 3) === 'win';

/**
* Tries to create a HooksRunner for passed project root.
* @constructor
Expand Down Expand Up @@ -173,16 +168,6 @@ function runScriptViaChildProcessSpawn (script, context) {
var command = script.fullPath;
var args = [opts.projectRoot];

if (isWindows) {
// TODO: Make shebang sniffing a setting (not everyone will want this).
var interpreter = extractSheBangInterpreter(script.fullPath);
// we have shebang, so try to run this script using correct interpreter
if (interpreter) {
args.unshift(command);
command = interpreter;
}
}

const execOpts = {
cwd: opts.projectRoot,
stdio: 'inherit',
Expand All @@ -201,21 +186,6 @@ function runScriptViaChildProcessSpawn (script, context) {
.then(data => data.stdout);
}

/**
* Extracts shebang interpreter from script' source. */
function extractSheBangInterpreter (fullpath) {
// this is a modern cluster size. no need to read less
const chunkSize = 4096;
const fileData = readChunk.sync(fullpath, 0, chunkSize);
const fileChunk = fileData.toString();
const hookCmd = shebangCommand(fileChunk);

if (hookCmd && fileData.length === chunkSize && !fileChunk.match(/[\r\n]/)) {
events.emit('warn', 'shebang is too long for "' + fullpath + '"');
}
return hookCmd;
}

/**
* Checks if the given hook type is disabled at the command line option.
* @param {Object} opts - the option object that contains command line options
Expand Down