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

Commit 74c5306

Browse files
zagginoswmitra
authored andcommitted
use npm to download extension dependencies after installing from registry (#10602)
* 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
1 parent 8b1ff4b commit 74c5306

File tree

10 files changed

+3768
-32
lines changed

10 files changed

+3768
-32
lines changed

npm-shrinkwrap.json

Lines changed: 3566 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"anymatch": "1.3.0",
1717
"chokidar": "1.6.0",
1818
"lodash": "4.15.0",
19+
"npm": "3.10.9",
1920
"ws": "~0.4.31"
2021
},
2122
"devDependencies": {

src/config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
"anymatch": "1.3.0",
4040
"chokidar": "1.6.0",
4141
"lodash": "4.15.0",
42+
"npm": "3.10.9",
4243
"ws": "~0.4.31"
4344
},
4445
"devDependencies": {
@@ -79,4 +80,4 @@
7980
"url": "https://github.com/adobe/brackets/blob/master/LICENSE"
8081
}
8182
]
82-
}
83+
}

src/extensibility/Package.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,12 @@ define(function (require, exports, module) {
105105
function validate(path, options) {
106106
return _extensionManagerCall(function (extensionManager) {
107107
var d = new $.Deferred();
108-
108+
109+
// make sure proxy is attached to options before calling validate
110+
// so npm can use it in the domain
111+
options = options || {};
112+
options.proxy = PreferencesManager.get("proxy");
113+
109114
extensionManager.validate(path, options)
110115
.done(function (result) {
111116
d.resolve({
@@ -157,7 +162,8 @@ define(function (require, exports, module) {
157162
disabledDirectory: disabledDirectory,
158163
systemExtensionDirectory: systemDirectory,
159164
apiVersion: brackets.metadata.apiVersion,
160-
nameHint: nameHint
165+
nameHint: nameHint,
166+
proxy: PreferencesManager.get("proxy")
161167
})
162168
.done(function (result) {
163169
result.keepFile = false;

src/extensibility/node/ExtensionManagerDomain.js

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,20 @@ function _removeFailedInstallation(installDirectory) {
9090
function _performInstall(packagePath, installDirectory, validationResult, callback) {
9191
validationResult.installedTo = installDirectory;
9292

93+
function fail(err) {
94+
_removeFailedInstallation(installDirectory);
95+
callback(err, null);
96+
}
97+
98+
function finish() {
99+
// The status may have already been set previously (as in the
100+
// DISABLED case.
101+
if (!validationResult.installationStatus) {
102+
validationResult.installationStatus = Statuses.INSTALLED;
103+
}
104+
callback(null, validationResult);
105+
}
106+
93107
fs.mkdirs(installDirectory, function (err) {
94108
if (err) {
95109
callback(err);
@@ -99,16 +113,9 @@ function _performInstall(packagePath, installDirectory, validationResult, callba
99113

100114
fs.copy(sourceDir, installDirectory, function (err) {
101115
if (err) {
102-
_removeFailedInstallation(installDirectory);
103-
callback(err, null);
104-
} else {
105-
// The status may have already been set previously (as in the
106-
// DISABLED case.
107-
if (!validationResult.installationStatus) {
108-
validationResult.installationStatus = Statuses.INSTALLED;
109-
}
110-
callback(null, validationResult);
116+
return fail(err);
111117
}
118+
finish();
112119
});
113120
});
114121
}
@@ -213,7 +220,7 @@ function _cmdInstall(packagePath, destinationDirectory, options, callback, pCall
213220
return;
214221
}
215222

216-
var validateCallback = function (err, validationResult) {
223+
function validateCallback(err, validationResult) {
217224
validationResult.localPath = packagePath;
218225

219226
// This is a wrapper for the callback that will delete the temporary
@@ -300,9 +307,9 @@ function _cmdInstall(packagePath, destinationDirectory, options, callback, pCall
300307
validationResult.disabledReason = null;
301308
_performInstall(packagePath, installDirectory, validationResult, deleteTempAndCallback);
302309
}
303-
};
310+
}
304311

305-
validate(packagePath, {}, validateCallback);
312+
validate(packagePath, options, validateCallback);
306313
}
307314

308315
/**
@@ -496,7 +503,7 @@ function init(domainManager) {
496503
description: "absolute filesystem path where this extension should be installed"
497504
}, {
498505
name: "options",
499-
type: "{disabledDirectory: !string, apiVersion: !string, nameHint: ?string, systemExtensionDirectory: !string}",
506+
type: "{disabledDirectory: !string, apiVersion: !string, nameHint: ?string, systemExtensionDirectory: !string, proxy: ?string}",
500507
description: "installation options: disabledDirectory should be set so that extensions can be installed disabled."
501508
}],
502509
[{
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
* Copyright (c) 2013 - present Adobe Systems Incorporated. All rights reserved.
3+
*
4+
* Permission is hereby granted, free of charge, to any person obtaining a
5+
* copy of this software and associated documentation files (the "Software"),
6+
* to deal in the Software without restriction, including without limitation
7+
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
8+
* and/or sell copies of the Software, and to permit persons to whom the
9+
* Software is furnished to do so, subject to the following conditions:
10+
*
11+
* The above copyright notice and this permission notice shall be included in
12+
* all copies or substantial portions of the Software.
13+
*
14+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
15+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
16+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
17+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
18+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
19+
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
20+
* DEALINGS IN THE SOFTWARE.
21+
*
22+
*/
23+
24+
/* eslint-env node */
25+
26+
"use strict";
27+
28+
var fs = require("fs-extra"),
29+
path = require("path"),
30+
spawn = require("child_process").spawn;
31+
32+
var Errors = {
33+
NPM_INSTALL_FAILED: "NPM_INSTALL_FAILED"
34+
};
35+
36+
/**
37+
* Private function to run "npm install --production" command in the extension directory.
38+
*
39+
* @param {string} installDirectory Directory to remove
40+
* @param {function} callback NodeJS style callback to call after finish
41+
*/
42+
function _performNpmInstall(installDirectory, npmOptions, callback) {
43+
var npmPath = path.resolve(path.dirname(require.resolve("npm")), "..", "bin", "npm-cli.js");
44+
var args = [npmPath, "install"];
45+
46+
// npmOptions can contain additional args like { "production": true, "proxy": "http://127.0.0.1:8888" }
47+
Object.keys(npmOptions).forEach(function (key) {
48+
var value = npmOptions[key];
49+
if (value === true) {
50+
args.push("--" + key);
51+
} else if (typeof value === "string" && value.length > 0) {
52+
args.push("--" + key, value);
53+
}
54+
});
55+
56+
console.log("running npm " + args.slice(1).join(" ") + " in " + installDirectory);
57+
58+
var child = spawn(process.execPath, args, { cwd: installDirectory });
59+
60+
child.on("error", function (err) {
61+
return callback(err);
62+
});
63+
64+
var stdout = [];
65+
child.stdout.addListener("data", function (buffer) {
66+
stdout.push(buffer);
67+
});
68+
69+
var stderr = [];
70+
child.stderr.addListener("data", function (buffer) {
71+
stderr.push(buffer);
72+
});
73+
74+
var exitCode = 0;
75+
child.addListener("exit", function (code) {
76+
exitCode = code;
77+
});
78+
79+
child.addListener("close", function () {
80+
stderr = Buffer.concat(stderr).toString();
81+
stdout = Buffer.concat(stdout).toString();
82+
if (exitCode > 0) {
83+
console.error("npm-stderr: " + stderr);
84+
return callback(new Error(stderr));
85+
}
86+
if (stderr) {
87+
console.warn("npm-stderr: " + stderr);
88+
}
89+
console.log("npm-stdout: " + stdout);
90+
return callback();
91+
});
92+
93+
child.stdin.end();
94+
}
95+
96+
/**
97+
* Checks package.json of the extracted extension for npm dependencies
98+
* and runs npm install when required.
99+
* @param {Object} validationResult return value of the validation procedure
100+
* @param {Function} callback function to be called after the end of validation procedure
101+
*/
102+
function performNpmInstallIfRequired(npmOptions, validationResult, callback) {
103+
104+
function finish() {
105+
callback(null, validationResult);
106+
}
107+
108+
var installDirectory = path.join(validationResult.extractDir, validationResult.commonPrefix);
109+
var packageJson;
110+
111+
try {
112+
packageJson = fs.readJsonSync(path.join(installDirectory, "package.json"));
113+
} catch (e) {
114+
packageJson = null;
115+
}
116+
117+
if (!packageJson || !packageJson.dependencies) {
118+
return finish();
119+
}
120+
121+
_performNpmInstall(installDirectory, npmOptions, function (err) {
122+
if (err) {
123+
validationResult.errors.push([Errors.NPM_INSTALL_FAILED, err.toString()]);
124+
}
125+
finish();
126+
});
127+
}
128+
129+
exports.performNpmInstallIfRequired = performNpmInstallIfRequired;

src/extensibility/node/package-validator.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@
2626

2727
"use strict";
2828

29-
var DecompressZip = require("decompress-zip"),
30-
semver = require("semver"),
31-
path = require("path"),
32-
temp = require("temp"),
33-
fs = require("fs-extra");
29+
var DecompressZip = require("decompress-zip"),
30+
semver = require("semver"),
31+
path = require("path"),
32+
temp = require("temp"),
33+
fs = require("fs-extra"),
34+
performNpmInstallIfRequired = require("./npm-installer").performNpmInstallIfRequired;
3435

3536
// Track and cleanup files at exit
3637
temp.track();
@@ -293,12 +294,16 @@ function extractAndValidateFiles(zipPath, extractDir, options, callback) {
293294
if (!isTheme && !fs.existsSync(mainJS)) {
294295
errors.push([Errors.MISSING_MAIN, zipPath, mainJS]);
295296
}
296-
callback(null, {
297+
298+
performNpmInstallIfRequired({
299+
production: true,
300+
proxy: options.proxy
301+
}, {
297302
errors: errors,
298303
metadata: metadata,
299304
commonPrefix: commonPrefix,
300305
extractDir: extractDir
301-
});
306+
}, callback);
302307
});
303308
});
304309
});
@@ -327,7 +332,7 @@ function extractAndValidateFiles(zipPath, extractDir, options, callback) {
327332
* read successfully from package.json in the zip file.
328333
*
329334
* @param {string} path Absolute path to the package zip file
330-
* @param {{requirePackageJSON: ?boolean, disallowedWords: ?Array.<string>}} options for validation
335+
* @param {{requirePackageJSON: ?boolean, disallowedWords: ?Array.<string>, proxy: ?<string>}} options for validation
331336
* @param {function} callback (err, result)
332337
*/
333338
function validate(path, options, callback) {

src/extensibility/node/spec/Installation.spec.js

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ var basicValidExtension = path.join(testFilesDirectory, "basic-valid-exten
5555
missingPackageJSON = path.join(testFilesDirectory, "missing-package-json.zip"),
5656
missingPackageJSONUpdate = path.join(testFilesDirectory, "missing-package-json-update.zip"),
5757
missingPackageJSONRenamed = path.join(testFilesDirectory, "added-package-json-test", "missing-package-json.zip"),
58-
withSymlink = path.join(testFilesDirectory, "with-symlink.zip");
58+
withSymlink = path.join(testFilesDirectory, "with-symlink.zip"),
59+
withNpmDependencies = path.join(testFilesDirectory, "with-npm-dependencies.zip");
5960

6061

6162
describe("Package Installation", function () {
@@ -324,4 +325,30 @@ describe("Package Installation", function () {
324325
done();
325326
});
326327
});
328+
329+
it("should download npm dependencies when present", function (done) {
330+
ExtensionsDomain._cmdInstall(withNpmDependencies, installDirectory, standardOptions, function (err, result) {
331+
expect(err).toBeNull();
332+
expect(result.errors.length).toEqual(0);
333+
expect(fs.existsSync(result.installedTo)).toBe(true);
334+
expect(fs.existsSync(path.join(result.installedTo, "node_modules"))).toBe(true);
335+
336+
expect(fs.existsSync(path.join(result.installedTo, "node_modules", "lodash"))).toBe(true);
337+
expect(fs.existsSync(path.join(result.installedTo, "node_modules", "lodash", "package.json"))).toBe(true);
338+
var packageInfo = JSON.parse(fs.readFileSync(path.join(result.installedTo, "node_modules", "lodash", "package.json")));
339+
expect(packageInfo.version.slice(0,2)).toBe("3.");
340+
341+
expect(fs.existsSync(path.join(result.installedTo, "node_modules", "moment"))).toBe(true);
342+
expect(fs.existsSync(path.join(result.installedTo, "node_modules", "moment", "package.json"))).toBe(true);
343+
packageInfo = JSON.parse(fs.readFileSync(path.join(result.installedTo, "node_modules", "moment", "package.json")));
344+
expect(packageInfo.version.slice(0,4)).toBe("2.5.");
345+
346+
expect(fs.existsSync(path.join(result.installedTo, "node_modules", "underscore"))).toBe(true);
347+
expect(fs.existsSync(path.join(result.installedTo, "node_modules", "underscore", "package.json"))).toBe(true);
348+
packageInfo = JSON.parse(fs.readFileSync(path.join(result.installedTo, "node_modules", "underscore", "package.json")));
349+
expect(packageInfo.version).toBe("1.0.4");
350+
351+
done();
352+
});
353+
});
327354
});

src/nls/root/strings.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@ define({
513513
"INVALID_VERSION_NUMBER" : "The package version number ({0}) is invalid.",
514514
"INVALID_BRACKETS_VERSION" : "The {APP_NAME} compatibility string ({0}) is invalid.",
515515
"DISALLOWED_WORDS" : "The words ({1}) are not allowed in the {0} field.",
516+
"NPM_INSTALL_FAILED" : "npm install command failed: {0}",
516517
"API_NOT_COMPATIBLE" : "The extension isn't compatible with this version of {APP_NAME}. It's installed in your disabled extensions folder.",
517518
"MISSING_MAIN" : "The package has no main.js file.",
518519
"EXTENSION_ALREADY_INSTALLED" : "Installing this package will overwrite a previously installed extension. Overwrite the old extension?",
Binary file not shown.

0 commit comments

Comments
 (0)