You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ra...@apache.org on 2018/06/12 20:11:45 UTC

[cordova-fetch] branch master updated: Simplify installation location retrieval (#18)

This is an automated email from the ASF dual-hosted git repository.

raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-fetch.git


The following commit(s) were added to refs/heads/master by this push:
     new 307638d  Simplify installation location retrieval (#18)
307638d is described below

commit 307638d63604eb283b581c01a2e702ce39249473
Author: Raphael von der Grün <ra...@gmail.com>
AuthorDate: Tue Jun 12 22:11:40 2018 +0200

    Simplify installation location retrieval (#18)
    
    This replaces the whole existing logic to determine the path of an installed package with a new one.
    
    Instead of a lot of guessing we now read the output of npm. Unfortunately we have to parse the human readable output since the information we need (which package was just installed) is not reliably extractable from the output generated when passing --json or --parseable.
    
    Apart from a tremendous simplification of the existing code, with this change we now transparently support all spec formats that npm supports too.
    
    The removal of getPath is a possibly breaking change. The function was neither documented nor used in any apache/cordova-* repositories though.
    
    This supports all npm versions from 6 down to 3 (which are all that are bundled with supported node versions).
---
 index.js                | 181 +++++-------------------------------------------
 package.json            |   4 --
 spec/fetch-unit.spec.js |   3 +-
 3 files changed, 20 insertions(+), 168 deletions(-)

diff --git a/index.js b/index.js
index 6c8ff81..5a666ef 100644
--- a/index.js
+++ b/index.js
@@ -19,14 +19,9 @@ 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 url = require('url');
 var path = require('path');
 var fs = require('fs');
 var CordovaError = require('cordova-common').CordovaError;
-var isUrl = require('is-url');
-var isGitUrl = require('is-git-url');
-var hostedGitInfo = require('hosted-git-info');
 
 /*
  * A function that npm installs a module from npm or a git url
@@ -40,7 +35,6 @@ var hostedGitInfo = require('hosted-git-info');
  */
 module.exports = function (target, dest, opts = {}) {
     var fetchArgs = opts.link ? ['link'] : ['install'];
-    var tree1;
     var nodeModulesDir = dest;
 
     // check if npm is installed
@@ -62,8 +56,8 @@ module.exports = function (target, dest, opts = {}) {
 
             // set the directory where npm install will be run
             opts.cwd = dest;
-            // npm should use production by default when install is npm run
 
+            // npm should use production by default when install is npm run
             if ((opts.production) || (opts.production === undefined)) {
                 fetchArgs.push('--production');
                 opts.production = true;
@@ -79,172 +73,35 @@ module.exports = function (target, dest, opts = {}) {
             } else {
                 fetchArgs.push('--no-save');
             }
-            // Grab json object of installed modules before npm install
-            return depls(nodeModulesDir);
         })
-        .then(function (depTree) {
-            tree1 = depTree;
+        .then(function () {
             // install new module
             return superspawn.spawn('npm', fetchArgs, opts);
         })
-        .then(function (output) {
-            // Grab object of installed modules after npm install
-            return depls(nodeModulesDir);
-        })
-        .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 module.exports.getPath(id, nodeModulesDir, target);
-        })
+        .then(extractPackageName)
+        .then(pkgName => path.resolve(nodeModulesDir, pkgName))
         .catch(function (err) {
             throw 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.push(key);
-            }
-        }
-    }
-    if (result.length > 1) {
-        // something went wrong if we have more than one module installed at a time
-        return false;
-    }
-    // only return the first element
-    return result[0];
-}
-/*
- * 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, gitURL or relative path (file:relative/path)
- *
- * @return {String} ID       moduleID without version.
- */
-function trimID (target) {
-    var parts;
-    // If GITURL, set target to repo name
-    var gitInfo = hostedGitInfo.fromUrl(target);
-    if (gitInfo) {
-        target = gitInfo.project;
-    } else if (isUrl(target) || isGitUrl(target)) {
-        // strip away .git and everything that follows
-        var strippedTarget = target.split('.git');
-        var re = /.*\/(.*)/;
-        // Grabs everything in url after last `/`
-        parts = strippedTarget[0].match(re);
-
-        target = parts[1];
-    }
-
-    // If local path exists, try to get plugin id from package.json or set target to final directory
-    if (target.startsWith('file:')) {
-        // If target starts with file: prefix, strip it
-        target = target.substring(5);
-    }
-
-    if (fs.existsSync(target)) {
-        var pluginId;
-        var pkgJsonPath = path.join(target, 'package.json');
-
-        if (fs.existsSync(pkgJsonPath)) {
-            pluginId = JSON.parse(fs.readFileSync(pkgJsonPath)).name;
-        }
-
-        target = pluginId || path.basename(target);
-    }
-
-    // strip away everything after '@'
-    // also support scoped packages
-    if (target.indexOf('@') !== -1) {
-        parts = target.split('@');
-        if (parts.length > 1 && parts[0] === '') {
-            // scoped package
-            target = '@' + parts[1];
-        } else {
-            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, target) {
-    var destination = path.resolve(path.join(dest, id));
-    var finalDest = fs.existsSync(destination) ? destination : searchDirForTarget(dest, target);
-
-    if (!finalDest) {
-        throw new CordovaError('Failed to get absolute path to installed module');
-    }
-
-    return finalDest;
-}
-
-module.exports.getPath = getPath;
-/*
- * Make an additional search in destination folder using repository.url property from package.json
- *
- * @param {String} dest     destination of where to fetch the modules
- * @param {String} target   target that was passed into cordova-fetch. can be moduleID, moduleID@version or gitURL
- *
- * @return {String}         Returns the absolute url for the module or null
- *
- */
-
-function searchDirForTarget (dest, target) {
-    if (!isUrl(target)) {
-        return;
+function extractPackageName (npmInstallOutput) {
+    const lines = npmInstallOutput.split('\n');
+    let spec;
+    if (lines[0].startsWith('+ ')) {
+        // npm >= 5
+        spec = lines[0].slice(2);
+    } else if (lines[1].startsWith('└─') || lines[1].startsWith('`-')) {
+        // 3 <= npm <= 4
+        spec = lines[1].slice(4);
+    } else {
+        throw new CordovaError('Could not determine package name from output:\n' + npmInstallOutput);
     }
 
-    var targetPathname = url.parse(target).pathname;
-
-    var pkgJsonPath = fs.readdirSync(dest).map(function (dir) {
-        return path.join(dest, dir, 'package.json');
-    })
-        .filter(fs.existsSync)
-        .find(function (pkgJsonPath) {
-            var repo = JSON.parse(fs.readFileSync(pkgJsonPath)).repository;
-            return repo && url.parse(repo.url).pathname === targetPathname;
-        });
-
-    return pkgJsonPath && path.dirname(pkgJsonPath);
+    // Strip version from spec
+    const parts = spec.split('@');
+    const isScoped = parts.length > 1 && parts[0] === '';
+    return isScoped ? '@' + parts[1] : parts[0];
 }
 
 /*
diff --git a/package.json b/package.json
index 04ede01..c828575 100644
--- a/package.json
+++ b/package.json
@@ -22,10 +22,6 @@
   },
   "dependencies": {
     "cordova-common": "^2.2.0",
-    "dependency-ls": "^1.1.0",
-    "hosted-git-info": "^2.5.0",
-    "is-url": "^1.2.1",
-    "is-git-url": "^1.0.0",
     "q": "^1.4.1",
     "shelljs": "^0.7.0"
   },
diff --git a/spec/fetch-unit.spec.js b/spec/fetch-unit.spec.js
index 107c249..18449ed 100644
--- a/spec/fetch-unit.spec.js
+++ b/spec/fetch-unit.spec.js
@@ -23,11 +23,10 @@ var superspawn = require('cordova-common').superspawn;
 
 describe('unit tests for index.js', function () {
     beforeEach(function () {
-        spyOn(superspawn, 'spawn').and.returnValue(true);
+        spyOn(superspawn, 'spawn').and.returnValue('+ foo@1.2.3');
         spyOn(shell, 'mkdir').and.returnValue(true);
         spyOn(shell, 'which').and.returnValue(Promise.resolve());
         spyOn(fetch, 'isNpmInstalled').and.returnValue(Promise.resolve());
-        spyOn(fetch, 'getPath').and.returnValue('some/path');
         spyOn(fs, 'existsSync').and.returnValue(false);
     });
 

-- 
To stop receiving notification emails like this one, please contact
raphinesse@apache.org.

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org