Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

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

Merged
merged 13 commits into from
Feb 15, 2017
3,573 changes: 3,566 additions & 7 deletions npm-shrinkwrap.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
3 changes: 2 additions & 1 deletion src/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -79,4 +80,4 @@
"url": "https://github.com/adobe/brackets/blob/master/LICENSE"
}
]
}
}
10 changes: 8 additions & 2 deletions src/extensibility/Package.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;
Expand Down
33 changes: 20 additions & 13 deletions src/extensibility/node/ExtensionManagerDomain.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
});
});
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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."
}],
[{
Expand Down
129 changes: 129 additions & 0 deletions src/extensibility/node/npm-installer.js
Original file line number Diff line number Diff line change
@@ -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": "http://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;
21 changes: 13 additions & 8 deletions src/extensibility/node/package-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
});
});
});
Expand Down Expand Up @@ -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) {
Expand Down
29 changes: 28 additions & 1 deletion src/extensibility/node/spec/Installation.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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();
});
});
});
1 change: 1 addition & 0 deletions src/nls/root/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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?",
Expand Down
Binary file not shown.