From ecb9c9a97ce329ce33263fa30a2c942c8dbe71ec Mon Sep 17 00:00:00 2001 From: Patricio Palladino Date: Tue, 11 Jun 2019 16:03:10 -0300 Subject: [PATCH 1/7] Change tslint rule for enum names --- config/tslint/tslint.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/tslint/tslint.json b/config/tslint/tslint.json index 0ecd9d2f73..f3721f6608 100644 --- a/config/tslint/tslint.json +++ b/config/tslint/tslint.json @@ -53,7 +53,8 @@ }, { "type": "type", "format": "PascalCase" }, { "type": "genericTypeParameter", "suffix": "T" }, - { "type": "enumMember", "format": "PascalCase" } + { "type": "enum", "format": "PascalCase" }, + { "type": "enumMember", "format": "UPPER_CASE" } ] } } From 8f9149184fe924fc0cb4ebf32ec7dd06f7f0b783 Mon Sep 17 00:00:00 2001 From: Patricio Palladino Date: Tue, 11 Jun 2019 16:20:08 -0300 Subject: [PATCH 2/7] Make Buidler's execution mode explicit --- packages/buidler-core/package-lock.json | 30 ++++++++++++++++ packages/buidler-core/package.json | 1 + .../src/internal/core/execution-mode.ts | 34 +++++++++++++++++++ .../src/internal/core/typescript-support.ts | 16 ++++----- .../src/internal/util/scripts-runner.ts | 4 +-- 5 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 packages/buidler-core/src/internal/core/execution-mode.ts diff --git a/packages/buidler-core/package-lock.json b/packages/buidler-core/package-lock.json index c8010e08c5..6d55c46bd6 100644 --- a/packages/buidler-core/package-lock.json +++ b/packages/buidler-core/package-lock.json @@ -1357,6 +1357,14 @@ "process": "~0.5.1" } }, + "global-dirs": { + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/global-dirs/-/global-dirs-0.1.1.tgz", + "integrity": "sha1-sxnA3UYH81PzvpzKTHL8FIxJ9EU=", + "requires": { + "ini": "^1.3.4" + } + }, "got": { "version": "8.3.2", "resolved": "https://registry.npmjs.org/got/-/got-8.3.2.tgz", @@ -1608,6 +1616,15 @@ "resolved": "https://registry.npmjs.org/is-hex-prefixed/-/is-hex-prefixed-1.0.0.tgz", "integrity": "sha1-fY035q135dEnFIkTxXPggtd39VQ=" }, + "is-installed-globally": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/is-installed-globally/-/is-installed-globally-0.2.0.tgz", + "integrity": "sha512-g3TzWCnR/eO4Q3abCwgFjOFw7uVOfxG4m8hMr/39Jcf2YvE5mHrFKqpyuraWV4zwx9XhjnVO4nY0ZI4llzl0Pg==", + "requires": { + "global-dirs": "^0.1.1", + "is-path-inside": "^2.1.0" + } + }, "is-natural-number": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/is-natural-number/-/is-natural-number-4.0.1.tgz", @@ -1618,6 +1635,14 @@ "resolved": "https://registry.npmjs.org/is-object/-/is-object-1.0.1.tgz", "integrity": "sha1-iVJojF7C/9awPsyF52ngKQMINHA=" }, + "is-path-inside": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/is-path-inside/-/is-path-inside-2.1.0.tgz", + "integrity": "sha512-wiyhTzfDWsvwAW53OBWF5zuvaOGlZ6PwYxAbPVDhpm+gM09xKQGjBq/8uYN12aDvMxnAnq3dxTyoSoRNmg5YFg==", + "requires": { + "path-is-inside": "^1.0.2" + } + }, "is-plain-obj": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/is-plain-obj/-/is-plain-obj-1.1.0.tgz", @@ -2187,6 +2212,11 @@ "resolved": "https://registry.npmjs.org/path-is-absolute/-/path-is-absolute-1.0.1.tgz", "integrity": "sha1-F0uSaHNVNP+8es5r9TpanhtcX18=" }, + "path-is-inside": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/path-is-inside/-/path-is-inside-1.0.2.tgz", + "integrity": "sha1-NlQX3t5EQw0cEa9hAn+s8HS9/FM=" + }, "path-key": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/path-key/-/path-key-2.0.1.tgz", diff --git a/packages/buidler-core/package.json b/packages/buidler-core/package.json index d97a164dc3..01c594afac 100644 --- a/packages/buidler-core/package.json +++ b/packages/buidler-core/package.json @@ -71,6 +71,7 @@ "fs-extra": "^7.0.1", "glob": "^7.1.3", "io-ts": "^1.8.6", + "is-installed-globally": "^0.2.0", "lodash": "^4.17.11", "mocha": "^5.2.0", "semver": "^5.6.0", diff --git a/packages/buidler-core/src/internal/core/execution-mode.ts b/packages/buidler-core/src/internal/core/execution-mode.ts new file mode 100644 index 0000000000..1b18fc2a17 --- /dev/null +++ b/packages/buidler-core/src/internal/core/execution-mode.ts @@ -0,0 +1,34 @@ +/** + * This module defines different Buidler execution modes and autodetects them. + * + * IMPORTANT: This will have to be revisited once Yarn PnP and npm's tink get + * widely adopted. + */ +export enum ExecutionMode { + EXECUTION_MODE_TS_NODE_TESTS, + EXECUTION_MODE_LINKED, + EXECUTION_MODE_GLOBAL_INSTALLATION, + EXECUTION_MODE_LOCAL_INSTALLATION +} + +const workingDirectoryOnLoad = process.cwd(); + +export function getExecutionMode(): ExecutionMode { + const isInstalled = __filename.includes("node_modules"); + + if (!isInstalled) { + // When running the tests with ts-node we set the CWD to the root of + // buidler-core. We could check if the __filename ends with .ts + if (__dirname.startsWith(workingDirectoryOnLoad)) { + return ExecutionMode.EXECUTION_MODE_TS_NODE_TESTS; + } + + return ExecutionMode.EXECUTION_MODE_LINKED; + } + + if (require("is-installed-globally")) { + return ExecutionMode.EXECUTION_MODE_GLOBAL_INSTALLATION; + } + + return ExecutionMode.EXECUTION_MODE_LOCAL_INSTALLATION; +} diff --git a/packages/buidler-core/src/internal/core/typescript-support.ts b/packages/buidler-core/src/internal/core/typescript-support.ts index bf82432c93..accc8f1e65 100644 --- a/packages/buidler-core/src/internal/core/typescript-support.ts +++ b/packages/buidler-core/src/internal/core/typescript-support.ts @@ -2,15 +2,9 @@ import colors from "ansi-colors"; import * as fs from "fs"; import * as path from "path"; -const NODE_MODULES_DIR = "node_modules"; +import { ExecutionMode, getExecutionMode } from "./execution-mode"; -/** - * This function returns true only if Buidler was installed as a dependency. It - * returns false otherwise, including when it's being linked. - */ -function isBuidlerInstalledAsADependency() { - return __dirname.lastIndexOf(NODE_MODULES_DIR) !== -1; -} +const NODE_MODULES_DIR = "node_modules"; function getBuidlerNodeModules() { return __dirname.substring( @@ -23,7 +17,11 @@ let cachedIsTypescriptSupported: boolean | undefined; export function isTypescriptSupported() { if (cachedIsTypescriptSupported === undefined) { - if (isBuidlerInstalledAsADependency()) { + const executionMode = getExecutionMode(); + if ( + executionMode === ExecutionMode.EXECUTION_MODE_LOCAL_INSTALLATION || + executionMode === ExecutionMode.EXECUTION_MODE_GLOBAL_INSTALLATION + ) { const nodeModules = getBuidlerNodeModules(); cachedIsTypescriptSupported = fs.existsSync(path.join(nodeModules, "typescript")) && diff --git a/packages/buidler-core/src/internal/util/scripts-runner.ts b/packages/buidler-core/src/internal/util/scripts-runner.ts index f5149ca491..a4132b2f07 100644 --- a/packages/buidler-core/src/internal/util/scripts-runner.ts +++ b/packages/buidler-core/src/internal/util/scripts-runner.ts @@ -1,4 +1,5 @@ import { BuidlerArguments } from "../../types"; +import { ExecutionMode, getExecutionMode } from "../core/execution-mode"; import { getEnvVariablesMap } from "../core/params/env-variables"; export async function runScript( @@ -46,8 +47,7 @@ export async function runScriptWithBuidler( } function getTsNodeArgsIfNeeded() { - // This means we are running the tests - if (!__filename.endsWith(".ts")) { + if (getExecutionMode() !== ExecutionMode.EXECUTION_MODE_TS_NODE_TESTS) { return []; } From 68cb6ac72186454deb965c6c9501388d72dd433c Mon Sep 17 00:00:00 2001 From: Patricio Palladino Date: Tue, 11 Jun 2019 16:21:11 -0300 Subject: [PATCH 3/7] Make ensurePluginLoadedWithUsePlugin slightly more premissive --- packages/buidler-core/src/internal/core/plugins.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/buidler-core/src/internal/core/plugins.ts b/packages/buidler-core/src/internal/core/plugins.ts index 31c938ea6c..5f512c451a 100644 --- a/packages/buidler-core/src/internal/core/plugins.ts +++ b/packages/buidler-core/src/internal/core/plugins.ts @@ -102,8 +102,16 @@ export function ensurePluginLoadedWithUsePlugin() { for (const callSite of stack) { const fileName = callSite.getFileName(); + if (fileName === null) { + continue; + } + const functionName = callSite.getFunctionName(); - if (fileName === __filename && functionName === loadPluginFile.name) { + + if ( + path.basename(fileName) === path.basename(__filename) && + functionName === loadPluginFile.name + ) { return; } } From 699efd8ff9c7f8550cc81ffa2c9fd55ddc149d3d Mon Sep 17 00:00:00 2001 From: Patricio Palladino Date: Tue, 11 Jun 2019 16:41:39 -0300 Subject: [PATCH 4/7] Make usePlugin's relative imports just a testing option --- .../src/internal/core/config/config-env.ts | 5 +++- .../buidler-core/src/internal/core/plugins.ts | 29 +++++++++---------- .../test/internal/core/plugins.ts | 18 +++++++----- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/packages/buidler-core/src/internal/core/config/config-env.ts b/packages/buidler-core/src/internal/core/config/config-env.ts index 90a79dceb1..922d7e79e4 100644 --- a/packages/buidler-core/src/internal/core/config/config-env.ts +++ b/packages/buidler-core/src/internal/core/config/config-env.ts @@ -6,6 +6,7 @@ import { } from "../../../types"; import { BuidlerContext } from "../../context"; import * as argumentTypes from "../params/argumentTypes"; +import { usePlugin as usePluginImplementation } from "../plugins"; export function task( name: string, @@ -82,4 +83,6 @@ export function extendEnvironment(extender: EnvironmentExtender) { extenderManager.add(extender); } -export { usePlugin } from "../plugins"; +export function usePlugin(pluginName: string) { + usePluginImplementation(pluginName); +} diff --git a/packages/buidler-core/src/internal/core/plugins.ts b/packages/buidler-core/src/internal/core/plugins.ts index 5f512c451a..a91324f9a4 100644 --- a/packages/buidler-core/src/internal/core/plugins.ts +++ b/packages/buidler-core/src/internal/core/plugins.ts @@ -1,7 +1,6 @@ import * as path from "path"; import * as semver from "semver"; -import { BuidlerContext } from "../context"; import { getClosestCallerPackage } from "../util/caller-package"; import { BuidlerError, ERRORS } from "./errors"; @@ -13,11 +12,14 @@ interface PackageJson { }; } -export function usePlugin(pluginName: string) { - const ctx = BuidlerContext.getBuidlerContext(); - const configFileDir = path.dirname(ctx.configPath!); - - const pluginPackageJson = readPackageJson(pluginName, configFileDir); +/** + * Validates a plugin dependencies and loads it. + * @param pluginName - The plugin name + * @param from - Where to resolve plugins and dependencies from. Only for + * testing purposes. + */ +export function usePlugin(pluginName: string, from?: string) { + const pluginPackageJson = readPackageJson(pluginName, from); if (pluginPackageJson === undefined) { throw new BuidlerError(ERRORS.PLUGINS.NOT_INSTALLED, pluginName); @@ -27,10 +29,7 @@ export function usePlugin(pluginName: string) { for (const [dependencyName, versionSpec] of Object.entries( pluginPackageJson.peerDependencies )) { - const dependencyPackageJson = readPackageJson( - dependencyName, - configFileDir - ); + const dependencyPackageJson = readPackageJson(dependencyName, from); if (dependencyPackageJson === undefined) { throw new BuidlerError( @@ -60,7 +59,8 @@ export function usePlugin(pluginName: string) { } } - const pluginPath = require.resolve(pluginName, { paths: [configFileDir] }); + const options = from !== undefined ? { paths: [from] } : undefined; + const pluginPath = require.resolve(pluginName, options); loadPluginFile(pluginPath); } @@ -74,14 +74,13 @@ export function loadPluginFile(absolutePluginFilePath: string) { export function readPackageJson( packageName: string, - from: string + from?: string ): PackageJson | undefined { try { + const options = from !== undefined ? { paths: [from] } : undefined; const packageJsonPath = require.resolve( path.join(packageName, "package.json"), - { - paths: [from] - } + options ); return require(packageJsonPath); diff --git a/packages/buidler-core/test/internal/core/plugins.ts b/packages/buidler-core/test/internal/core/plugins.ts index 1d920fb62f..fc1c03331b 100644 --- a/packages/buidler-core/test/internal/core/plugins.ts +++ b/packages/buidler-core/test/internal/core/plugins.ts @@ -31,6 +31,10 @@ describe("plugin system", function() { assert.isUndefined(readPackageJson("NOPE", FIXTURE_PROJECT_PATH)); assert.isUndefined(readPackageJson("NOPE2", __dirname)); }); + + it("Should work without from param", function() { + assert.isDefined(readPackageJson("mocha")); + }); }); describe("loadPluginFile", function() { @@ -68,11 +72,11 @@ describe("plugin system", function() { describe("loadPluginFile", function() { const globalAsAny = global as any; + const projectPath = + FIXTURE_PROJECT_PATH + "/doesnt-need-to-exist-config.js"; beforeEach(function() { BuidlerContext.createBuidlerContext(); - const ctx = BuidlerContext.getBuidlerContext(); - ctx.configPath = FIXTURE_PROJECT_PATH + "/doesnt-need-to-exist-config.js"; }); afterEach(function() { @@ -81,32 +85,32 @@ describe("plugin system", function() { }); it("Should load a plugin if it has no peer dependency", function() { - usePlugin("pack1"); + usePlugin("pack1", projectPath); assert.isTrue(globalAsAny.loaded); }); it("Should load a plugin if it has all of its dependencies", function() { - usePlugin("requires-pack1"); + usePlugin("requires-pack1", projectPath); assert.isTrue(globalAsAny.loaded); }); it("Should fail if a peer dependency is missing", function() { expectBuidlerError( - () => usePlugin("requires-missing-pack"), + () => usePlugin("requires-missing-pack", projectPath), ERRORS.PLUGINS.MISSING_DEPENDENCY ); }); it("Should fail if a peer dependency has an incompatible version", function() { expectBuidlerError( - () => usePlugin("requires-other-version-pack1"), + () => usePlugin("requires-other-version-pack1", projectPath), ERRORS.PLUGINS.DEPENDENCY_VERSION_MISMATCH ); }); it("Should fail if the plugin isn't installed", function() { expectBuidlerError( - () => usePlugin("not-installed"), + () => usePlugin("not-installed", projectPath), ERRORS.PLUGINS.NOT_INSTALLED ); }); From 5cfd9e64598ce19085ea5ade2f96982c44366b28 Mon Sep 17 00:00:00 2001 From: Patricio Palladino Date: Tue, 11 Jun 2019 17:21:37 -0300 Subject: [PATCH 5/7] Don't load plugins twice --- packages/buidler-core/src/internal/context.ts | 6 +++- .../src/internal/core/config/config-env.ts | 7 +++- .../internal/core/config/config-loading.ts | 15 --------- .../buidler-core/src/internal/core/plugins.ts | 33 ++++++++++++++++++- .../node_modules/pack1/package.json | 1 + .../requires-missing-pack/package.json | 1 + .../requires-other-version-pack1/package.json | 1 + .../node_modules/requires-pack1/package.json | 1 + .../validates-import-style/package.json | 1 + .../test/internal/core/plugins.ts | 23 +++++++++---- 10 files changed, 65 insertions(+), 24 deletions(-) diff --git a/packages/buidler-core/src/internal/context.ts b/packages/buidler-core/src/internal/context.ts index 8f685dcc70..5058b73807 100644 --- a/packages/buidler-core/src/internal/context.ts +++ b/packages/buidler-core/src/internal/context.ts @@ -38,7 +38,7 @@ export class BuidlerContext { public readonly tasksDSL = new TasksDSL(); public readonly extendersManager = new ExtenderManager(); public environment?: BuidlerRuntimeEnvironment; - public configPath?: string; + public readonly loadedPlugins: string[] = []; public setBuidlerRuntimeEnvironment(env: BuidlerRuntimeEnvironment) { if (this.environment !== undefined) { @@ -53,4 +53,8 @@ export class BuidlerContext { } return this.environment; } + + public setPluginAsLoaded(pluginName: string) { + this.loadedPlugins.push(pluginName); + } } diff --git a/packages/buidler-core/src/internal/core/config/config-env.ts b/packages/buidler-core/src/internal/core/config/config-env.ts index 922d7e79e4..c60b5a88e4 100644 --- a/packages/buidler-core/src/internal/core/config/config-env.ts +++ b/packages/buidler-core/src/internal/core/config/config-env.ts @@ -83,6 +83,11 @@ export function extendEnvironment(extender: EnvironmentExtender) { extenderManager.add(extender); } +/** + * Loads a Buidler plugin + * @param pluginName The plugin name. + */ export function usePlugin(pluginName: string) { - usePluginImplementation(pluginName); + const ctx = BuidlerContext.getBuidlerContext(); + usePluginImplementation(ctx, pluginName); } diff --git a/packages/buidler-core/src/internal/core/config/config-loading.ts b/packages/buidler-core/src/internal/core/config/config-loading.ts index 4c3f6c54e6..6c0321519b 100644 --- a/packages/buidler-core/src/internal/core/config/config-loading.ts +++ b/packages/buidler-core/src/internal/core/config/config-loading.ts @@ -34,21 +34,6 @@ export function loadConfigAndTasks(configPath?: string): ResolvedBuidlerConfig { ([key, value]) => (globalAsAny[key] = value) ); - // This is a horrible hack that deserves an explanation. - // - config files can execute the usePlugin function, which is imported - // from config.ts. - // - There's no way to pass it arguments. - // - Internally, usePlugin calls require, which should use the same - // node_module's paths than the Buidler project. - // - Except that it doesn't when we are linking Buidler for local tests. - // - node resolves symlinks before loading modules, so imports from - // Buidler files are run in this context, not inside the Buidler project. - // - We solve this by using require.resolve and specifying the paths, - // but we need the config path in order to do so. - // - We set the config path into the BuidlerContext and cry a little 😢 - const ctx = BuidlerContext.getBuidlerContext(); - ctx.configPath = configPath; - loadPluginFile(__dirname + "/../tasks/builtin-tasks"); const defaultConfig = importCsjOrEsModule("./default-config"); diff --git a/packages/buidler-core/src/internal/core/plugins.ts b/packages/buidler-core/src/internal/core/plugins.ts index a91324f9a4..9cd4edab9e 100644 --- a/packages/buidler-core/src/internal/core/plugins.ts +++ b/packages/buidler-core/src/internal/core/plugins.ts @@ -1,11 +1,14 @@ import * as path from "path"; import * as semver from "semver"; +import { BuidlerContext } from "../context"; import { getClosestCallerPackage } from "../util/caller-package"; import { BuidlerError, ERRORS } from "./errors"; +import { ExecutionMode, getExecutionMode } from "./execution-mode"; interface PackageJson { + name: string; version: string; peerDependencies: { [name: string]: string; @@ -15,16 +18,42 @@ interface PackageJson { /** * Validates a plugin dependencies and loads it. * @param pluginName - The plugin name + * @param buidlerContext - The BuidlerContext * @param from - Where to resolve plugins and dependencies from. Only for * testing purposes. */ -export function usePlugin(pluginName: string, from?: string) { +export function usePlugin( + buidlerContext: BuidlerContext, + pluginName: string, + from?: string +) { + // We have a special case for `ExecutionMode.EXECUTION_MODE_LINKED` + // + // If Buidler is linked, a require without `from` would be executed in the + // context of Buidler, and not find any plugin (linked or not). We workaround + // this by using the CWD here. + // + // This is not ideal, but the only reason to link Buidler is testing. + if ( + from === undefined && + getExecutionMode() === ExecutionMode.EXECUTION_MODE_LINKED + ) { + from = process.cwd(); + } + const pluginPackageJson = readPackageJson(pluginName, from); if (pluginPackageJson === undefined) { throw new BuidlerError(ERRORS.PLUGINS.NOT_INSTALLED, pluginName); } + // We use the package.json's version of the name, as it is normalized. + pluginName = pluginPackageJson.name; + + if (buidlerContext.loadedPlugins.includes(pluginName)) { + return; + } + if (pluginPackageJson.peerDependencies !== undefined) { for (const [dependencyName, versionSpec] of Object.entries( pluginPackageJson.peerDependencies @@ -62,6 +91,8 @@ export function usePlugin(pluginName: string, from?: string) { const options = from !== undefined ? { paths: [from] } : undefined; const pluginPath = require.resolve(pluginName, options); loadPluginFile(pluginPath); + + buidlerContext.setPluginAsLoaded(pluginName); } export function loadPluginFile(absolutePluginFilePath: string) { diff --git a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/pack1/package.json b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/pack1/package.json index 87032da008..bff5d7bffd 100644 --- a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/pack1/package.json +++ b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/pack1/package.json @@ -1,3 +1,4 @@ { + "name": "pack1", "version": "2.1.0" } diff --git a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-missing-pack/package.json b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-missing-pack/package.json index 6c70e7bfa8..0b31881afc 100644 --- a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-missing-pack/package.json +++ b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-missing-pack/package.json @@ -1,4 +1,5 @@ { + "name": "requires-missing-pack", "version": "1.0.0", "peerDependencies": { "non-existent": "^0.0.1" diff --git a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-other-version-pack1/package.json b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-other-version-pack1/package.json index 8cff63e693..cb0b567588 100644 --- a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-other-version-pack1/package.json +++ b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-other-version-pack1/package.json @@ -1,4 +1,5 @@ { + "name": "requires-other-version-pack1", "version": "1.0.0", "peerDependencies": { "pack1": "^0.0.1" diff --git a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-pack1/package.json b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-pack1/package.json index e85016404e..3170a7d55f 100644 --- a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-pack1/package.json +++ b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/requires-pack1/package.json @@ -1,4 +1,5 @@ { + "name": "requires-pack1", "version": "1.2.3", "peerDependencies": { "pack1": "^2.0.1" diff --git a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/validates-import-style/package.json b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/validates-import-style/package.json index 87032da008..c2c45d88fd 100644 --- a/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/validates-import-style/package.json +++ b/packages/buidler-core/test/fixture-projects/plugin-loading-project/node_modules/validates-import-style/package.json @@ -1,3 +1,4 @@ { + "name": "validates-import-style", "version": "2.1.0" } diff --git a/packages/buidler-core/test/internal/core/plugins.ts b/packages/buidler-core/test/internal/core/plugins.ts index fc1c03331b..c22b3d640a 100644 --- a/packages/buidler-core/test/internal/core/plugins.ts +++ b/packages/buidler-core/test/internal/core/plugins.ts @@ -74,9 +74,10 @@ describe("plugin system", function() { const globalAsAny = global as any; const projectPath = FIXTURE_PROJECT_PATH + "/doesnt-need-to-exist-config.js"; + let ctx: BuidlerContext; beforeEach(function() { - BuidlerContext.createBuidlerContext(); + ctx = BuidlerContext.createBuidlerContext(); }); afterEach(function() { @@ -85,32 +86,42 @@ describe("plugin system", function() { }); it("Should load a plugin if it has no peer dependency", function() { - usePlugin("pack1", projectPath); + usePlugin(ctx, "pack1", projectPath); assert.isTrue(globalAsAny.loaded); }); + it("Shouldn't load a plugin twice", function() { + usePlugin(ctx, "pack1", projectPath); + assert.isTrue(globalAsAny.loaded); + + globalAsAny.loaded = false; + + usePlugin(ctx, "pack1", projectPath); + assert.isFalse(globalAsAny.loaded); + }); + it("Should load a plugin if it has all of its dependencies", function() { - usePlugin("requires-pack1", projectPath); + usePlugin(ctx, "requires-pack1", projectPath); assert.isTrue(globalAsAny.loaded); }); it("Should fail if a peer dependency is missing", function() { expectBuidlerError( - () => usePlugin("requires-missing-pack", projectPath), + () => usePlugin(ctx, "requires-missing-pack", projectPath), ERRORS.PLUGINS.MISSING_DEPENDENCY ); }); it("Should fail if a peer dependency has an incompatible version", function() { expectBuidlerError( - () => usePlugin("requires-other-version-pack1", projectPath), + () => usePlugin(ctx, "requires-other-version-pack1", projectPath), ERRORS.PLUGINS.DEPENDENCY_VERSION_MISMATCH ); }); it("Should fail if the plugin isn't installed", function() { expectBuidlerError( - () => usePlugin("not-installed", projectPath), + () => usePlugin(ctx, "not-installed", projectPath), ERRORS.PLUGINS.NOT_INSTALLED ); }); From c98961d8c863f3a36bc2fec383a7cefae94c0969 Mon Sep 17 00:00:00 2001 From: Patricio Palladino Date: Tue, 11 Jun 2019 18:48:40 -0300 Subject: [PATCH 6/7] Better plugin dependencies errors --- .../buidler-core/src/internal/core/errors.ts | 8 ++++---- .../buidler-core/src/internal/core/plugins.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/buidler-core/src/internal/core/errors.ts b/packages/buidler-core/src/internal/core/errors.ts index db6adc233a..d901dbce96 100644 --- a/packages/buidler-core/src/internal/core/errors.ts +++ b/packages/buidler-core/src/internal/core/errors.ts @@ -373,14 +373,14 @@ To learn more about Buidler's configuration, please go to https://buidler.dev/do MISSING_DEPENDENCY: { number: 801, message: - "Plugin %s requires %s to be installed.\n" + - "Please run: npm install --save-dev %s@%s" + "Plugin %s requires %s to be installed.\n%s" + + "Please run: npm install --save-dev%s %s@%s" }, DEPENDENCY_VERSION_MISMATCH: { number: 802, message: - "Plugin %s requires %s version %s but got %s.\n" + - "If you haven't installed %s manually, please run: npm install --save-dev %s@%s\n" + + "Plugin %s requires %s version %s but got %s.\n%s" + + "If you haven't installed %s manually, please run: npm install --save-dev%s %s@%s\n" + "If you have installed %s yourself, please reinstall it with a valid version." }, OLD_STYLE_IMPORT_DETECTED: { diff --git a/packages/buidler-core/src/internal/core/plugins.ts b/packages/buidler-core/src/internal/core/plugins.ts index 9cd4edab9e..34384b5f29 100644 --- a/packages/buidler-core/src/internal/core/plugins.ts +++ b/packages/buidler-core/src/internal/core/plugins.ts @@ -54,17 +54,33 @@ export function usePlugin( return; } + let globalFlag = ""; + let globalWarning = ""; + if (getExecutionMode() === ExecutionMode.EXECUTION_MODE_GLOBAL_INSTALLATION) { + globalFlag = " --global"; + globalWarning = + "You are using a global installation of Buidler. Plugins and their dependencies must also be global.\n"; + } + if (pluginPackageJson.peerDependencies !== undefined) { for (const [dependencyName, versionSpec] of Object.entries( pluginPackageJson.peerDependencies )) { const dependencyPackageJson = readPackageJson(dependencyName, from); + let installExtraFlags = globalFlag; + + if (versionSpec.match(/^[0-9]/) !== null) { + installExtraFlags += " --save-exact"; + } + if (dependencyPackageJson === undefined) { throw new BuidlerError( ERRORS.PLUGINS.MISSING_DEPENDENCY, pluginName, dependencyName, + globalWarning, + installExtraFlags, dependencyName, versionSpec ); @@ -79,7 +95,9 @@ export function usePlugin( dependencyName, versionSpec, installedVersion, + globalWarning, dependencyName, + installExtraFlags, dependencyName, versionSpec, dependencyName From 353cdd31dcecf14bef0b851d3900fe3512378e78 Mon Sep 17 00:00:00 2001 From: Patricio Palladino Date: Tue, 11 Jun 2019 18:51:07 -0300 Subject: [PATCH 7/7] Add message about global plugins to the README --- packages/buidler-core/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/buidler-core/README.md b/packages/buidler-core/README.md index 5d624265fa..ab70304c3b 100644 --- a/packages/buidler-core/README.md +++ b/packages/buidler-core/README.md @@ -22,6 +22,8 @@ The recommended way of using Buidler is through a local installation in your pro Be careful about inconsistent behavior across different projects that use different Buidler versions. npm -g install @nomiclabs/buidler + +If you choose to install Buidler globally, you have to do the same for its plugins and their dependencies. ## Documentation