Details
-
Type:
Improvement
-
Status: Resolved
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 6.0.0
-
Component/s: cordova-lib
-
Labels:
Issue Links
- links to
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Github user stevengill commented on the issue:
https://github.com/apache/cordova-cli/pull/239
This got merged in already. Don't know why it didn't close.
Github user purplecabbage commented on the issue:
https://github.com/apache/cordova-cli/pull/239
Is this still valid? Needs a rebase at least ...
Commit 9e98cda186e91b53736a9e4d9ee99e09a0eec5ec in cordova-coho's branch refs/heads/master from Vladimir Kotikov
[ https://git-wip-us.apache.org/repos/asf?p=cordova-coho.git;h=9e98cda ]
CB-9858 Set up links for cordova-common properly
Commit 8f1e51d366a2faa7a69335d503265c54e18771c8 in cordova-coho's branch refs/heads/master from Vladimir Kotikov
[ https://git-wip-us.apache.org/repos/asf?p=cordova-coho.git;h=8f1e51d ]
CB-9858 Link cordova-common to cordova-fetch
Commit 83d3383dc66d069ae902f100a407031f57bf93be in cordova-cli's branch refs/heads/master from Steve Gill
[ https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;h=83d3383 ]
CB-9858 fixed failing appveyor and travis builds
Commit f1e34f9b5113065a0ed49f05423e93bc3cd24c93 in cordova-lib's branch refs/heads/master from Steve Gill
[ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=f1e34f9 ]
CB-9858 added fetch tests to travis
Commit 2aac4434486278379f22ca0715eec5f506401671 in cordova-lib's branch refs/heads/master from Steve Gill
[ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=2aac443 ]
CB-9858 fixed failing travis and appveyor tests
Commit be7eb6edb56de7022f00af53d79aa6705c0d2f66 in cordova-coho's branch refs/heads/master from Vladimir Kotikov
[ https://git-wip-us.apache.org/repos/asf?p=cordova-coho.git;h=be7eb6e ]
CB-9858 Implement Cordova Fetch Proposal
Ensure fetch and cli is properly installed/linked
Add fetch to list of tools repos
Resolve cordova-fetch location properly when npm-linking modules
Commit 773d2a8b679234f4189f0017f89db64c68d8bb90 in cordova-cli's branch refs/heads/master from Steve Gill
[ https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;h=773d2a8 ]
CB-9858 added --fetch to readme
Commit 18e7377b9ef64c67a80fc8ea754e5bc9d7636be8 in cordova-cli's branch refs/heads/master from Vladimir Kotikov
[ https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;h=18e7377 ]
CB-9858 Fix tests failing due to new --fetch option
Commit 5e2149feec7139ab24439b5f6d22b5763d278242 in cordova-cli's branch refs/heads/master from Steve Gill
[ https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;h=5e2149f ]
CB-9858 added --fetch to cli docs
Github user stevengill closed the pull request at:
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-218000066
Merged
Commit d19111506f870eb92d783976a371996071792c5c in cordova-plugman's branch refs/heads/master from Steve Gill
[ https://git-wip-us.apache.org/repos/asf?p=cordova-plugman.git;h=d191115 ]
CB-9858 added --fetch option to plugman
Commit 96c522f0ce3901a2364c0a648a62e36dc4dd7a45 in cordova-cli's branch refs/heads/master from Steve Gill
[ https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;h=96c522f ]
CB-9858 added --fetch option
Commit 6025a5f23c00284fb3416d8042ff5d738bf03db3 in cordova-lib's branch refs/heads/master from Steve Gill
[ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=6025a5f ]
CB-9858 merging initial fetch work for plugin and platform fetching
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-217091665
LGTM
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-217087853
Made the changes. LMK.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r62104877
— Diff: cordova-fetch/spec/package.json —
@@ -0,0 +1,11 @@
+{
— End diff –
Hmm. Maybe just name it differently. My concern is that presence of another `package.json` might be a bit confusing + break some tools in some scenarios
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r62101897
— Diff: cordova-fetch/spec/package.json —
@@ -0,0 +1,11 @@
+{
— End diff –
I just copy it into temporary tests folder so I can test - - save. Any alternative suggestion? Should I store it somewhere else?
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r62100822
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,234 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ //check if npm is installed
+ isNpmInstalled();
— End diff –
Your right. I originally has it connected to the promise change but removed it during a refactor. I'll fix this!
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r62100600
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,234 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
— End diff –
Thanks! I'll fix that
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r62000656
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,234 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
+ *
+ * @return {String||Promise} Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ //check if npm is installed
+ isNpmInstalled();
+
+ if(dest && target) {
+ //add target to fetchArgs Array
+ fetchArgs.push(target);
+
+ //append node_modules to dest if it doesn't come included
+ if (path.basename(dest) !== 'node_modules') { + dest = path.resolve(path.join(dest, 'node_modules')); + }
+
+ //create dest if it doesn't exist
+ if(!fs.existsSync(dest)) { + shell.mkdir('-p', dest); + }
+
+ } else return Q.reject(new CordovaError('Need to supply a target and destination'));
+
+ //set the directory where npm install will be run
+ opts.cwd = dest;
+
+ //if user added --save flag, pass it to npm install command
+ if(opts.save) { + events.emit('verbose', 'saving'); + fetchArgs.push('--save'); + }
+
+
+ //Grab json object of installed modules before npm install
+ return depls(dest)
+ .then(function(depTree) { + tree1 = depTree; + + //install new module + return superspawn.spawn('npm', fetchArgs, opts); + })
+ .then(function(output) { + //Grab object of installed modules after npm install + return depls(dest); + })
+ .then(function(depTree2) { + var tree2 = depTree2; + + //getJsonDiff will fail if the module already exists in node_modules. + //Need to use trimID in that case. + //This could happen on a platform update. + var id = getJsonDiff(tree1, tree2) || trimID(target); + + return getPath(id, dest); + })
+ .fail(function(err){ + return Q.reject(new CordovaError(err)); + });
+};
+
+
+/*
+ * Takes two JSON objects and returns the key of the new property as a string.
+ * If a module already exists in node_modules, the diff will be blank.
+ * cordova-fetch will use trimID in that case.
+ *
+ * @param {Object}
obj1 json object representing installed modules before latest npm install
+ * @param
+ *
+ * @return {String} String containing the key value of the difference between the two objects
+ *
+ */
+function getJsonDiff(obj1, obj2) {
+ var result = '';
+
+ //regex to filter out peer dependency warnings from result
+ var re = /UNMET PEER DEPENDENCY/;
+
+ for (var key in obj2) {
+ //if it isn't a unmet peer dependency, continue
+ if (key.search(re) === -1) { + if(obj2[key] != obj1[key]) result = key; + }
+ }
+ return result;
+}
+
+/*
+ * Takes the specified target and returns the moduleID
+ * If the git repoName is different than moduleID, then the
+ * output from this function will be incorrect. This is the
+ * backup way to get ID. getJsonDiff is the preferred way to
+ * get the moduleID of the installed module.
+ *
+ * @param {String} target target that was passed into cordova-fetch.
+ * can be moduleID, moduleID@version or gitURL
+ *
+ * @return {String} ID moduleID without version.
+ */
+function trimID(target) {
+ var parts;
+
+ //If GITURL, set target to repo name
+ if (isUrl(target)) { + var re = /.*\/(.*).git/; + parts = target.match(re); + target = parts[1]; + }
+
+ //strip away everything after '@'
+ if(target.indexOf('@') != -1) { + parts = target.split('@'); + target = parts[0]; + }
+
+ return target;
+}
+
+/*
+ * Takes the moduleID and destination and returns an absolute path to the module
+ *
+ * @param {String} id the packageID
+ * @param {String} dest destination of where to fetch the modules
+ *
+ * @return {String||Error} Returns the absolute url for the module or throws a error
+ *
+ */
+
+function getPath(id, dest) {
+ var finalDest = path.resolve(path.join(dest, id));
+
+ //Sanity check it exists
+ if(fs.existsSync(finalDest)){ + return finalDest; + } else return Q.reject(new CordovaError('Failed to get absolute path to installed module'));
+}
+
+
+/*
+ * Checks to see if npm is installed on the users system
+ * @return {Boolean||Error} Returns true or a cordova error.
+ */
+
+function isNpmInstalled() {
+ if(!shell.which('npm')) { + return Q.reject(new CordovaError('"npm" command line tool is not installed: make sure it is accessible on your PATH.')); + }
+ return true;
+}
+
+/*
+ * A function that deletes the target from node_modules and runs npm uninstall
+ *
+ * @param {String} target the packageID
+ * @param {String} dest destination of where to uninstall the module from
+ * @param {Object}
opts [opts=
{save:true}] options to pass to npm uninstall
+ *
+ * @return
Returns a promise with the npm uninstall output or an error.
+ *
+ */
+module.exports.uninstall = function(target, dest, opts) {
+ var fetchArgs = ['uninstall'];
+ opts = opts || {};
+
+ //check if npm is installed on the system
+ isNpmInstalled();
— End diff –
The same issue with not rejecting the promise in case of missing `npm`
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r62000572
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,234 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ //check if npm is installed
+ isNpmInstalled();
— End diff –
Looks like in case if `npm` is not available this will return a promise rejection which is never propagated to the caller. Probably you need to wrap this call and all the code below into top-level promise (similar to #384). This would also protect the code against CB-10519(https://issues.apache.org/jira/browse/CB-10519)
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r61999817
— Diff: cordova-fetch/spec/package.json —
@@ -0,0 +1,11 @@
+{
— End diff –
Do we need a separate `package.json` for specs?
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r61998473
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,234 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
+ *
+ * @return {String||Promise} Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ //check if npm is installed
+ isNpmInstalled();
+
+ if(dest && target) {
+ //add target to fetchArgs Array
+ fetchArgs.push(target);
+
+ //append node_modules to dest if it doesn't come included
+ if (path.basename(dest) !== 'node_modules') { + dest = path.resolve(path.join(dest, 'node_modules')); + }
+
+ //create dest if it doesn't exist
+ if(!fs.existsSync(dest)) { + shell.mkdir('-p', dest); + }
+
+ } else return Q.reject(new CordovaError('Need to supply a target and destination'));
+
+ //set the directory where npm install will be run
+ opts.cwd = dest;
+
+ //if user added --save flag, pass it to npm install command
+ if(opts.save) { + events.emit('verbose', 'saving'); + fetchArgs.push('--save'); + }
+
+
+ //Grab json object of installed modules before npm install
+ return depls(dest)
+ .then(function(depTree) { + tree1 = depTree; + + //install new module + return superspawn.spawn('npm', fetchArgs, opts); + })
+ .then(function(output) { + //Grab object of installed modules after npm install + return depls(dest); + })
+ .then(function(depTree2) { + var tree2 = depTree2; + + //getJsonDiff will fail if the module already exists in node_modules. + //Need to use trimID in that case. + //This could happen on a platform update. + var id = getJsonDiff(tree1, tree2) || trimID(target); + + return getPath(id, dest); + })
+ .fail(function(err){ + return Q.reject(new CordovaError(err)); + });
+};
+
+
+/*
+ * Takes two JSON objects and returns the key of the new property as a string.
+ * If a module already exists in node_modules, the diff will be blank.
+ * cordova-fetch will use trimID in that case.
+ *
+ * @param {Object}
obj1 json object representing installed modules before latest npm install
+ * @param
obj2 json object representing installed modules after latest npm install
+ *
+ * @return
+ *
+ */
+function getJsonDiff(obj1, obj2) {
+ var result = '';
+
+ //regex to filter out peer dependency warnings from result
+ var re = /UNMET PEER DEPENDENCY/;
+
+ for (var key in obj2) {
+ //if it isn't a unmet peer dependency, continue
+ if (key.search(re) === -1) { + if(obj2[key] != obj1[key]) result = key; + }
+ }
+ return result;
+}
+
+/*
+ * Takes the specified target and returns the moduleID
+ * If the git repoName is different than moduleID, then the
+ * output from this function will be incorrect. This is the
+ * backup way to get ID. getJsonDiff is the preferred way to
+ * get the moduleID of the installed module.
+ *
+ * @param {String}
target target that was passed into cordova-fetch.
+ * can be moduleID, moduleID@version or gitURL
+ *
+ * @return
+ */
+function trimID(target) {
+ var parts;
+
+ //If GITURL, set target to repo name
+ if (isUrl(target)) { + var re = /.*\/(.*).git/; + parts = target.match(re); + target = parts[1]; + }
+
+ //strip away everything after '@'
+ if(target.indexOf('@') != -1) { + parts = target.split('@'); + target = parts[0]; + }
+
+ return target;
+}
+
+/*
+ * Takes the moduleID and destination and returns an absolute path to the module
+ *
+ * @param {String}
id the packageID
+ * @param
dest destination of where to fetch the modules
+ *
+ * @return
Returns the absolute url for the module or throws a error
— End diff –
Same here
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r61998292
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,234 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
— End diff –
Nit: the JSDoc notation for multiple types is a bit different - single `|` instead `||` (please see http://usejsdoc.org/tags-type.html).
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-216691951
Hey Everyone,
I think the first implementation of this is ready to hit master behind the --fetch flag. It includes platform and plugin fetching + removing.
Take a look @nikhilkh @purplecabbage @vladimir-kotikov
Needs: apache/cordova-cli#239 and apache/cordova-plugman#87
I will add template fetching next.
I did run into some issues with our cordova tests. The tests finish before the promise resolves. Probably a issue with how we are handling promises in cordova, but could also be related to us using jasmine 1.3 instead of jasmine 2.x. I'll continue to investigate but not a blocker. My manually testing shows that the uninstall command works fine with cordova.
cordova-fetch tests are written using jasmine 2.0. No issues there.
Github user codecov-io commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-215918679
-
- [Current coverage][cc-pull] is *13.19%*
> Merging 407[cc-pull] into [master][cc-base-branch] will increase coverage by *-86.81%*
- [Current coverage][cc-pull] is *13.19%*
```diff
@@ master #407 diff @@
==========================================
Files 69 34 -35
Lines 9878 3261 -6617
Methods 841 525 -316
Messages 0 0
Branches 1271 636 -635
==========================================
- Hits 9878 430 -9448
- Misses 0 2831 +2831
Partials 0 0
```
1. 3 files (not in diff) in `cordova-lib/src/util` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F7574696C)
- Misses `+46`
- Hits `-86`
1. 4 files (not in diff) in `...dova-lib/src/plugman` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F706C75676D616E) - Misses `+219`
- Hits `-437`
1. 2 files (not in diff) in `...va-lib/src/platforms` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F706C6174666F726D73) - Misses `+234`
- Hits `-444`
1. 3 files (not in diff) in `...ordova-lib/src/hooks` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F686F6F6B73) - Misses `+159`
- Hits `-315`
1. 2 files (not in diff) in `...etadata/parserhelper` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F76612F6D657461646174612F70617273657268656C706572) - Misses `+24`
- Hits `-84`
1. 9 files (not in diff) in `...src/cordova/metadata` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F76612F6D65746164617461) - Misses `+859`
- Hits `-1578`
1. 7 files (not in diff) in `...dova-lib/src/cordova` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F7661) - Misses `+440`
- Hits `-855`
1. 1 files (not in diff) in `cordova-lib/src` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F737263) - Misses `+22`
- Hits `-42`
1. 2 files in `...dova-lib/src/cordova` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F7661) - Misses `+716`
- Hits `-1327`
1. 1 files in `cordova-lib/src` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F737263) - Misses `+73`
- Hits `-136`
> Powered by [Codecov](https://codecov.io?src=pr). Last updated by [5867657...b61258f][cc-compare]
[cc-base-branch]: https://codecov.io/gh/apache/cordova-lib/branch/master?src=pr
[cc-compare]: https://codecov.io/gh/apache/cordova-lib/compare/5867657d989b136702fe2f14172661ca489573ed...b61258fd4d50e6f1b71b55a6200a4e73b7d1857f
[cc-pull]: https://codecov.io/gh/apache/cordova-lib/pull/407?src=pr
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r60995427
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,226 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(dest && target) {
+ //add target to fetchArgs Array
+ fetchArgs.push(target);
+
+ //append node_modules to dest if it doesn't come included
+ if (path.basename(dest) !== 'node_modules')
+
+ //create dest if it doesn't exist
+ if(!fs.existsSync(dest))
+
+ } else return Q.reject(new CordovaError('Need to supply a target and destination'));
+
+ //set the directory where npm install will be run
+ opts.cwd = dest;
+
+ //if user added --save flag, pass it to npm install command
+ if(opts.save)
+
+ //check if npm is installed
+ isNpmInstalled();
+
+ //Grab json object of installed modules before npm install
+ return depls(dest)
+ .then(function(depTree) {
+ tree1 = depTree;
+
+ //install new module
+ return superspawn.spawn('npm', fetchArgs, opts);
— End diff –
I don't think it is necessary. It is running `npm install` for modules from registry or git url. `devDependencies` shouldn't be getting installed in this scenario regardless of `--production`
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r60995226
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,226 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(dest && target) {
+ //add target to fetchArgs Array
+ fetchArgs.push(target);
+
+ //append node_modules to dest if it doesn't come included
+ if (path.basename(dest) !== 'node_modules')
+
+ //create dest if it doesn't exist
+ if(!fs.existsSync(dest))
+
+ } else return Q.reject(new CordovaError('Need to supply a target and destination'));
+
+ //set the directory where npm install will be run
+ opts.cwd = dest;
+
+ //if user added --save flag, pass it to npm install command
+ if(opts.save)
+
+ //check if npm is installed
+ isNpmInstalled();
— End diff –
True. I'll move it up
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r60899092
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,226 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(dest && target) {
+ //add target to fetchArgs Array
+ fetchArgs.push(target);
+
+ //append node_modules to dest if it doesn't come included
+ if (path.basename(dest) !== 'node_modules')
+
+ //create dest if it doesn't exist
+ if(!fs.existsSync(dest))
+
+ } else return Q.reject(new CordovaError('Need to supply a target and destination'));
+
+ //set the directory where npm install will be run
+ opts.cwd = dest;
+
+ //if user added --save flag, pass it to npm install command
+ if(opts.save)
+
+ //check if npm is installed
+ isNpmInstalled();
+
+ //Grab json object of installed modules before npm install
+ return depls(dest)
+ .then(function(depTree) {
+ tree1 = depTree;
+
+ //install new module
+ return superspawn.spawn('npm', fetchArgs, opts);
— End diff –
Perhaps it is worth to add `--production` flag to avoid installing `devDependencies`
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r60898584
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,226 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(dest && target) {
+ //add target to fetchArgs Array
+ fetchArgs.push(target);
+
+ //append node_modules to dest if it doesn't come included
+ if (path.basename(dest) !== 'node_modules')
+
+ //create dest if it doesn't exist
+ if(!fs.existsSync(dest))
+
+ } else return Q.reject(new CordovaError('Need to supply a target and destination'));
+
+ //set the directory where npm install will be run
+ opts.cwd = dest;
+
+ //if user added --save flag, pass it to npm install command
+ if(opts.save)
+
+ //check if npm is installed
+ isNpmInstalled();
— End diff –
Probably it makes sense to check this at the very beginning of the method
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-199661470
Plugman PR adding --fetch flag support https://github.com/apache/cordova-plugman/pull/87
GitHub user stevengill opened a pull request:
https://github.com/apache/cordova-plugman/pull/87
CB-9858 added --fetch option to plugman
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/stevengill/cordova-plugman CB-9858
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugman/pull/87.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 #87
commit 5af3180aa03c1f37ef07d343ee9d5385d1a4248d
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-03-22T06:41:27Z
CB-9858 added --fetch option to plugman
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r55578321
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,176 @@
+/**
+ 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 Q = require('q');
— End diff –
You are correct. Looks like I had it locally but didn't save it. I'll update this when I land and get to my computer.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r55575046
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,176 @@
+/**
+ 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 Q = require('q');
— End diff –
I was trying to give it a spin but I get this error:
λ cordova platform add android --verbose
Error: Cannot find module 'q'
at Function.Module._resolveFilename (module.js:336:15)
at Function.Module._load (module.js:286:25)
at Module.require (module.js:365:17)
at require (module.js:384:17)
at Object.<anonymous> (D:\cordova\cordova-lib\cordova-fetch\index.js:20:9)
Looks like the package.json for this module needs more dependencies.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r55567900
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,176 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(!shell.which('npm')) {
— End diff –
https://github.com/apache/cordova-android/blob/master/bin/lib/check_reqs.js#L97. We end up modifying process.env["PATH"] with probable directories for JAVA_HOME/ANDROID_HOME before invoking superspawn.
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-194441672
To-do: add cordova-lib tests for this
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r55480177
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,176 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(!shell.which('npm'))
+
+ if(dest && target) {
+ //add target to fetchArgs Array
+ fetchArgs.push(target);
+
+ //append node_modules to dest if it doesn't come included
+ if (path.basename(dest) !== 'node_modules')
+
+ //create dest if it doesn't exist
+ if(!fs.existsSync(dest))
+
+ } else return Q.reject(new CordovaError('Need to supply a target and destination'));
+
+ //set the directory where npm install will be run
+ opts.cwd = dest;
— End diff –
Yup! Pass everything to superspawn that fetch was passed. Add the cwd to opts though.
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r55480086
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,176 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(!shell.which('npm')) {
— End diff –
Sounds like a good idea! Do you have any examples of doing this with Java_home I could use as a basis for this? I pretty much mickimed how our current fetch code looks for git.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r55473246
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,176 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(!shell.which('npm'))
+
+ if(dest && target) {
+ //add target to fetchArgs Array
+ fetchArgs.push(target);
+
+ //append node_modules to dest if it doesn't come included
+ if (path.basename(dest) !== 'node_modules')
+
+ //create dest if it doesn't exist
+ if(!fs.existsSync(dest))
+
+ } else return Q.reject(new CordovaError('Need to supply a target and destination'));
+
+ //set the directory where npm install will be run
+ opts.cwd = dest;
— End diff –
Are the `opts` for superspawn the same as the `opts` passed to this function?
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/407#discussion_r55472886
— Diff: cordova-fetch/index.js —
@@ -0,0 +1,176 @@
+/**
+ 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 Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/*
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param
+ * @param {String}
dest destination of where to install the module
+ * @param
opts [opts=
{save:true}] options to pass to fetch module
+ *
+ * @return
Returns string of the absolute path to the installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+ var fetchArgs = ['install'];
+ opts = opts || {};
+ var tree1;
+
+ if(!shell.which('npm')) {
— End diff –
Judging from previous experience with our tools not being able to find ANDROID_HOME/JAVA_HOME - this might become an error message that will affect new people. We should consider doing a best effort to find the npm installed on the system if `which` can't find it. There are some well known places on Windows/Mac where this gets installed.
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-194065776
Needs https://github.com/apache/cordova-cli/pull/239.
Discussion at https://github.com/cordova/cordova-discuss/pull/33
GitHub user stevengill opened a pull request:
https://github.com/apache/cordova-cli/pull/239
CB-9858 added --fetch option
Needs https://github.com/apache/cordova-lib/pull/407
Part of https://github.com/cordova/cordova-discuss/pull/33
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/stevengill/cordova-cli CB-9858
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-cli/pull/239.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 #239
commit ac24fde7dba444aabc143ff04fe9f5d7ed8872e4
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-03-04T00:32:37Z
CB-9858 added --fetch option
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/407#issuecomment-194059148
I decided to write the tests in node-tap. I am now regretting that decision a bit. Tests randomly timeout and fail. Doesn't happen in actual use.
GitHub user stevengill opened a pull request:
https://github.com/apache/cordova-lib/pull/407
CB-9858 Cordova Fetch Work
This is not ready to merge yet! Opening the PR so it is easy to manage diffs and take feedback
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/stevengill/cordova-lib CB-9858
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/407.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 #407
commit 378b876bdc7aa4e80450e3a3e3d45f02891fc50d
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-02-06T02:13:51Z
initial commit for cordova-fetch
commit 2206b1e2d8976dd2d0b21adf7ee4042b5eeec462
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-02-09T02:24:41Z
implemented fetch module and added tests
commit 1e3853d0e768dd9a7af63e026643601ac900a5d6
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-02-24T02:03:40Z
CB-9858: completed first implementation of cordova-fetch module
commit de15f74e7872fc249346faf152e63bb061036a5f
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-03-02T02:11:08Z
CB-9858 updated tests
commit 9a1373571b082f1a8e6e643306f89c204ea9fcd5
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-03-04T01:31:49Z
CB-9858 adding platforms can now use cordova-fetch with --fetch flag
commit 47ae946562058fb7563685d1fdee61d7b7fcbf9f
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-03-09T01:10:17Z
CB-9859 added trimID to handle cordova platform update usecase
commit 1fa38af3c3b6a35cb84d7e01b651b5aafef33e28
Author: Steve Gill <stevengill97@gmail.com>
Date: 2016-03-09T01:30:22Z
CB-9858 cordova platform remove --fetch deletes module from node_modules directory
Github user stevengill closed the pull request at:
https://github.com/apache/cordova-cli/pull/239