From cbdef6639664d1bd098d459a06dd2168b3a6db4b Mon Sep 17 00:00:00 2001 From: Sebastian McKenzie Date: Thu, 24 Nov 2016 15:48:09 +0000 Subject: [PATCH] [WIP] Add verbose flag - fixes #763 (#1994) * Add verbose flag - fixes #763 * fix lint * fix lint add verboseInspect method --- __tests__/commands/_helpers.js | 25 +++++++------ __tests__/commands/add.js | 4 +-- __tests__/commands/install.js | 10 +++--- __tests__/commands/self-update.js | 2 +- .../fixtures/request-cache/GET/localhost/.bin | 2 +- __tests__/lifecycle-scripts.js | 3 +- src/cli/index.js | 6 +++- src/fetchers/base-fetcher.js | 3 ++ src/fetchers/copy-fetcher.js | 2 +- src/fetchers/tarball-fetcher.js | 2 +- src/package-linker.js | 2 +- src/reporters/base-reporter.js | 18 ++++++++++ src/reporters/console/console-reporter.js | 8 +++++ src/reporters/json-reporter.js | 4 +++ src/reporters/lang/en.js | 11 ++++++ src/util/fs.js | 35 ++++++++++++++----- src/util/request-manager.js | 5 ++- 17 files changed, 105 insertions(+), 37 deletions(-) diff --git a/__tests__/commands/_helpers.js b/__tests__/commands/_helpers.js index 9424a2087b..44b72a550a 100644 --- a/__tests__/commands/_helpers.js +++ b/__tests__/commands/_helpers.js @@ -68,14 +68,24 @@ export async function run( checkInstalled: ?(config: Config, reporter: R, install: T) => ?Promise, beforeInstall: ?(cwd: string) => ?Promise, ): Promise { - const dir = path.join(fixturesLoc, name); + let out = ''; + const stdout = new stream.Writable({ + decodeStrings: false, + write(data, encoding, cb) { + out += data; + cb(); + }, + }); + const reporter = new Reporter({stdout, stderr: stdout}); + + const dir = path.join(fixturesLoc, name); const cwd = path.join( os.tmpdir(), `yarn-${path.basename(dir)}-${Math.random()}`, ); await fs.unlink(cwd); - await fs.copy(dir, cwd); + await fs.copy(dir, cwd, reporter); for (const {basename, absolute} of await fs.walk(cwd)) { if (basename.toLowerCase() === '.ds_store') { @@ -83,17 +93,6 @@ export async function run( } } - let out = ''; - const stdout = new stream.Writable({ - decodeStrings: false, - write(data, encoding, cb) { - out += data; - cb(); - }, - }); - - const reporter = new Reporter({stdout, stderr: stdout}); - if (beforeInstall) { await beforeInstall(cwd); } diff --git a/__tests__/commands/add.js b/__tests__/commands/add.js index 3910183e47..0933d3c641 100644 --- a/__tests__/commands/add.js +++ b/__tests__/commands/add.js @@ -154,8 +154,8 @@ test.concurrent('add with new dependency should be deterministic 3', (): Promise return runAdd([], {}, 'install-should-cleanup-when-package-json-changed-3', async (config, reporter) => { // expecting yarn check after installation not to fail - await fs.copy(path.join(config.cwd, 'yarn.lock.after'), path.join(config.cwd, 'yarn.lock')); - await fs.copy(path.join(config.cwd, 'package.json.after'), path.join(config.cwd, 'package.json')); + await fs.copy(path.join(config.cwd, 'yarn.lock.after'), path.join(config.cwd, 'yarn.lock'), reporter); + await fs.copy(path.join(config.cwd, 'package.json.after'), path.join(config.cwd, 'package.json'), reporter); const lockfile = await createLockfile(config.cwd); const install = new Install({}, config, reporter, lockfile); diff --git a/__tests__/commands/install.js b/__tests__/commands/install.js index 043f4818f3..3930eb0926 100644 --- a/__tests__/commands/install.js +++ b/__tests__/commands/install.js @@ -394,8 +394,8 @@ test.concurrent( await fs.unlink(path.join(config.cwd, 'yarn.lock')); await fs.unlink(path.join(config.cwd, 'package.json')); - await fs.copy(path.join(config.cwd, 'yarn.lock.after'), path.join(config.cwd, 'yarn.lock')); - await fs.copy(path.join(config.cwd, 'package.json.after'), path.join(config.cwd, 'package.json')); + await fs.copy(path.join(config.cwd, 'yarn.lock.after'), path.join(config.cwd, 'yarn.lock'), reporter); + await fs.copy(path.join(config.cwd, 'package.json.after'), path.join(config.cwd, 'package.json'), reporter); const reinstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd)); await reinstall.init(); @@ -426,8 +426,8 @@ test.concurrent( await fs.unlink(path.join(config.cwd, 'yarn.lock')); await fs.unlink(path.join(config.cwd, 'package.json')); - await fs.copy(path.join(config.cwd, 'yarn.lock.after'), path.join(config.cwd, 'yarn.lock')); - await fs.copy(path.join(config.cwd, 'package.json.after'), path.join(config.cwd, 'package.json')); + await fs.copy(path.join(config.cwd, 'yarn.lock.after'), path.join(config.cwd, 'yarn.lock'), reporter); + await fs.copy(path.join(config.cwd, 'package.json.after'), path.join(config.cwd, 'package.json'), reporter); const reinstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd)); await reinstall.init(); @@ -606,7 +606,7 @@ test.concurrent('install should update a dependency to yarn and mirror (PR impor '2.0.0', ); - await fs.copy(path.join(config.cwd, 'package.json.after'), path.join(config.cwd, 'package.json')); + await fs.copy(path.join(config.cwd, 'package.json.after'), path.join(config.cwd, 'package.json'), reporter); const reinstall = new Install({}, config, reporter, await Lockfile.fromDirectory(config.cwd)); await reinstall.init(); diff --git a/__tests__/commands/self-update.js b/__tests__/commands/self-update.js index 1361dd8dfb..b6583f42d3 100644 --- a/__tests__/commands/self-update.js +++ b/__tests__/commands/self-update.js @@ -83,7 +83,7 @@ xit('Self-update should work from self-updated location', (): Promise => { // mock an existing self-update await child.exec('npm run build'); const versionFolder = path.resolve(updatesFolder, '0.2.0'); - await fs.copy(path.resolve(updatesFolder, '..'), versionFolder); + await fs.copy(path.resolve(updatesFolder, '..'), versionFolder, reporter); await fs.symlink(versionFolder, path.resolve(updatesFolder, 'current')); let packageJson = await fs.readJson(path.resolve(updatesFolder, 'current', 'package.json')); packageJson.version = '0.2.0'; diff --git a/__tests__/fixtures/request-cache/GET/localhost/.bin b/__tests__/fixtures/request-cache/GET/localhost/.bin index 8b81374302..48da4c36df 100644 --- a/__tests__/fixtures/request-cache/GET/localhost/.bin +++ b/__tests__/fixtures/request-cache/GET/localhost/.bin @@ -1,5 +1,5 @@ HTTP/1.1 200 OK -Date: Thu, 24 Nov 2016 18:21:50 GMT +Date: Thu, 24 Nov 2016 18:27:00 GMT Connection: close Content-Length: 2 diff --git a/__tests__/lifecycle-scripts.js b/__tests__/lifecycle-scripts.js index 8632d9fb51..95b960b1ef 100644 --- a/__tests__/lifecycle-scripts.js +++ b/__tests__/lifecycle-scripts.js @@ -1,5 +1,6 @@ /* @flow */ +import NoopReporter from '../src/reporters/base-reporter.js'; import makeTemp from './_temp'; import * as fs from '../src/util/fs.js'; @@ -15,7 +16,7 @@ async function execCommand(cmd: string, packageName: string): Promise { const srcPackageDir = path.join(fixturesLoc, packageName); const packageDir = await makeTemp(packageName); - await fs.copy(srcPackageDir, packageDir); + await fs.copy(srcPackageDir, packageDir, new NoopReporter()); return new Promise((resolve, reject) => { const env = Object.assign({}, process.env); diff --git a/src/cli/index.js b/src/cli/index.js index 685c7bb817..209c07258c 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -41,6 +41,7 @@ for (let i = 0; i < args.length; i++) { // set global options commander.version(pkg.version); commander.usage('[command] [flags]'); +commander.option('--verbose', 'output verbose messages on internal operations'); commander.option('--offline', 'trigger an error if any required dependencies are not available in local cache'); commander.option('--prefer-offline', 'use network only if dependencies are not available in local cache'); commander.option('--strict-semver'); @@ -185,7 +186,8 @@ if (commander.json) { } const reporter = new Reporter({ emoji: commander.emoji && process.stdout.isTTY && process.platform === 'darwin', - noProgress: commander.noProgress, + verbose: commander.verbose, + noProgress: !commander.progress, }); reporter.initPeakMemoryCounter(); @@ -379,6 +381,8 @@ config.init({ return run().then(exit); } }).catch((err: Error) => { + reporter.verbose(err.stack); + if (err instanceof MessageError) { reporter.error(err.message); } else { diff --git a/src/fetchers/base-fetcher.js b/src/fetchers/base-fetcher.js index 388622550d..824844d628 100644 --- a/src/fetchers/base-fetcher.js +++ b/src/fetchers/base-fetcher.js @@ -1,6 +1,7 @@ /* @flow */ /* eslint no-unused-vars: 0 */ +import type Reporter from '../reporters/base-reporter.js'; import type {PackageRemote, FetchedMetadata, FetchedOverride} from '../types.js'; import type {RegistryNames} from '../registries/index.js'; import type Config from '../config.js'; @@ -11,6 +12,7 @@ const path = require('path'); export default class BaseFetcher { constructor(dest: string, remote: PackageRemote, config: Config) { + this.reporter = config.reporter; this.reference = remote.reference; this.registry = remote.registry; this.hash = remote.hash; @@ -19,6 +21,7 @@ export default class BaseFetcher { this.dest = dest; } + reporter: Reporter; remote: PackageRemote; registry: RegistryNames; reference: string; diff --git a/src/fetchers/copy-fetcher.js b/src/fetchers/copy-fetcher.js index 9c8f89e552..1dca5c1624 100644 --- a/src/fetchers/copy-fetcher.js +++ b/src/fetchers/copy-fetcher.js @@ -6,7 +6,7 @@ import * as fs from '../util/fs.js'; export default class CopyFetcher extends BaseFetcher { async _fetch(): Promise { - await fs.copy(this.reference, this.dest); + await fs.copy(this.reference, this.dest, this.reporter); return { hash: this.hash || '', resolved: null, diff --git a/src/fetchers/tarball-fetcher.js b/src/fetchers/tarball-fetcher.js index 95d6f24b3f..46b0d143d1 100644 --- a/src/fetchers/tarball-fetcher.js +++ b/src/fetchers/tarball-fetcher.js @@ -30,7 +30,7 @@ export default class TarballFetcher extends BaseFetcher { // copy the file over if (!await fsUtil.exists(mirrorPath)) { - await fsUtil.copy(tarballLoc, mirrorPath); + await fsUtil.copy(tarballLoc, mirrorPath, this.reporter); } const relativeMirrorPath = this.getRelativeMirrorPath(mirrorPath); diff --git a/src/package-linker.js b/src/package-linker.js index 346ff6fd7b..b637348374 100644 --- a/src/package-linker.js +++ b/src/package-linker.js @@ -174,7 +174,7 @@ export default class PackageLinker { // let tick; - await fs.copyBulk(Array.from(queue.values()), { + await fs.copyBulk(Array.from(queue.values()), this.reporter, { possibleExtraneous, phantomFiles, diff --git a/src/reporters/base-reporter.js b/src/reporters/base-reporter.js index 4018c193ae..7a99644d3b 100644 --- a/src/reporters/base-reporter.js +++ b/src/reporters/base-reporter.js @@ -22,6 +22,7 @@ const util = require('util'); type Language = $Keys; export type ReporterOptions = { + verbose?: boolean, language?: Language, stdout?: Stdout, stderr?: Stdout, @@ -54,6 +55,7 @@ export default class BaseReporter { this.stdin = opts.stdin || process.stdin; this.emoji = !!opts.emoji; this.noProgress = !!opts.noProgress || isCI; + this.isVerbose = !!opts.verbose; // $FlowFixMe: this is valid! this.isTTY = this.stdout.isTTY; @@ -71,6 +73,7 @@ export default class BaseReporter { isTTY: boolean; emoji: boolean; noProgress: boolean; + isVerbose: boolean; format: Formatter; peakMemoryInterval: ?number; @@ -92,6 +95,21 @@ export default class BaseReporter { }); } + verbose(msg: string) { + if (this.isVerbose) { + this._verbose(msg); + } + } + + verboseInspect(val: any) { + if (this.isVerbose) { + this._verboseInspect(val); + } + } + + _verbose(msg: string) {} + _verboseInspect(val: any) {} + initPeakMemoryCounter() { this.checkPeakMemory(); this.peakMemoryInterval = setInterval(() => { diff --git a/src/reporters/console/console-reporter.js b/src/reporters/console/console-reporter.js index 214475f144..94a0975b0e 100644 --- a/src/reporters/console/console-reporter.js +++ b/src/reporters/console/console-reporter.js @@ -46,6 +46,14 @@ export default class ConsoleReporter extends BaseReporter { this._log(`${this.format[color](category)} ${msg}`); } + _verbose(msg: string) { + this._logCategory('verbose', 'grey', msg); + } + + _verboseInspect(obj: any) { + this.inspect(obj); + } + table(head: Array, body: Array) { // head = head.map((field: string): string => this.format.underline(field)); diff --git a/src/reporters/json-reporter.js b/src/reporters/json-reporter.js index fe3b100fa6..de79dfe4fc 100644 --- a/src/reporters/json-reporter.js +++ b/src/reporters/json-reporter.js @@ -22,6 +22,10 @@ export default class JSONReporter extends BaseReporter { stdout.write(`${JSON.stringify({type, data})}\n`); } + _verbose(msg: string) { + this._dump('verbose', msg); + } + list(type: string, items: Array) { this._dump('list', {type, items}); } diff --git a/src/reporters/lang/en.js b/src/reporters/lang/en.js index 878bd10ea4..81c1d310b9 100644 --- a/src/reporters/lang/en.js +++ b/src/reporters/lang/en.js @@ -32,6 +32,17 @@ const messages = { manifestDependencyBuiltin: 'Dependency $0 listed in $1 is the name of a built-in module', manifestDependencyCollision: '$0 has dependency $1 with range $2 that collides with a dependency in $3 of the same name with version $4', + verboseFileCopy: 'Copying $0 to $1.', + verboseFileSymlink: 'Creating symlink at $0 to $1.', + verboseFileSkip: 'Skipping copying of file $0 as the file at $1 is the same size ($2) and mtime ($3).', + verboseFileSkipSymlink: 'Skipping copying of $0 as the file at $1 is the same symlink ($2).', + verboseFileRemoveExtraneous: 'Removing extraneous file $0.', + verboseFilePhantomExtraneous: "File $0 would be marked as extraneous but has been removed as it's listed as a phantom file.", + verboseFileFolder: 'Creating directory $0.', + + verboseRequestStart: 'Performing $0 request to $1.', + verboseRequestFinish: 'Request $0 finished with status code $1.', + configSet: 'Set $0 to $1.', configDelete: 'Deleted $0.', configNpm: 'npm config', diff --git a/src/util/fs.js b/src/util/fs.js index 7259ae2f92..af62978667 100644 --- a/src/util/fs.js +++ b/src/util/fs.js @@ -1,5 +1,6 @@ /* @flow */ +import type Reporter from '../reporters/base-reporter.js'; import BlockingQueue from './blocking-queue.js'; import * as promise from './promise.js'; import {promisify} from './promise.js'; @@ -74,6 +75,7 @@ async function buildActionsForCopy( queue: CopyQueue, events: CopyOptions, possibleExtraneousSeed: PossibleExtraneous, + reporter: Reporter, ): Promise { const possibleExtraneous: Set = new Set(possibleExtraneousSeed || []); const phantomFiles: Set = new Set(events.phantomFiles || []); @@ -104,13 +106,17 @@ async function buildActionsForCopy( // simulate the existence of some files to prevent considering them extraenous for (const file of phantomFiles) { - possibleExtraneous.delete(file); + if (possibleExtraneous.has(file)) { + reporter.verbose(reporter.lang('verboseFilePhantomExtraneous', file)); + possibleExtraneous.delete(file); + } } // remove all extraneous files that weren't in the tree if (!noExtraneous) { for (const loc of possibleExtraneous) { if (!files.has(loc)) { + reporter.verbose(reporter.lang('verboseFileRemoveExtraneous', loc)); await unlink(loc); } } @@ -156,13 +162,18 @@ async function buildActionsForCopy( if (bothFiles && srcStat.size === destStat.size && +srcStat.mtime === +destStat.mtime) { // we can safely assume this is the same file onDone(); + reporter.verbose(reporter.lang('verboseFileSkip', src, dest, srcStat.size, +srcStat.mtime)); return; } - if (bothSymlinks && await readlink(src) === await readlink(dest)) { - // if both symlinks are the same then we can continue on - onDone(); - return; + if (bothSymlinks) { + const srcReallink = await readlink(src); + if (srcReallink === await readlink(dest)) { + // if both symlinks are the same then we can continue on + onDone(); + reporter.verbose(reporter.lang('verboseFileSkipSymlink', src, dest, srcReallink)); + return; + } } if (bothFolders && !noExtraneous) { @@ -195,6 +206,7 @@ async function buildActionsForCopy( }); onDone(); } else if (srcStat.isDirectory()) { + reporter.verbose(reporter.lang('verboseFileFolder', dest)); await mkdirp(dest); const destParts = dest.split(path.sep); @@ -238,12 +250,13 @@ async function buildActionsForCopy( } } -export function copy(src: string, dest: string): Promise { - return copyBulk([{src, dest}]); +export function copy(src: string, dest: string, reporter: Reporter): Promise { + return copyBulk([{src, dest}], reporter); } export async function copyBulk( queue: CopyQueue, + reporter: Reporter, _events?: { onProgress?: ?(dest: string) => void, onStart?: ?(num: number) => void, @@ -260,7 +273,7 @@ export async function copyBulk( phantomFiles: (_events && _events.phantomFiles) || [], }; - const actions: CopyActions = await buildActionsForCopy(queue, events, events.possibleExtraneous); + const actions: CopyActions = await buildActionsForCopy(queue, events, events.possibleExtraneous, reporter); events.onStart(actions.length); const fileActions: Array = (actions.filter((action) => action.type === 'file'): any); @@ -278,6 +291,8 @@ export async function copyBulk( const readStream = fs.createReadStream(data.src); const writeStream = fs.createWriteStream(data.dest, {mode: data.mode}); + reporter.verbose(reporter.lang('verboseFileCopy', data.src, data.dest)); + readStream.on('error', reject); writeStream.on('error', reject); @@ -308,7 +323,9 @@ export async function copyBulk( // we need to copy symlinks last as the could reference files we were copying const symlinkActions: Array = (actions.filter((action) => action.type === 'symlink'): any); await promise.queue(symlinkActions, (data): Promise => { - return symlink(path.resolve(path.dirname(data.dest), data.linkname), data.dest); + const linkname = path.resolve(path.dirname(data.dest), data.linkname); + reporter.verbose(reporter.lang('verboseFileSymlink', data.dest, linkname)); + return symlink(linkname, data.dest); }); } diff --git a/src/util/request-manager.js b/src/util/request-manager.js index 12d9ae40d5..ebc0285c0d 100644 --- a/src/util/request-manager.js +++ b/src/util/request-manager.js @@ -342,7 +342,7 @@ export default class RequestManager { if (!params.process) { const parts = url.parse(params.url); - params.callback = function(err, res, body) { + params.callback = (err, res, body) => { if (err) { onError(err); return; @@ -350,6 +350,8 @@ export default class RequestManager { successHosts[parts.hostname] = true; + this.reporter.verbose(this.reporter.lang('verboseRequestFinish', params.url, res.statusCode)); + if (body && typeof body.error === 'string') { reject(new Error(body.error)); return; @@ -393,6 +395,7 @@ export default class RequestManager { const request = this._getRequestModule(); const req = request(params); + this.reporter.verbose(this.reporter.lang('verboseRequestStart', params.method, params.url)); req.on('error', onError);