From 18aa173d39e55e61135be6fe9892adfa503a9e48 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Sat, 3 Aug 2019 17:37:19 +0300 Subject: [PATCH] feat(docs-infra): support building the local Angular packages in `NgPackagesInstaller` (#31985) Previously, when `NgPackagesInstaller` needed to install the local Angular packages to a project, it assumed the `dist/packages-dist/` would exist and contain up-to-date built packages. I.e. it was up to the user to have built the packages first (by running the appropriate script). However, many people not familiar with the `aio/` infrastructure assumed that `yarn build-local` or `yarn build-with-ivy` would take care of all the necessary steps. To avoid getting confusing errors (or worse yet, using outdated local packages), `NgPackagesInstaller` now has an option to build the packages before using them. One caveat is that the build script does not currently work on Windows, so a warning is printed instead, letting the user know they need to (somehow) take care of it themselves. NOTE 1: Since the build script is using bazel, running it should not be expensive if the directory is up-to-date (i.e. no changes have been made to source code after the last build). NOTE 2: This commit adds support for `--build-packages`, but does not change the default behavior (not building the packages). It will be turned on in a subsequent commit. PR Close #31985 --- aio/tools/ng-packages-installer/index.js | 57 +++++++++-- aio/tools/ng-packages-installer/index.spec.js | 94 +++++++++++++++---- 2 files changed, 129 insertions(+), 22 deletions(-) diff --git a/aio/tools/ng-packages-installer/index.js b/aio/tools/ng-packages-installer/index.js index a9952eba79..0fbc9d9dce 100644 --- a/aio/tools/ng-packages-installer/index.js +++ b/aio/tools/ng-packages-installer/index.js @@ -14,7 +14,8 @@ const LOCAL_MARKER_PATH = 'node_modules/_local_.json'; const PACKAGE_JSON_REGEX = /^[^/]+\/package\.json$/; const ANGULAR_ROOT_DIR = path.resolve(__dirname, '../../..'); -const ANGULAR_DIST_PACKAGES = path.resolve(ANGULAR_ROOT_DIR, 'dist/packages-dist'); +const ANGULAR_DIST_PACKAGES = path.join(ANGULAR_ROOT_DIR, 'dist/packages-dist'); +const ANGULAR_DIST_PACKAGES_BUILD_CMD = path.join(ANGULAR_ROOT_DIR, 'scripts/build-packages-dist.sh'); /** * A tool that can install Angular dependencies for a project from NPM or from the @@ -32,11 +33,14 @@ class NgPackagesInstaller { * @param {object} options - a hash of options for the install: * * `debug` (`boolean`) - whether to display debug messages. * * `force` (`boolean`) - whether to force a local installation even if there is a local marker file. + * * `buildPackages` (`boolean`) - whether to build the local Angular packages before using them. + * (NOTE: Building the packages is currently not supported on Windows, so a message is printed instead.) * * `ignorePackages` (`string[]`) - a collection of names of packages that should not be copied over. */ constructor(projectDir, options = {}) { this.debug = options.debug; this.force = options.force; + this.buildPackages = options.buildPackages; this.ignorePackages = options.ignorePackages || []; this.projectDir = path.resolve(projectDir); this.localMarkerPath = path.resolve(this.projectDir, LOCAL_MARKER_PATH); @@ -158,6 +162,31 @@ class NgPackagesInstaller { }); } + /** + * Build the local Angular packages. + * + * NOTE: + * Building the packages is currently not supported on Windows, so a message is printed instead, prompting the user to + * do it themselves (e.g. using Windows Subsystem for Linux or a docker container). + */ + _buildDistPackages() { + const canBuild = process.platform !== 'win32'; + + if (canBuild) { + this._log(`Building the Angular packages with: ${ANGULAR_DIST_PACKAGES_BUILD_CMD}`); + shelljs.exec(ANGULAR_DIST_PACKAGES_BUILD_CMD); + } else { + this._warn([ + 'Automatically building the local Angular packages is currently not supported on Windows.', + `Please, ensure '${ANGULAR_DIST_PACKAGES}' exists and is up-to-date (e.g. by running ` + + `'${ANGULAR_DIST_PACKAGES_BUILD_CMD}' in Git Bash for Windows, Windows Subsystem for Linux or a Linux ` + + 'docker container or VM).', + '', + 'Proceeding anyway...', + ].join('\n')); + } + } + _collectDependencies(dependencies, packages) { const peerDependencies = Object.create(null); const mergedDependencies = Object.assign(Object.create(null), dependencies); @@ -190,6 +219,10 @@ class NgPackagesInstaller { this._log(`Angular distributable directory: ${distDir}.`); + if (this.buildPackages) { + this._buildDistPackages(); + } + shelljs .find(distDir) .map(filePath => filePath.slice(distDir.length + 1)) @@ -251,17 +284,28 @@ class NgPackagesInstaller { const restoreCmd = `node ${relativeScriptPath} restore ${absoluteProjectDir}`; // Log a warning. + this._warn([ + `The project at "${absoluteProjectDir}" is running against the local Angular build.`, + '', + 'To restore the npm packages run:', + '', + ` "${restoreCmd}"`, + ].join('\n')); + } + + /** + * Log a warning message do draw user's attention. + * @param {...string[]} messages - The messages to be logged. + */ + _warn(...messages) { + const lines = messages.join(' ').split('\n'); console.warn(chalk.yellow([ '', '!'.repeat(110), '!!!', '!!! WARNING', '!!!', - `!!! The project at "${absoluteProjectDir}" is running against the local Angular build.`, - '!!!', - '!!! To restore the npm packages run:', - '!!!', - `!!! "${restoreCmd}"`, + ...lines.map(line => `!!! ${line}`), '!!!', '!'.repeat(110), '', @@ -294,6 +338,7 @@ function main() { .option('debug', { describe: 'Print additional debug information.', default: false }) .option('force', { describe: 'Force the command to execute even if not needed.', default: false }) + .option('build-packages', { describe: 'Build the local Angular packages, before using them.', default: false }) .option('ignore-packages', { describe: 'List of Angular packages that should not be used in local mode.', default: [], array: true }) .command('overwrite [--force] [--debug] [--ignore-packages package1 package2]', 'Install dependencies from the locally built Angular distributables.', () => {}, argv => { diff --git a/aio/tools/ng-packages-installer/index.spec.js b/aio/tools/ng-packages-installer/index.spec.js index 681abd1d9f..718ce7d26a 100644 --- a/aio/tools/ng-packages-installer/index.spec.js +++ b/aio/tools/ng-packages-installer/index.spec.js @@ -8,13 +8,14 @@ const shelljs = require('shelljs'); const NgPackagesInstaller = require('./index'); describe('NgPackagesInstaller', () => { - const rootDir = 'root/dir'; - const absoluteRootDir = path.resolve(rootDir); - const nodeModulesDir = path.resolve(absoluteRootDir, 'node_modules'); - const packageJsonPath = path.resolve(absoluteRootDir, 'package.json'); - const yarnLockPath = path.resolve(absoluteRootDir, 'yarn.lock'); - const packagesDir = path.resolve(path.resolve(__dirname, '../../../dist/packages-dist')); - const toolsDir = path.resolve(path.resolve(__dirname, '../../../dist/tools/@angular')); + const projectDir = 'root/dir'; + const absoluteProjectDir = path.resolve(projectDir); + const nodeModulesDir = path.resolve(absoluteProjectDir, 'node_modules'); + const packageJsonPath = path.resolve(absoluteProjectDir, 'package.json'); + const yarnLockPath = path.resolve(absoluteProjectDir, 'yarn.lock'); + const ngRootDir = path.resolve(__dirname, '../../..'); + const packagesDir = path.join(ngRootDir, 'dist/packages-dist'); + const toolsDir = path.join(ngRootDir, 'dist/tools/@angular'); let installer; beforeEach(() => { @@ -25,7 +26,7 @@ describe('NgPackagesInstaller', () => { spyOn(shelljs, 'rm'); spyOn(console, 'log'); spyOn(console, 'warn'); - installer = new NgPackagesInstaller(rootDir); + installer = new NgPackagesInstaller(projectDir); }); describe('checkDependencies()', () => { @@ -36,14 +37,14 @@ describe('NgPackagesInstaller', () => { it('should not print a warning if there is no _local_.json file', () => { fs.existsSync.and.returnValue(false); installer.checkDependencies(); - expect(fs.existsSync).toHaveBeenCalledWith(path.resolve(rootDir, 'node_modules/_local_.json')); + expect(fs.existsSync).toHaveBeenCalledWith(path.resolve(projectDir, 'node_modules/_local_.json')); expect(installer._printWarning).not.toHaveBeenCalled(); }); it('should print a warning if there is a _local_.json file', () => { fs.existsSync.and.returnValue(true); installer.checkDependencies(); - expect(fs.existsSync).toHaveBeenCalledWith(path.resolve(rootDir, 'node_modules/_local_.json')); + expect(fs.existsSync).toHaveBeenCalledWith(path.resolve(projectDir, 'node_modules/_local_.json')); expect(installer._printWarning).toHaveBeenCalled(); }); }); @@ -242,7 +243,68 @@ describe('NgPackagesInstaller', () => { }); }); + describe('_buildDistPackages()', () => { + // Call `_buildDistPackages()` with a mock `process.platform` value. + const buildDistPackagesOnPlatform = platform => { + const originalDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + Object.defineProperty(process, 'platform', {...originalDescriptor, value: platform}); + installer._buildDistPackages(); + Object.defineProperty(process, 'platform', originalDescriptor); + }; + + it('should build the local packages, when not on Windows', () => { + const buildScript = path.join(ngRootDir, 'scripts/build-packages-dist.sh'); + + buildDistPackagesOnPlatform('linux'); + expect(shelljs.exec).toHaveBeenCalledWith(buildScript); + + shelljs.exec.calls.reset(); + + buildDistPackagesOnPlatform('darwin'); + expect(shelljs.exec).toHaveBeenCalledWith(buildScript); + + shelljs.exec.calls.reset(); + + buildDistPackagesOnPlatform('anythingButWindows :('); + expect(shelljs.exec).toHaveBeenCalledWith(buildScript); + + // Ensure that the script does actually exist (e.g. it was not renamed/moved). + fs.existsSync.and.callThrough(); + expect(fs.existsSync(buildScript)).toBe(true); + }); + + it('should print a warning, when on Windows', () => { + buildDistPackagesOnPlatform('win32'); + const warning = console.warn.calls.argsFor(0)[0]; + + expect(shelljs.exec).not.toHaveBeenCalled(); + expect(warning).toContain( + 'Automatically building the local Angular packages is currently not supported on Windows.'); + expect(warning).toContain('Git Bash for Windows'); + expect(warning).toContain('Windows Subsystem for Linux'); + expect(warning).toContain('Linux docker container or VM'); + }); + }); + describe('_getDistPackages()', () => { + beforeEach(() => spyOn(NgPackagesInstaller.prototype, '_buildDistPackages')); + + it('should not build the local packages by default', () => { + installer._getDistPackages(); + expect(installer._buildDistPackages).not.toHaveBeenCalled(); + }); + + it('should build the local packages, if `buildPackages` is true', () => { + installer = new NgPackagesInstaller(projectDir, {buildPackages: true}); + installer._getDistPackages(); + expect(installer._buildDistPackages).toHaveBeenCalledTimes(1); + }); + + it('should not build the local packages by default', () => { + installer._getDistPackages(); + expect(installer._buildDistPackages).not.toHaveBeenCalled(); + }); + it('should include top level Angular packages', () => { const ngPackages = installer._getDistPackages(); const expectedValue = jasmine.objectContaining({ @@ -269,7 +331,7 @@ describe('NgPackagesInstaller', () => { }); it('should not include packages that have been ignored', () => { - installer = new NgPackagesInstaller(rootDir, { ignorePackages: ['@angular/router'] }); + installer = new NgPackagesInstaller(projectDir, { ignorePackages: ['@angular/router'] }); const ngPackages = installer._getDistPackages(); expect(ngPackages['@angular/common']).toBeDefined(); @@ -283,9 +345,9 @@ describe('NgPackagesInstaller', () => { }); it('should assign the debug property from the options', () => { - installer = new NgPackagesInstaller(rootDir, { debug: true }); + installer = new NgPackagesInstaller(projectDir, { debug: true }); expect(installer.debug).toBe(true); - installer = new NgPackagesInstaller(rootDir, { }); + installer = new NgPackagesInstaller(projectDir, { }); expect(installer.debug).toBe(undefined); }); @@ -350,7 +412,7 @@ describe('NgPackagesInstaller', () => { expect(console.warn.calls.argsFor(0)[0]).toMatch(restoreCmdRe1); // When run for a different directory... - const dir2 = rootDir; + const dir2 = projectDir; const restoreCmdRe2 = RegExp(`\\bnode .*?ng-packages-installer/index restore .*?${path.resolve(dir1)}\\b`); installer = new NgPackagesInstaller(dir2); installer._printWarning(''); @@ -361,14 +423,14 @@ describe('NgPackagesInstaller', () => { describe('_installDeps()', () => { it('should run yarn install with the given options', () => { installer._installDeps('option-1', 'option-2'); - expect(shelljs.exec).toHaveBeenCalledWith('yarn install option-1 option-2', { cwd: absoluteRootDir }); + expect(shelljs.exec).toHaveBeenCalledWith('yarn install option-1 option-2', { cwd: absoluteProjectDir }); }); }); describe('local marker helpers', () => { let installer; beforeEach(() => { - installer = new NgPackagesInstaller(rootDir); + installer = new NgPackagesInstaller(projectDir); }); describe('_checkLocalMarker', () => {