Skip to content

Commit

Permalink
[WIP] Add verbose flag - fixes #763 (#1994)
Browse files Browse the repository at this point in the history
* Add verbose flag - fixes #763

* fix lint

* fix lint add verboseInspect method
  • Loading branch information
Sebastian McKenzie authored and bestander committed Nov 24, 2016
1 parent fc94696 commit cbdef66
Show file tree
Hide file tree
Showing 17 changed files with 105 additions and 37 deletions.
25 changes: 12 additions & 13 deletions __tests__/commands/_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,32 +68,31 @@ export async function run<T, R>(
checkInstalled: ?(config: Config, reporter: R, install: T) => ?Promise<void>,
beforeInstall: ?(cwd: string) => ?Promise<void>,
): Promise<void> {
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') {
await fs.unlink(absolute);
}
}

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);
}
Expand Down
4 changes: 2 additions & 2 deletions __tests__/commands/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions __tests__/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion __tests__/commands/self-update.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ xit('Self-update should work from self-updated location', (): Promise<void> => {
// 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';
Expand Down
2 changes: 1 addition & 1 deletion __tests__/fixtures/request-cache/GET/localhost/.bin
Original file line number Diff line number Diff line change
@@ -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

Expand Down
3 changes: 2 additions & 1 deletion __tests__/lifecycle-scripts.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -15,7 +16,7 @@ async function execCommand(cmd: string, packageName: string): Promise<string> {
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);
Expand Down
6 changes: 5 additions & 1 deletion src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions src/fetchers/base-fetcher.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
Expand All @@ -19,6 +21,7 @@ export default class BaseFetcher {
this.dest = dest;
}

reporter: Reporter;
remote: PackageRemote;
registry: RegistryNames;
reference: string;
Expand Down
2 changes: 1 addition & 1 deletion src/fetchers/copy-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as fs from '../util/fs.js';

export default class CopyFetcher extends BaseFetcher {
async _fetch(): Promise<FetchedOverride> {
await fs.copy(this.reference, this.dest);
await fs.copy(this.reference, this.dest, this.reporter);
return {
hash: this.hash || '',
resolved: null,
Expand Down
2 changes: 1 addition & 1 deletion src/fetchers/tarball-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/package-linker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
18 changes: 18 additions & 0 deletions src/reporters/base-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const util = require('util');
type Language = $Keys<typeof languages>;

export type ReporterOptions = {
verbose?: boolean,
language?: Language,
stdout?: Stdout,
stderr?: Stdout,
Expand Down Expand Up @@ -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;
Expand All @@ -71,6 +73,7 @@ export default class BaseReporter {
isTTY: boolean;
emoji: boolean;
noProgress: boolean;
isVerbose: boolean;
format: Formatter;

peakMemoryInterval: ?number;
Expand All @@ -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(() => {
Expand Down
8 changes: 8 additions & 0 deletions src/reporters/console/console-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>, body: Array<Row>) {
//
head = head.map((field: string): string => this.format.underline(field));
Expand Down
4 changes: 4 additions & 0 deletions src/reporters/json-reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>) {
this._dump('list', {type, items});
}
Expand Down
11 changes: 11 additions & 0 deletions src/reporters/lang/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
35 changes: 26 additions & 9 deletions src/util/fs.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -74,6 +75,7 @@ async function buildActionsForCopy(
queue: CopyQueue,
events: CopyOptions,
possibleExtraneousSeed: PossibleExtraneous,
reporter: Reporter,
): Promise<CopyActions> {
const possibleExtraneous: Set<string> = new Set(possibleExtraneousSeed || []);
const phantomFiles: Set<string> = new Set(events.phantomFiles || []);
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -238,12 +250,13 @@ async function buildActionsForCopy(
}
}

export function copy(src: string, dest: string): Promise<void> {
return copyBulk([{src, dest}]);
export function copy(src: string, dest: string, reporter: Reporter): Promise<void> {
return copyBulk([{src, dest}], reporter);
}

export async function copyBulk(
queue: CopyQueue,
reporter: Reporter,
_events?: {
onProgress?: ?(dest: string) => void,
onStart?: ?(num: number) => void,
Expand All @@ -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<CopyFileAction> = (actions.filter((action) => action.type === 'file'): any);
Expand All @@ -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);

Expand Down Expand Up @@ -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<CopySymlinkAction> = (actions.filter((action) => action.type === 'symlink'): any);
await promise.queue(symlinkActions, (data): Promise<void> => {
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);
});
}

Expand Down
Loading

0 comments on commit cbdef66

Please sign in to comment.