use npm to download extension dependencies after installing from registry (#10602)

zaggino · swmitra · commit 74c5306e51ff · 2017-02-15T09:16:47.000+05:30
* use npm to download extension dependencies

add shrinkwrap

* exec npm instead of using internal API

refactor

refactor 2

* update npm

* initialize exitCode

* move fail and finish blocks out

* do not use ternary ifs

* little fix

* update npm-shrinkwrap

* move npm installation to the package extraction step

* consistent function style

* add proxy support

* npm-shrinkwrap.json
diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json
diff --git a/package.json b/package.json
@@ -16,6 +16,7 @@
         "anymatch": "1.3.0",
         "chokidar": "1.6.0",
         "lodash": "4.15.0",
+        "npm": "3.10.9",
         "ws": "~0.4.31"
     },
     "devDependencies": {
diff --git a/src/config.json b/src/config.json
@@ -39,6 +39,7 @@
         "anymatch": "1.3.0",
         "chokidar": "1.6.0",
         "lodash": "4.15.0",
+        "npm": "3.10.9",
         "ws": "~0.4.31"
     },
     "devDependencies": {
@@ -79,4 +80,4 @@
             "url": "https://github.com/adobe/brackets/blob/master/LICENSE"
         }
     ]
-}
+}
diff --git a/src/extensibility/Package.js b/src/extensibility/Package.js
@@ -105,7 +105,12 @@ define(function (require, exports, module) {
     function validate(path, options) {
         return _extensionManagerCall(function (extensionManager) {
             var d = new $.Deferred();
-
+            
+            // make sure proxy is attached to options before calling validate
+            // so npm can use it in the domain
+            options = options || {};
+            options.proxy = PreferencesManager.get("proxy");
+            
             extensionManager.validate(path, options)
                 .done(function (result) {
                     d.resolve({
@@ -157,7 +162,8 @@ define(function (require, exports, module) {
                 disabledDirectory: disabledDirectory,
                 systemExtensionDirectory: systemDirectory,
                 apiVersion: brackets.metadata.apiVersion,
-                nameHint: nameHint
+                nameHint: nameHint,
+                proxy: PreferencesManager.get("proxy")
             })
                 .done(function (result) {
                     result.keepFile = false;
diff --git a/src/extensibility/node/ExtensionManagerDomain.js b/src/extensibility/node/ExtensionManagerDomain.js
@@ -90,6 +90,20 @@ function _removeFailedInstallation(installDirectory) {
 function _performInstall(packagePath, installDirectory, validationResult, callback) {
     validationResult.installedTo = installDirectory;
 
+    function fail(err) {
+        _removeFailedInstallation(installDirectory);
+        callback(err, null);
+    }
+
+    function finish() {
+        // The status may have already been set previously (as in the
+        // DISABLED case.
+        if (!validationResult.installationStatus) {
+            validationResult.installationStatus = Statuses.INSTALLED;
+        }
+        callback(null, validationResult);
+    }
+
     fs.mkdirs(installDirectory, function (err) {
         if (err) {
             callback(err);
@@ -99,16 +113,9 @@ function _performInstall(packagePath, installDirectory, validationResult, callba
 
         fs.copy(sourceDir, installDirectory, function (err) {
             if (err) {
-                _removeFailedInstallation(installDirectory);
-                callback(err, null);
-            } else {
-                // The status may have already been set previously (as in the
-                // DISABLED case.
-                if (!validationResult.installationStatus) {
-                    validationResult.installationStatus = Statuses.INSTALLED;
-                }
-                callback(null, validationResult);
+                return fail(err);
             }
+            finish();
         });
     });
 }
@@ -213,7 +220,7 @@ function _cmdInstall(packagePath, destinationDirectory, options, callback, pCall
         return;
     }
 
-    var validateCallback = function (err, validationResult) {
+    function validateCallback(err, validationResult) {
         validationResult.localPath = packagePath;
 
         // This is a wrapper for the callback that will delete the temporary
@@ -300,9 +307,9 @@ function _cmdInstall(packagePath, destinationDirectory, options, callback, pCall
             validationResult.disabledReason = null;
             _performInstall(packagePath, installDirectory, validationResult, deleteTempAndCallback);
         }
-    };
+    }
 
-    validate(packagePath, {}, validateCallback);
+    validate(packagePath, options, validateCallback);
 }
 
 /**
@@ -496,7 +503,7 @@ function init(domainManager) {
             description: "absolute filesystem path where this extension should be installed"
         }, {
             name: "options",
-            type: "{disabledDirectory: !string, apiVersion: !string, nameHint: ?string, systemExtensionDirectory: !string}",
+            type: "{disabledDirectory: !string, apiVersion: !string, nameHint: ?string, systemExtensionDirectory: !string, proxy: ?string}",
             description: "installation options: disabledDirectory should be set so that extensions can be installed disabled."
         }],
         [{
diff --git a/src/extensibility/node/npm-installer.js b/src/extensibility/node/npm-installer.js
@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2013 - present Adobe Systems Incorporated. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+/* eslint-env node */
+
+"use strict";
+
+var fs    = require("fs-extra"),
+    path  = require("path"),
+    spawn = require("child_process").spawn;
+
+var Errors = {
+    NPM_INSTALL_FAILED: "NPM_INSTALL_FAILED"
+};
+
+/**
+ * Private function to run "npm install --production" command in the extension directory.
+ *
+ * @param {string} installDirectory Directory to remove
+ * @param {function} callback NodeJS style callback to call after finish
+ */
+function _performNpmInstall(installDirectory, npmOptions, callback) {
+    var npmPath = path.resolve(path.dirname(require.resolve("npm")), "..", "bin", "npm-cli.js");
+    var args = [npmPath, "install"];
+    
+    // npmOptions can contain additional args like { "production": true, "proxy": "https://127.0.0.1:8888" }
+    Object.keys(npmOptions).forEach(function (key) {
+        var value = npmOptions[key];
+        if (value === true) {
+            args.push("--" + key);
+        } else if (typeof value === "string" && value.length > 0) {
+            args.push("--" + key, value);
+        }
+    });
+    
+    console.log("running npm " + args.slice(1).join(" ") + " in " + installDirectory);
+
+    var child = spawn(process.execPath, args, { cwd: installDirectory });
+
+    child.on("error", function (err) {
+        return callback(err);
+    });
+
+    var stdout = [];
+    child.stdout.addListener("data", function (buffer) {
+        stdout.push(buffer);
+    });
+
+    var stderr = [];
+    child.stderr.addListener("data", function (buffer) {
+        stderr.push(buffer);
+    });
+
+    var exitCode = 0;
+    child.addListener("exit", function (code) {
+        exitCode = code;
+    });
+
+    child.addListener("close", function () {
+        stderr = Buffer.concat(stderr).toString();
+        stdout = Buffer.concat(stdout).toString();
+        if (exitCode > 0) {
+            console.error("npm-stderr: " + stderr);
+            return callback(new Error(stderr));
+        }
+        if (stderr) {
+            console.warn("npm-stderr: " + stderr);
+        }
+        console.log("npm-stdout: " + stdout);
+        return callback();
+    });
+
+    child.stdin.end();
+}
+
+/**
+ * Checks package.json of the extracted extension for npm dependencies
+ * and runs npm install when required.
+ * @param {Object} validationResult return value of the validation procedure
+ * @param {Function} callback function to be called after the end of validation procedure
+ */
+function performNpmInstallIfRequired(npmOptions, validationResult, callback) {
+
+    function finish() {
+        callback(null, validationResult);
+    }
+
+    var installDirectory = path.join(validationResult.extractDir, validationResult.commonPrefix);
+    var packageJson;
+
+    try {
+        packageJson = fs.readJsonSync(path.join(installDirectory, "package.json"));
+    } catch (e) {
+        packageJson = null;
+    }
+
+    if (!packageJson || !packageJson.dependencies) {
+        return finish();
+    }
+
+    _performNpmInstall(installDirectory, npmOptions, function (err) {
+        if (err) {
+            validationResult.errors.push([Errors.NPM_INSTALL_FAILED, err.toString()]);
+        }
+        finish();
+    });
+}
+
+exports.performNpmInstallIfRequired = performNpmInstallIfRequired;
diff --git a/src/extensibility/node/package-validator.js b/src/extensibility/node/package-validator.js
@@ -26,11 +26,12 @@
 
 "use strict";
 
-var DecompressZip = require("decompress-zip"),
-    semver        = require("semver"),
-    path          = require("path"),
-    temp          = require("temp"),
-    fs            = require("fs-extra");
+var DecompressZip               = require("decompress-zip"),
+    semver                      = require("semver"),
+    path                        = require("path"),
+    temp                        = require("temp"),
+    fs                          = require("fs-extra"),
+    performNpmInstallIfRequired = require("./npm-installer").performNpmInstallIfRequired;
 
 // Track and cleanup files at exit
 temp.track();
@@ -293,12 +294,16 @@ function extractAndValidateFiles(zipPath, extractDir, options, callback) {
                 if (!isTheme && !fs.existsSync(mainJS)) {
                     errors.push([Errors.MISSING_MAIN, zipPath, mainJS]);
                 }
-                callback(null, {
+
+                performNpmInstallIfRequired({
+                    production: true,
+                    proxy: options.proxy
+                }, {
                     errors: errors,
                     metadata: metadata,
                     commonPrefix: commonPrefix,
                     extractDir: extractDir
-                });
+                }, callback);
             });
         });
     });
@@ -327,7 +332,7 @@ function extractAndValidateFiles(zipPath, extractDir, options, callback) {
  * read successfully from package.json in the zip file.
  *
  * @param {string} path Absolute path to the package zip file
- * @param {{requirePackageJSON: ?boolean, disallowedWords: ?Array.<string>}} options for validation
+ * @param {{requirePackageJSON: ?boolean, disallowedWords: ?Array.<string>, proxy: ?<string>}} options for validation
  * @param {function} callback (err, result)
  */
 function validate(path, options, callback) {
diff --git a/src/extensibility/node/spec/Installation.spec.js b/src/extensibility/node/spec/Installation.spec.js
@@ -55,7 +55,8 @@ var basicValidExtension       = path.join(testFilesDirectory, "basic-valid-exten
     missingPackageJSON        = path.join(testFilesDirectory, "missing-package-json.zip"),
     missingPackageJSONUpdate  = path.join(testFilesDirectory, "missing-package-json-update.zip"),
     missingPackageJSONRenamed = path.join(testFilesDirectory, "added-package-json-test", "missing-package-json.zip"),
-    withSymlink               = path.join(testFilesDirectory, "with-symlink.zip");
+    withSymlink               = path.join(testFilesDirectory, "with-symlink.zip"),
+    withNpmDependencies       = path.join(testFilesDirectory, "with-npm-dependencies.zip");
 
 
 describe("Package Installation", function () {
@@ -324,4 +325,30 @@ describe("Package Installation", function () {
             done();
         });
     });
+    
+    it("should download npm dependencies when present", function (done) {
+        ExtensionsDomain._cmdInstall(withNpmDependencies, installDirectory, standardOptions, function (err, result) {
+            expect(err).toBeNull();
+            expect(result.errors.length).toEqual(0);
+            expect(fs.existsSync(result.installedTo)).toBe(true);
+            expect(fs.existsSync(path.join(result.installedTo, "node_modules"))).toBe(true);
+            
+            expect(fs.existsSync(path.join(result.installedTo, "node_modules", "lodash"))).toBe(true);
+            expect(fs.existsSync(path.join(result.installedTo, "node_modules", "lodash", "package.json"))).toBe(true);
+            var packageInfo = JSON.parse(fs.readFileSync(path.join(result.installedTo, "node_modules", "lodash", "package.json")));
+            expect(packageInfo.version.slice(0,2)).toBe("3.");
+            
+            expect(fs.existsSync(path.join(result.installedTo, "node_modules", "moment"))).toBe(true);
+            expect(fs.existsSync(path.join(result.installedTo, "node_modules", "moment", "package.json"))).toBe(true);
+            packageInfo = JSON.parse(fs.readFileSync(path.join(result.installedTo, "node_modules", "moment", "package.json")));
+            expect(packageInfo.version.slice(0,4)).toBe("2.5.");
+            
+            expect(fs.existsSync(path.join(result.installedTo, "node_modules", "underscore"))).toBe(true);
+            expect(fs.existsSync(path.join(result.installedTo, "node_modules", "underscore", "package.json"))).toBe(true);
+            packageInfo = JSON.parse(fs.readFileSync(path.join(result.installedTo, "node_modules", "underscore", "package.json")));
+            expect(packageInfo.version).toBe("1.0.4");
+            
+            done();
+        });
+    });
 });
diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js
@@ -513,6 +513,7 @@ define({
     "INVALID_VERSION_NUMBER"               : "The package version number ({0}) is invalid.",
     "INVALID_BRACKETS_VERSION"             : "The {APP_NAME} compatibility string ({0}) is invalid.",
     "DISALLOWED_WORDS"                     : "The words ({1}) are not allowed in the {0} field.",
+    "NPM_INSTALL_FAILED"                   : "npm install command failed: {0}",
     "API_NOT_COMPATIBLE"                   : "The extension isn't compatible with this version of {APP_NAME}. It's installed in your disabled extensions folder.",
     "MISSING_MAIN"                         : "The package has no main.js file.",
     "EXTENSION_ALREADY_INSTALLED"          : "Installing this package will overwrite a previously installed extension. Overwrite the old extension?",
diff --git a/test/spec/extension-test-files/with-npm-dependencies.zip b/test/spec/extension-test-files/with-npm-dependencies.zip

Original file line number	Diff line number	Diff line change
`@@ -39,6 +39,7 @@`
`39`	`39`	`"anymatch": "1.3.0",`
`40`	`40`	`"chokidar": "1.6.0",`
`41`	`41`	`"lodash": "4.15.0",`
	`42`	`+ "npm": "3.10.9",`
`42`	`43`	`"ws": "~0.4.31"`
`43`	`44`	`},`
`44`	`45`	`"devDependencies": {`
`@@ -79,4 +80,4 @@`
`79`	`80`	`"url": "https://github.com/adobe/brackets/blob/master/LICENSE"`
`80`	`81`	`}`
`81`	`82`	`]`
`82`		`-}`
	`83`	`+}`