Skip to content
Permalink
Browse files
Do not run legacy hooks from dirs anymore (#797)
  • Loading branch information
raphinesse committed Oct 10, 2019
1 parent 46c75f6 commit 36a064d42601d16505b8adccadcde94976ed47aa
Showing 17 changed files with 31 additions and 141 deletions.
@@ -49,14 +49,8 @@ describe('HooksRunner', function () {
const projectFixture = path.join(fixtures, 'projWithHooks');
fs.copySync(projectFixture, preparedProject);

// Copy sh/bat scripts
const extDir = path.join(projectFixture, `_${ext}`);
fs.readdirSync(extDir).forEach(d => {
fs.copySync(path.join(extDir, d), path.join(preparedProject, d));
});

// Ensure scripts are executable
globby.sync(['hooks/**', '.cordova/hooks/**', 'scripts/**'], {
globby.sync(['scripts/**'], {
cwd: preparedProject, absolute: true
}).forEach(f => fs.chmodSync(f, 0o755));

@@ -170,19 +164,6 @@ describe('HooksRunner', function () {
}

describe('application hooks', function () {
it('Test 004 : should execute hook scripts serially', function () {
return hooksRunner.fire(test_event, hookOptions)
.then(checkHooksOrderFile);
});

it('Test 005 : should execute hook scripts serially from .cordova/hooks/hook_type and hooks/hook_type directories', function () {
// using empty platforms list to test only hooks/ directories
hookOptions.cordova.platforms = [];

return hooksRunner.fire(test_event, hookOptions)
.then(checkHooksOrderFile);
});

it('Test 006 : should execute hook scripts serially from config.xml', function () {
addHooksToConfig(BASE_HOOKS);

@@ -207,7 +188,7 @@ describe('HooksRunner', function () {
return hooksRunner.fire(test_event, hookOptions).then(function () {
checkHooksOrderFile();

const baseScriptResults = [1, 2, 3, 4, 5, 6, 7, 8, 9];
const baseScriptResults = [8, 9];
const androidPlatformScriptsResults = [14, 15];
const expectedResults = baseScriptResults.concat(androidPlatformScriptsResults);
expect(getActualHooksOrder()).toEqual(expectedResults);
@@ -279,7 +260,7 @@ describe('HooksRunner', function () {
return hooksRunner.fire(test_event, hookOptions).then(function () {
checkHooksOrderFile();

const baseScriptResults = [1, 2, 3, 4, 5, 6, 7, 21, 22];
const baseScriptResults = [21, 22];
const androidPlatformScriptsResults = [26];
const expectedResults = baseScriptResults.concat(androidPlatformScriptsResults);
expect(getActualHooksOrder()).toEqual(expectedResults);
@@ -434,7 +415,14 @@ describe('HooksRunner', function () {
return hooksRunner.fire(test_event, hookOptions);
});

it('Test 023 : should error if any script exits with non-zero code', function () {
it('Test 023 : should error if any hook fails', function () {
const FAIL_HOOK = `
<widget xmlns="http://www.w3.org/ns/widgets">
<hook type="fail" src="scripts/fail.js" />
</widget>
`;
addHooksToConfig(FAIL_HOOK);

return hooksRunner.fire('fail', hookOptions).then(function () {
fail('Expected promise to be rejected');
}, function (err) {

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

@@ -0,0 +1,3 @@
module.exports = () => {
throw new Error();
};
@@ -120,33 +120,24 @@ function runJobsSerially (jobs) {
* Async runs single script file.
*/
function runScript (script, context) {
if (typeof script.useModuleLoader === 'undefined') {
// if it is not explicitly defined whether we should use modeule loader or not
// we assume we should use module loader for .js files
script.useModuleLoader = path.extname(script.path).toLowerCase() === '.js';
}

var source;
var relativePath;

if (script.plugin) {
source = 'plugin ' + script.plugin.id;
relativePath = path.join('plugins', script.plugin.id, script.path);
} else if (script.useModuleLoader) {
} else {
source = 'config.xml';
relativePath = path.normalize(script.path);
} else {
source = 'hooks directory';
relativePath = path.join('hooks', context.hook, script.path);
}

events.emit('verbose', 'Executing script found in ' + source + ' for hook "' + context.hook + '": ' + relativePath);

if (script.useModuleLoader) {
return runScriptViaModuleLoader(script, context);
} else {
return runScriptViaChildProcessSpawn(script, context);
}
const runScriptStrategy = path.extname(script.path).toLowerCase() === '.js'
? runScriptViaModuleLoader
: runScriptViaChildProcessSpawn;

return runScriptStrategy(script, context);
}

/**
@@ -181,11 +172,6 @@ function runScriptViaChildProcessSpawn (script, context) {
var command = script.fullPath;
var args = [opts.projectRoot];

if (fs.statSync(script.fullPath).isDirectory()) {
events.emit('verbose', 'Skipped directory "' + script.fullPath + '" within hook directory');
return Promise.resolve();
}

if (isWindows) {
// TODO: Make shebang sniffing a setting (not everyone will want this).
var interpreter = extractSheBangInterpreter(script.fullPath);
@@ -211,12 +197,7 @@ function runScriptViaChildProcessSpawn (script, context) {

return superspawn.spawn(command, args, execOpts)
.catch(function (err) {
// Don't treat non-executable files as errors. They could be READMEs, or Windows-only scripts.
if (!isWindows && err.code === 'EACCES') {
events.emit('verbose', 'Skipped non-executable file: ' + script.fullPath);
} else {
throw new Error('Hook failed with error code ' + err.code + ': ' + script.fullPath);
}
throw new Error('Hook failed with error code ' + err.code + ': ' + script.fullPath);
});
}

@@ -18,7 +18,6 @@
*/

var path = require('path');
var fs = require('fs-extra');
var cordovaUtil = require('../cordova/util');
var events = require('cordova-common').events;
var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
@@ -43,17 +42,22 @@ module.exports = {
};

/**
* Returns script files defined on application level.
* They are stored in .cordova/hooks folders and in config.xml.
* Gets all scripts defined in config.xml with the specified type and platforms.
*/
function getApplicationHookScripts (hook, opts) {
// args check
if (!hook) {
throw new Error('hook type is not specified');
}
return getApplicationHookScriptsFromDir(path.join(opts.projectRoot, '.cordova', 'hooks', hook))
.concat(getApplicationHookScriptsFromDir(path.join(opts.projectRoot, 'hooks', hook)))
.concat(getScriptsFromConfigXml(hook, opts));

const configPath = cordovaUtil.projectConfig(opts.projectRoot);
const configXml = new ConfigParser(configPath);

return configXml.getHookScripts(hook, opts.cordova.platforms)
.map(scriptElement => ({
path: scriptElement.attrib.src,
fullPath: path.join(opts.projectRoot, scriptElement.attrib.src)
}));
}

/**
@@ -76,44 +80,6 @@ function getPluginsHookScripts (hook, opts) {
return getAllPluginsHookScriptFiles(hook, opts);
}

/**
* Gets application level hooks from the directrory specified.
*/
function getApplicationHookScriptsFromDir (dir) {
if (!(fs.existsSync(dir))) {
return [];
}

var compareNumbers = function (a, b) {
// TODO SG looks very complex, do we really need this?
return isNaN(parseInt(a, 10)) ? a.toLowerCase().localeCompare(b.toLowerCase ? b.toLowerCase() : b)
: parseInt(a, 10) > parseInt(b, 10) ? 1 : parseInt(a, 10) < parseInt(b, 10) ? -1 : 0;
};

var scripts = fs.readdirSync(dir).sort(compareNumbers).filter(function (s) {
return s[0] !== '.';
});
return scripts.map(function (scriptPath) {
// for old style hook files we don't use module loader for backward compatibility
return { path: scriptPath, fullPath: path.join(dir, scriptPath), useModuleLoader: false };
});
}

/**
* Gets all scripts defined in config.xml with the specified type and platforms.
*/
function getScriptsFromConfigXml (hook, opts) {
var configPath = cordovaUtil.projectConfig(opts.projectRoot);
var configXml = new ConfigParser(configPath);

return configXml.getHookScripts(hook, opts.cordova.platforms).map(function (scriptElement) {
return {
path: scriptElement.attrib.src,
fullPath: path.join(opts.projectRoot, scriptElement.attrib.src)
};
});
}

/**
* Gets hook scripts defined by the plugin.
*/

0 comments on commit 36a064d

Please sign in to comment.