You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by sgrebnov <gi...@git.apache.org> on 2014/04/18 02:14:41 UTC
[GitHub] cordova-plugman pull request: adds plugin level hooks support
GitHub user sgrebnov opened a pull request:
https://github.com/apache/cordova-plugman/pull/74
adds plugin level hooks support
1.Support of **js script files only** (nodejs is used to run script file)
2.Support of the following **hook types**
* beforeinstall/preinstall – run before plugin is installed
* install/postinstall/afterinstall – run after plugin is installed
* uninstall – run after plugin is uninstalled
```
<script type="postinstall" src="scripts/postinstall.js" />
<script type="preinstall" src="scripts/preinstall.js" />
<script type="install" src="scripts/install.js" />
```
3.To retrieve execution parameters **inside hook file**
````
console.log ('\tCORDOVA_PLATFORM: ' + process.env.CORDOVA_PLATFORM);
console.log ('\tCORDOVA_PROJECT_DIR: ' + process.env.CORDOVA_PROJECT_DIR);
console.log ('\tCORDOVA_PLUGIN_DIR: ' + process.env.CORDOVA_PLUGIN_DIR);
console.log ('\tCORDOVA_CMDLINE: ' + process.env.CORDOVA_CMDLINE);
````
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-plugman plugin-hooks
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugman/pull/74.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #74
----
commit 6197e43911e2ffb66a0bed4c56ba35fb365c5c8a
Author: sgrebnov <v-...@microsoft.com>
Date: 2014-04-18T00:08:01Z
adds plugin level hooks support
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-plugman/pull/74#issuecomment-42234483
Totally agree, will do as soon as cordova-lib work is stabilized so I can move the code to appropriate repo and perform final tests. @ligaz, thx for review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12193146
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) Microsoft Open Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
+ throw Error('unknown plugin hook type: "' + type + '"' );
+ }
+
+ var scriptFilesToRun = getScriptFiles(pluginElement, scriptTypes, platform);
+ context = {
--- End diff --
Missing "var". This is the main reason I think using commas & multi-line var statements is a bad idea.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12257681
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,148 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
+ throw Error('unknown plugin hook type: "' + type + '"' );
+ }
+
+ var scriptFilesToRun = getScriptFiles(pluginElement, scriptTypes, platform);
+ var context = {
+ platform: platform,
+ projectDir: project_dir,
+ pluginDir: plugin_dir,
+ cmdLine: process.argv.join(' '),
+ pluginId: plugin_id
+ };
+
+ return runScripts(scriptFilesToRun, context);
+ }
+};
+
+/**
+ * Returns all script types represented corresponding hook event.
+ * Allows to use multiple script types for the same hook event (usage simplicity),
+ * For example:
+ * <script type='install' .. /> or <script type='postinstall' .. /> could be used
+ * to define 'afterinstall' hook.
+ */
+function getScriptTypesForHook(hookType) {
+ var knownTypes = {
+ beforeinstall: ['beforeinstall', 'preinstall'],
+ afterinstall: ['install', 'afterinstall', 'postinstall'],
+ uninstall: ['uninstall']
+ }
+
+ return knownTypes[hookType.toLowerCase()];
+}
+
+/**
+ * Gets all scripts from the plugin xml with the specified types.
+ */
+function getScriptFiles(pluginElement, scriptTypes, platform) {
+ var scriptFiles= [];
+ var scriptElements = pluginElement.findall('./script').concat(
+ pluginElement.findall('./platform[@name="' + platform + '"]/script'));
+
+ var pendingScripts = [];
--- End diff --
Agree, will remove
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by ligaz <gi...@git.apache.org>.
Github user ligaz commented on the pull request:
https://github.com/apache/cordova-plugman/pull/74#issuecomment-42123996
Adding a couple of unit tests will be great :cop:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12257679
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,148 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
+ throw Error('unknown plugin hook type: "' + type + '"' );
+ }
+
+ var scriptFilesToRun = getScriptFiles(pluginElement, scriptTypes, platform);
+ var context = {
+ platform: platform,
+ projectDir: project_dir,
+ pluginDir: plugin_dir,
+ cmdLine: process.argv.join(' '),
+ pluginId: plugin_id
+ };
+
+ return runScripts(scriptFilesToRun, context);
+ }
+};
+
+/**
+ * Returns all script types represented corresponding hook event.
+ * Allows to use multiple script types for the same hook event (usage simplicity),
+ * For example:
+ * <script type='install' .. /> or <script type='postinstall' .. /> could be used
+ * to define 'afterinstall' hook.
+ */
+function getScriptTypesForHook(hookType) {
+ var knownTypes = {
+ beforeinstall: ['beforeinstall', 'preinstall'],
+ afterinstall: ['install', 'afterinstall', 'postinstall'],
+ uninstall: ['uninstall']
+ }
+
+ return knownTypes[hookType.toLowerCase()];
+}
+
+/**
+ * Gets all scripts from the plugin xml with the specified types.
+ */
+function getScriptFiles(pluginElement, scriptTypes, platform) {
+ var scriptFiles= [];
+ var scriptElements = pluginElement.findall('./script').concat(
+ pluginElement.findall('./platform[@name="' + platform + '"]/script'));
+
+ var pendingScripts = [];
+
+ scriptElements.forEach(function(script){
--- End diff --
Actually no, we are interested only in scripts with specified type.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-plugman/pull/74#issuecomment-40968416
Created issue CB-6481 to cover this work.
Switched to nodejs module loader.
Sample hook file implementation with async functionality:
```
var Q = require('./q');
module.exports = function(platform, projectDir, pluginDir, cmdLine) {
console.log('hook.js: ' + platform);
console.log('hook.js: ' + projectDir);
console.log('hook.js: ' + pluginDir);
console.log('hook.js: ' + cmdLine);
var deferral = new Q.defer();
setTimeout(function(){
deferral.resolve();
}, 1000);
return deferral.promise;
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by ligaz <gi...@git.apache.org>.
Github user ligaz commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12259722
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,148 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
+ throw Error('unknown plugin hook type: "' + type + '"' );
+ }
+
+ var scriptFilesToRun = getScriptFiles(pluginElement, scriptTypes, platform);
+ var context = {
+ platform: platform,
+ projectDir: project_dir,
+ pluginDir: plugin_dir,
+ cmdLine: process.argv.join(' '),
+ pluginId: plugin_id
+ };
+
+ return runScripts(scriptFilesToRun, context);
+ }
+};
+
+/**
+ * Returns all script types represented corresponding hook event.
+ * Allows to use multiple script types for the same hook event (usage simplicity),
+ * For example:
+ * <script type='install' .. /> or <script type='postinstall' .. /> could be used
+ * to define 'afterinstall' hook.
+ */
+function getScriptTypesForHook(hookType) {
+ var knownTypes = {
+ beforeinstall: ['beforeinstall', 'preinstall'],
+ afterinstall: ['install', 'afterinstall', 'postinstall'],
+ uninstall: ['uninstall']
+ }
+
+ return knownTypes[hookType.toLowerCase()];
+}
+
+/**
+ * Gets all scripts from the plugin xml with the specified types.
+ */
+function getScriptFiles(pluginElement, scriptTypes, platform) {
+ var scriptFiles= [];
+ var scriptElements = pluginElement.findall('./script').concat(
+ pluginElement.findall('./platform[@name="' + platform + '"]/script'));
+
+ var pendingScripts = [];
+
+ scriptElements.forEach(function(script){
--- End diff --
Indeed you should use `filter` and `map`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by ligaz <gi...@git.apache.org>.
Github user ligaz commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12255215
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,148 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
+ throw Error('unknown plugin hook type: "' + type + '"' );
+ }
+
+ var scriptFilesToRun = getScriptFiles(pluginElement, scriptTypes, platform);
+ var context = {
+ platform: platform,
+ projectDir: project_dir,
+ pluginDir: plugin_dir,
+ cmdLine: process.argv.join(' '),
+ pluginId: plugin_id
+ };
+
+ return runScripts(scriptFilesToRun, context);
+ }
+};
+
+/**
+ * Returns all script types represented corresponding hook event.
+ * Allows to use multiple script types for the same hook event (usage simplicity),
+ * For example:
+ * <script type='install' .. /> or <script type='postinstall' .. /> could be used
+ * to define 'afterinstall' hook.
+ */
+function getScriptTypesForHook(hookType) {
+ var knownTypes = {
+ beforeinstall: ['beforeinstall', 'preinstall'],
+ afterinstall: ['install', 'afterinstall', 'postinstall'],
+ uninstall: ['uninstall']
+ }
+
+ return knownTypes[hookType.toLowerCase()];
+}
+
+/**
+ * Gets all scripts from the plugin xml with the specified types.
+ */
+function getScriptFiles(pluginElement, scriptTypes, platform) {
+ var scriptFiles= [];
+ var scriptElements = pluginElement.findall('./script').concat(
+ pluginElement.findall('./platform[@name="' + platform + '"]/script'));
+
+ var pendingScripts = [];
--- End diff --
Is this used?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12193116
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) Microsoft Open Technologies, Inc.
--- End diff --
Don't think it should say this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by ligaz <gi...@git.apache.org>.
Github user ligaz commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12255211
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,148 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
--- End diff --
`getScriptTypesForHook` actually returns `undefined` when there is no matching type. Why not just `(!scriptTypes)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-plugman/pull/74#issuecomment-41967514
@agrieve, thx for review; updated the code. Also did rebase
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-plugman/pull/74#issuecomment-41850345
Switched to single parameter called context
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12193181
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) Microsoft Open Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
+ throw Error('unknown plugin hook type: "' + type + '"' );
+ }
+
+ var scriptFilesToRun = getScriptFiles(pluginElement, scriptTypes, platform);
+ context = {
+ platform: platform,
+ projectDir: project_dir,
+ pluginDir: plugin_dir,
+ cmdLine: process.argv.join(' '),
+ pluginId: plugin_id
+ };
+
+ return runScripts(scriptFilesToRun, context);
+ }
+};
+
+/**
+ * Returns all script types represented corresponding hook event.
+ * Allows to use multiple script types for the same hook event (usage simplicity),
+ * For example:
+ * <script type='install' .. /> or <script type='postinstall' .. /> could be used
+ * to define 'afterinstall' hook.
+ */
+function getScriptTypesForHook(hookType) {
+ var knownTypes = {
+ beforeinstall: ['beforeinstall', 'preinstall'],
+ afterinstall: ['install', 'afterinstall', 'postinstall'],
+ uninstall: ['uninstall']
+ }
+
+ return knownTypes[hookType.toLowerCase()];
+}
+
+/**
+ * Gets all scripts from the plugin xml with the specified types.
+ */
+function getScriptFiles(pluginElement, scriptTypes, platform) {
+ var scriptFiles= [],
+ scriptElements = pluginElement.findall('./script').concat(
+ pluginElement.findall('./platform[@name="' + platform + '"]/script'));
+
+ var pendingScripts = [];
+
+ scriptElements.forEach(function(script){
+ if (script.attrib.type && scriptTypes.indexOf(script.attrib.type.toLowerCase()) > -1 && script.attrib.src) {
+ scriptFiles.push(script.attrib.src);
+ }
+ });
+ return scriptFiles;
+}
+
+/**
+ * Async runs the script files.
+ */
+function runScripts(scripts, context) {
+ var pendingScripts = [];
+
+ scripts.forEach(function(script){
+ pendingScripts.push(runScriptFile(script, context));
--- End diff --
This runs all scripts at the same time. Probably want to run them serially.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12193231
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) Microsoft Open Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
+ throw Error('unknown plugin hook type: "' + type + '"' );
+ }
+
+ var scriptFilesToRun = getScriptFiles(pluginElement, scriptTypes, platform);
+ context = {
+ platform: platform,
+ projectDir: project_dir,
+ pluginDir: plugin_dir,
+ cmdLine: process.argv.join(' '),
+ pluginId: plugin_id
+ };
+
+ return runScripts(scriptFilesToRun, context);
+ }
+};
+
+/**
+ * Returns all script types represented corresponding hook event.
+ * Allows to use multiple script types for the same hook event (usage simplicity),
+ * For example:
+ * <script type='install' .. /> or <script type='postinstall' .. /> could be used
+ * to define 'afterinstall' hook.
+ */
+function getScriptTypesForHook(hookType) {
+ var knownTypes = {
+ beforeinstall: ['beforeinstall', 'preinstall'],
+ afterinstall: ['install', 'afterinstall', 'postinstall'],
+ uninstall: ['uninstall']
+ }
+
+ return knownTypes[hookType.toLowerCase()];
+}
+
+/**
+ * Gets all scripts from the plugin xml with the specified types.
+ */
+function getScriptFiles(pluginElement, scriptTypes, platform) {
+ var scriptFiles= [],
+ scriptElements = pluginElement.findall('./script').concat(
+ pluginElement.findall('./platform[@name="' + platform + '"]/script'));
+
+ var pendingScripts = [];
+
+ scriptElements.forEach(function(script){
+ if (script.attrib.type && scriptTypes.indexOf(script.attrib.type.toLowerCase()) > -1 && script.attrib.src) {
+ scriptFiles.push(script.attrib.src);
+ }
+ });
+ return scriptFiles;
+}
+
+/**
+ * Async runs the script files.
+ */
+function runScripts(scripts, context) {
+ var pendingScripts = [];
+
+ scripts.forEach(function(script){
+ pendingScripts.push(runScriptFile(script, context));
+ });
+
+ return Q.all(pendingScripts);
+};
+
+/**
+ * Async runs single script file.
+ */
+function runScriptFile(scriptPath, context) {
+
+ scriptPath = path.join(context.pluginDir, scriptPath);
+
+ if(!fs.existsSync(scriptPath)) {
+ events.emit('warn', "Script file does't exist and will be skipped: " + scriptPath);
+ return Q();
+ }
+ var scriptFn = require(scriptPath);
+
+ // if hook is async it can return promise instance and we will handle it
+ return Q(new scriptFn(context));
--- End diff --
why "new" it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12257686
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,148 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
--- End diff --
Agree (!scriptTypes) is better here.
Btw, 'undefined == null' is true so it works correct
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov closed the pull request at:
https://github.com/apache/cordova-plugman/pull/74
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by ligaz <gi...@git.apache.org>.
Github user ligaz commented on a diff in the pull request:
https://github.com/apache/cordova-plugman/pull/74#discussion_r12255221
--- Diff: src/util/hooks.js ---
@@ -0,0 +1,148 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var path = require('path'),
+ fs = require('fs'),
+ child_process = require('child_process'),
+ Q = require('q'),
+ events = require('../events'),
+ os = require('os'),
+ isWindows = (os.platform().substr(0,3) === 'win');
+
+/**
+ * Implements functionality to run plugin level hooks.
+ * Hooks are defined in plugin config file as <script> elements.
+ */
+module.exports = {
+ /**
+ * Fires specific plugin hook: 'preinstall', 'afterinstall', 'uninstall', etc.
+ * Async. Returns promise.
+ */
+ fire : function(type, plugin_id, pluginElement, platform, project_dir, plugin_dir) {
+ // args check
+ if (!type) {
+ throw Error('hook type is not specified');
+ }
+
+ events.emit('debug', 'Executing "' + type + '" hook for "' + plugin_id + '" on ' + platform + '.');
+
+ var scriptTypes = getScriptTypesForHook(type);
+ if (scriptTypes == null) {
+ throw Error('unknown plugin hook type: "' + type + '"' );
+ }
+
+ var scriptFilesToRun = getScriptFiles(pluginElement, scriptTypes, platform);
+ var context = {
+ platform: platform,
+ projectDir: project_dir,
+ pluginDir: plugin_dir,
+ cmdLine: process.argv.join(' '),
+ pluginId: plugin_id
+ };
+
+ return runScripts(scriptFilesToRun, context);
+ }
+};
+
+/**
+ * Returns all script types represented corresponding hook event.
+ * Allows to use multiple script types for the same hook event (usage simplicity),
+ * For example:
+ * <script type='install' .. /> or <script type='postinstall' .. /> could be used
+ * to define 'afterinstall' hook.
+ */
+function getScriptTypesForHook(hookType) {
+ var knownTypes = {
+ beforeinstall: ['beforeinstall', 'preinstall'],
+ afterinstall: ['install', 'afterinstall', 'postinstall'],
+ uninstall: ['uninstall']
+ }
+
+ return knownTypes[hookType.toLowerCase()];
+}
+
+/**
+ * Gets all scripts from the plugin xml with the specified types.
+ */
+function getScriptFiles(pluginElement, scriptTypes, platform) {
+ var scriptFiles= [];
+ var scriptElements = pluginElement.findall('./script').concat(
+ pluginElement.findall('./platform[@name="' + platform + '"]/script'));
+
+ var pendingScripts = [];
+
+ scriptElements.forEach(function(script){
--- End diff --
Why not use [`Array.prototype.map`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map) here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-plugman pull request: CB-6481 adds plugin level hooks supp...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-plugman/pull/74#issuecomment-43280272
Moved to appropriate repo, closing...
https://github.com/apache/cordova-lib/pull/12
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---