Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.0.0
    • Component/s: CordovaLib
    • Labels:

      Issue Links

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        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


        Show
        githubbot ASF GitHub Bot added a comment - 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
        Hide
        githubbot ASF GitHub Bot added a comment -

        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.

        Show
        githubbot ASF GitHub Bot added a comment - 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.
        Hide
        githubbot ASF GitHub Bot added a comment -

        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


        Show
        githubbot ASF GitHub Bot added a comment - 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
        Show
        githubbot ASF GitHub Bot added a comment - 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
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + 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.

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + 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.
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + if(!shell.which('npm'))

        { + return Q.reject(new CordovaError('"npm" command line tool is not installed: make sure it is accessible on your PATH.')); + }

        +
        + 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;
        — End diff –

        Are the `opts` for superspawn the same as the `opts` passed to this function?

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + if(!shell.which('npm')) { + return Q.reject(new CordovaError('"npm" command line tool is not installed: make sure it is accessible on your PATH.')); + } + + 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; — End diff – Are the `opts` for superspawn the same as the `opts` passed to this function?
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + 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.

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + 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.
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + if(!shell.which('npm'))

        { + return Q.reject(new CordovaError('"npm" command line tool is not installed: make sure it is accessible on your PATH.')); + }

        +
        + 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;
        — End diff –

        Yup! Pass everything to superspawn that fetch was passed. Add the cwd to opts though.

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + if(!shell.which('npm')) { + return Q.reject(new CordovaError('"npm" command line tool is not installed: make sure it is accessible on your PATH.')); + } + + 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; — End diff – Yup! Pass everything to superspawn that fetch was passed. Add the cwd to opts though.
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        Show
        githubbot ASF GitHub Bot added a comment - 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
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + 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.

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + 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.
        Hide
        githubbot ASF GitHub Bot added a comment -

        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.

        Show
        githubbot ASF GitHub Bot added a comment - 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.
        Hide
        githubbot ASF GitHub Bot added a comment -

        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.

        Show
        githubbot ASF GitHub Bot added a comment - 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.
        Hide
        githubbot ASF GitHub Bot added a comment -

        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


        Show
        githubbot ASF GitHub Bot added a comment - 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
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        Show
        githubbot ASF GitHub Bot added a comment - 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
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + 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'); + }


        +
        + //check if npm is installed
        + isNpmInstalled();
        — End diff –

        Probably it makes sense to check this at the very beginning of the method

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + 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'); + } + + //check if npm is installed + isNpmInstalled(); — End diff – Probably it makes sense to check this at the very beginning of the method
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + 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'); + }


        +
        + //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`

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + 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'); + } + + //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`
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + 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'); + }


        +
        + //check if npm is installed
        + isNpmInstalled();
        — End diff –

        True. I'll move it up

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + 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'); + } + + //check if npm is installed + isNpmInstalled(); — End diff – True. I'll move it up
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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;
        +
        + 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'); + }


        +
        + //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`

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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; + + 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'); + } + + //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`
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user codecov-io commented on the pull request:

        https://github.com/apache/cordova-lib/pull/407#issuecomment-215918679

          1. [Current coverage][cc-pull] is *13.19%*
            > Merging 407[cc-pull] into [master][cc-base-branch] will increase coverage by *-86.81%*

        ```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)

        > 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

        Show
        githubbot ASF GitHub Bot added a comment - 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% * ```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
        Hide
        githubbot ASF GitHub Bot added a comment -

        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.

        Show
        githubbot ASF GitHub Bot added a comment - 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.
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @return

        {String||Promise}

        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).

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @return {String||Promise} 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 ).
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object} opts [opts={save:true}] options to pass to fetch module
        + *
        + * @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

        {Object}

        obj2 json object representing installed modules after latest npm install
        + *
        + * @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
        — End diff –

        Same here

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts={save:true}] options to pass to fetch module + * + * @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 {Object} obj2 json object representing installed modules after latest npm install + * + * @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 — End diff – Same here
        Hide
        githubbot ASF GitHub Bot added a comment -

        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?

        Show
        githubbot ASF GitHub Bot added a comment - 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?
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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();
        — 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)

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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(); — 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 )
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object} opts [opts={save:true}] options to pass to fetch module
        + *
        + * @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

        {Object} obj2 json object representing installed modules after latest npm install
        + *
        + * @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

        {Promise||Error}

        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`

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts={save:true}] options to pass to fetch module + * + * @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 {Object} obj2 json object representing installed modules after latest npm install + * + * @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 {Promise||Error} 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`
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @return

        {String||Promise}

        Returns string of the absolute path to the installed module.
        — End diff –

        Thanks! I'll fix that

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @return {String||Promise} Returns string of the absolute path to the installed module. — End diff – Thanks! I'll fix that
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        {String} target the packageID or git url
        + * @param {String}

        dest destination of where to install the module
        + * @param

        {Object}

        opts [opts=

        {save:true}

        ] options to pass to fetch module
        + *
        + * @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();
        — End diff –

        Your right. I originally has it connected to the promise change but removed it during a refactor. I'll fix this!

        Show
        githubbot ASF GitHub Bot added a comment - 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 {String} target the packageID or git url + * @param {String} dest destination of where to install the module + * @param {Object} opts [opts= {save:true} ] options to pass to fetch module + * + * @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(); — End diff – Your right. I originally has it connected to the promise change but removed it during a refactor. I'll fix this!
        Hide
        githubbot ASF GitHub Bot added a comment -

        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?

        Show
        githubbot ASF GitHub Bot added a comment - 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?
        Hide
        githubbot ASF GitHub Bot added a comment -

        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

        Show
        githubbot ASF GitHub Bot added a comment - 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
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user stevengill commented on the pull request:

        https://github.com/apache/cordova-lib/pull/407#issuecomment-217087853

        Made the changes. LMK.

        Show
        githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the pull request: https://github.com/apache/cordova-lib/pull/407#issuecomment-217087853 Made the changes. LMK.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user vladimir-kotikov commented on the pull request:

        https://github.com/apache/cordova-lib/pull/407#issuecomment-217091665

        LGTM

        Show
        githubbot ASF GitHub Bot added a comment - Github user vladimir-kotikov commented on the pull request: https://github.com/apache/cordova-lib/pull/407#issuecomment-217091665 LGTM
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user stevengill commented on the pull request:

        https://github.com/apache/cordova-lib/pull/407#issuecomment-218000066

        Merged

        Show
        githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the pull request: https://github.com/apache/cordova-lib/pull/407#issuecomment-218000066 Merged
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user stevengill closed the pull request at:

        https://github.com/apache/cordova-lib/pull/407

        Show
        githubbot ASF GitHub Bot added a comment - Github user stevengill closed the pull request at: https://github.com/apache/cordova-lib/pull/407
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        jira-bot ASF subversion and git services added a comment -

        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

        Show
        jira-bot ASF subversion and git services added a comment - 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
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user purplecabbage commented on the issue:

        https://github.com/apache/cordova-cli/pull/239

        Is this still valid? Needs a rebase at least ...

        Show
        githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on the issue: https://github.com/apache/cordova-cli/pull/239 Is this still valid? Needs a rebase at least ...
        Hide
        githubbot ASF GitHub Bot added a comment -

        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.

        Show
        githubbot ASF GitHub Bot added a comment - 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.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user stevengill closed the pull request at:

        https://github.com/apache/cordova-cli/pull/239

        Show
        githubbot ASF GitHub Bot added a comment - Github user stevengill closed the pull request at: https://github.com/apache/cordova-cli/pull/239

          People

          • Assignee:
            stevegill Steve Gill
            Reporter:
            stevegill Steve Gill
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development