You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ra...@apache.org on 2019/10/10 17:57:46 UTC
[cordova-lib] branch master updated: Do not run legacy hooks from
dirs anymore (#797)
This is an automated email from the ASF dual-hosted git repository.
raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-lib.git
The following commit(s) were added to refs/heads/master by this push:
new 36a064d Do not run legacy hooks from dirs anymore (#797)
36a064d is described below
commit 36a064d42601d16505b8adccadcde94976ed47aa
Author: Raphael von der Grün <ra...@gmail.com>
AuthorDate: Thu Oct 10 19:57:42 2019 +0200
Do not run legacy hooks from dirs anymore (#797)
---
integration-tests/HooksRunner.spec.js | 34 +++++---------
.../fixtures/projWithHooks/.cordova/config.json | 22 ---------
.../.cordova/hooks/before_build/hookScriptDot03.js | 4 --
.../.cordova/hooks/before_build/hookScriptDot1.js | 4 --
.../projWithHooks/.cordova/hooks/fail/fail.js | 2 -
.../.cordova/hooks/before_build/hookScriptDot2.bat | 2 -
.../_bat/hooks/before_build/hookScript01.bat | 2 -
.../_bat/hooks/before_build/hookScript02.bat | 2 -
.../_bat/hooks/before_build/hookScript3.bat | 2 -
.../.cordova/hooks/before_build/hookScriptDot2.sh | 1 -
.../_sh/hooks/before_build/hookScript01.sh | 1 -
.../_sh/hooks/before_build/hookScript02.sh | 1 -
.../_sh/hooks/before_build/hookScript3.sh | 1 -
.../hooks/before_build/hookScript10.js | 4 --
.../cordova/fixtures/projWithHooks/scripts/fail.js | 3 ++
src/hooks/HooksRunner.js | 33 +++----------
src/hooks/scriptsFinder.js | 54 ++++------------------
17 files changed, 31 insertions(+), 141 deletions(-)
diff --git a/integration-tests/HooksRunner.spec.js b/integration-tests/HooksRunner.spec.js
index a80d8d6..24241f0 100644
--- a/integration-tests/HooksRunner.spec.js
+++ b/integration-tests/HooksRunner.spec.js
@@ -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) {
diff --git a/spec/cordova/fixtures/projWithHooks/.cordova/config.json b/spec/cordova/fixtures/projWithHooks/.cordova/config.json
deleted file mode 100644
index 4f52ca7..0000000
--- a/spec/cordova/fixtures/projWithHooks/.cordova/config.json
+++ /dev/null
@@ -1,22 +0,0 @@
-{
- "id": "org.testing",
- "name":"TestBase",
- "lib": {
- "android": {
- "url": "/some/junk/path",
- "version": "dev",
- "id": "cordova-android-dev"
- },
- "ios": {
- "url": "/some/junk/path",
- "version": "dev",
- "id": "cordova-ios-dev"
- },
- "wp8": {
- "url": "/some/junk/path",
- "version": "dev",
- "id": "cordova-wp8-dev"
- }
- }
-}
-
diff --git a/spec/cordova/fixtures/projWithHooks/.cordova/hooks/before_build/hookScriptDot03.js b/spec/cordova/fixtures/projWithHooks/.cordova/hooks/before_build/hookScriptDot03.js
deleted file mode 100644
index 5e69a7f..0000000
--- a/spec/cordova/fixtures/projWithHooks/.cordova/hooks/before_build/hookScriptDot03.js
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/usr/bin/env node
-var path = require('path');
-var orderLogger = require(path.join(process.argv.slice(2)[0], 'scripts', 'orderLogger'));
-orderLogger.logOrder('01');
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/.cordova/hooks/before_build/hookScriptDot1.js b/spec/cordova/fixtures/projWithHooks/.cordova/hooks/before_build/hookScriptDot1.js
deleted file mode 100644
index 2e8d8d5..0000000
--- a/spec/cordova/fixtures/projWithHooks/.cordova/hooks/before_build/hookScriptDot1.js
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/usr/bin/env node
-var path = require('path');
-var orderLogger = require(path.join(process.argv.slice(2)[0], 'scripts', 'orderLogger'));
-orderLogger.logOrder('02');
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/.cordova/hooks/fail/fail.js b/spec/cordova/fixtures/projWithHooks/.cordova/hooks/fail/fail.js
deleted file mode 100644
index 7e8ff52..0000000
--- a/spec/cordova/fixtures/projWithHooks/.cordova/hooks/fail/fail.js
+++ /dev/null
@@ -1,2 +0,0 @@
-#!/usr/bin/env node
-process.exit(1);
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/_bat/.cordova/hooks/before_build/hookScriptDot2.bat b/spec/cordova/fixtures/projWithHooks/_bat/.cordova/hooks/before_build/hookScriptDot2.bat
deleted file mode 100644
index 0142765..0000000
--- a/spec/cordova/fixtures/projWithHooks/_bat/.cordova/hooks/before_build/hookScriptDot2.bat
+++ /dev/null
@@ -1,2 +0,0 @@
-@echo off
-echo 03 >> hooks_order.txt
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript01.bat b/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript01.bat
deleted file mode 100644
index a6711cf..0000000
--- a/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript01.bat
+++ /dev/null
@@ -1,2 +0,0 @@
-@echo off
-echo 04 >> hooks_order.txt
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript02.bat b/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript02.bat
deleted file mode 100644
index 1e824a4..0000000
--- a/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript02.bat
+++ /dev/null
@@ -1,2 +0,0 @@
-@echo off
-echo 05 >> hooks_order.txt
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript3.bat b/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript3.bat
deleted file mode 100644
index f339213..0000000
--- a/spec/cordova/fixtures/projWithHooks/_bat/hooks/before_build/hookScript3.bat
+++ /dev/null
@@ -1,2 +0,0 @@
-@echo off
-echo 07 >> hooks_order.txt
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/_sh/.cordova/hooks/before_build/hookScriptDot2.sh b/spec/cordova/fixtures/projWithHooks/_sh/.cordova/hooks/before_build/hookScriptDot2.sh
deleted file mode 100644
index 7c0c654..0000000
--- a/spec/cordova/fixtures/projWithHooks/_sh/.cordova/hooks/before_build/hookScriptDot2.sh
+++ /dev/null
@@ -1 +0,0 @@
-echo 03 >> hooks_order.txt
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript01.sh b/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript01.sh
deleted file mode 100644
index ccc8c89..0000000
--- a/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript01.sh
+++ /dev/null
@@ -1 +0,0 @@
-echo 04 >> hooks_order.txt
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript02.sh b/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript02.sh
deleted file mode 100644
index 4926cd8..0000000
--- a/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript02.sh
+++ /dev/null
@@ -1 +0,0 @@
-echo 05 >> hooks_order.txt
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript3.sh b/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript3.sh
deleted file mode 100644
index c52c84a..0000000
--- a/spec/cordova/fixtures/projWithHooks/_sh/hooks/before_build/hookScript3.sh
+++ /dev/null
@@ -1 +0,0 @@
-echo 07 >> hooks_order.txt
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/hooks/before_build/hookScript10.js b/spec/cordova/fixtures/projWithHooks/hooks/before_build/hookScript10.js
deleted file mode 100644
index bad1036..0000000
--- a/spec/cordova/fixtures/projWithHooks/hooks/before_build/hookScript10.js
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/usr/bin/env node
-var path = require('path');
-var orderLogger = require(path.join(process.argv.slice(2)[0], 'scripts', 'orderLogger'));
-orderLogger.logOrder('06');
\ No newline at end of file
diff --git a/spec/cordova/fixtures/projWithHooks/scripts/fail.js b/spec/cordova/fixtures/projWithHooks/scripts/fail.js
new file mode 100644
index 0000000..d6f2c02
--- /dev/null
+++ b/spec/cordova/fixtures/projWithHooks/scripts/fail.js
@@ -0,0 +1,3 @@
+module.exports = () => {
+ throw new Error();
+};
\ No newline at end of file
diff --git a/src/hooks/HooksRunner.js b/src/hooks/HooksRunner.js
index a42044f..16c518c 100644
--- a/src/hooks/HooksRunner.js
+++ b/src/hooks/HooksRunner.js
@@ -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);
});
}
diff --git a/src/hooks/scriptsFinder.js b/src/hooks/scriptsFinder.js
index e8a6d43..8e501f7 100644
--- a/src/hooks/scriptsFinder.js
+++ b/src/hooks/scriptsFinder.js
@@ -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)
+ }));
}
/**
@@ -77,44 +81,6 @@ function getPluginsHookScripts (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.
*/
function getPluginScriptFiles (plugin, hook, platforms) {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org