From d7212eb419ce5c7f265a3559f1a57c2b2bd43a65 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Thu, 30 Aug 2018 01:11:20 -0400 Subject: [PATCH] Reworks git commands a bit to reduce rethrows Fixes #405 - stops non-repo from showing up in explorer --- CHANGELOG.md | 1 + src/annotations/gutterBlameAnnotationProvider.ts | 3 +- src/annotations/heatmapBlameAnnotationProvider.ts | 4 +- src/annotations/recentChangesAnnotationProvider.ts | 6 +- src/extension.ts | 7 +- src/git/git.ts | 308 ++++++++++----------- src/git/gitLocator.ts | 8 +- src/git/shell.ts | 60 ++-- src/gitService.ts | 20 +- src/logger.ts | 6 +- src/system/string.ts | 5 + src/telemetry.ts | 6 +- 12 files changed, 225 insertions(+), 209 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0cc1d4..6b70f85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ### Fixed +- Fixes [#405](https://github.com/eamodio/vscode-gitlens/issues/405) - Secondary, blank repository appears repeatedly in gitExplorer view - Fixes issues with git log caching ### Removed diff --git a/src/annotations/gutterBlameAnnotationProvider.ts b/src/annotations/gutterBlameAnnotationProvider.ts index ad50483..0387f8a 100644 --- a/src/annotations/gutterBlameAnnotationProvider.ts +++ b/src/annotations/gutterBlameAnnotationProvider.ts @@ -142,8 +142,7 @@ export class GutterBlameAnnotationProvider extends BlameAnnotationProviderBase { } } - const duration = process.hrtime(start); - Logger.log(`${duration[0] * 1000 + Math.floor(duration[1] / 1000000)} ms to compute gutter blame annotations`); + Logger.log(`${Strings.getDurationMilliseconds(start)} ms to compute gutter blame annotations`); this.registerHoverProviders(Container.config.hovers.annotations); void this.selection(shaOrLine, blame); diff --git a/src/annotations/heatmapBlameAnnotationProvider.ts b/src/annotations/heatmapBlameAnnotationProvider.ts index 20ee13a..81aca05 100644 --- a/src/annotations/heatmapBlameAnnotationProvider.ts +++ b/src/annotations/heatmapBlameAnnotationProvider.ts @@ -4,6 +4,7 @@ import { FileAnnotationType } from '../configuration'; import { Container } from '../container'; import { GitBlameCommit } from '../gitService'; import { Logger } from '../logger'; +import { Strings } from '../system'; import { Annotations } from './annotations'; import { BlameAnnotationProviderBase } from './blameAnnotationProvider'; @@ -55,8 +56,7 @@ export class HeatmapBlameAnnotationProvider extends BlameAnnotationProviderBase this.editor.setDecorations(this.decoration!, this.decorations); } - const duration = process.hrtime(start); - Logger.log(`${duration[0] * 1000 + Math.floor(duration[1] / 1000000)} ms to compute heatmap annotations`); + Logger.log(`${Strings.getDurationMilliseconds(start)} ms to compute heatmap annotations`); this.registerHoverProviders(Container.config.hovers.annotations); void this.selection(shaOrLine, blame); diff --git a/src/annotations/recentChangesAnnotationProvider.ts b/src/annotations/recentChangesAnnotationProvider.ts index cb15a5d..91481ef 100644 --- a/src/annotations/recentChangesAnnotationProvider.ts +++ b/src/annotations/recentChangesAnnotationProvider.ts @@ -4,6 +4,7 @@ import { FileAnnotationType } from '../configuration'; import { Container } from '../container'; import { GitUri } from '../gitService'; import { Logger } from '../logger'; +import { Strings } from '../system'; import { GitDocumentState, TrackedDocument } from '../trackers/gitDocumentTracker'; import { AnnotationProviderBase } from './annotationProvider'; import { Annotations } from './annotations'; @@ -82,10 +83,7 @@ export class RecentChangesAnnotationProvider extends AnnotationProviderBase { this.editor.setDecorations(this.decoration, this.decorations); - const duration = process.hrtime(start); - Logger.log( - `${duration[0] * 1000 + Math.floor(duration[1] / 1000000)} ms to compute recent changes annotations` - ); + Logger.log(`${Strings.getDurationMilliseconds(start)} ms to compute recent changes annotations`); return true; } diff --git a/src/extension.ts b/src/extension.ts index bf8a17c..bb16880 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -13,12 +13,12 @@ import { KeyMap, OutputLevel } from './configuration'; -import { CommandContext, extensionQualifiedId, GlobalState, setCommandContext } from './constants'; +import { CommandContext, extensionQualifiedId, GlobalState, GlyphChars, setCommandContext } from './constants'; import { Container } from './container'; import { GitService } from './gitService'; import { Logger } from './logger'; import { Messages } from './messages'; -import { Versions } from './system'; +import { Strings, Versions } from './system'; // import { Telemetry } from './telemetry'; export async function activate(context: ExtensionContext) { @@ -83,8 +83,7 @@ export async function activate(context: ExtensionContext) { // Constantly over my data cap so stop collecting initialized event // Telemetry.trackEvent('initialized', Objects.flatten(cfg, 'config', true)); - const duration = process.hrtime(start); - Logger.log(`GitLens(v${gitlensVersion}) activated in ${duration[0] * 1000 + Math.floor(duration[1] / 1000000)} ms`); + Logger.log(`GitLens(v${gitlensVersion}) activated ${GlyphChars.Dot} ${Strings.getDurationMilliseconds(start)} ms`); } export function deactivate() {} diff --git a/src/git/git.ts b/src/git/git.ts index 8e69653..1cbf8ee 100644 --- a/src/git/git.ts +++ b/src/git/git.ts @@ -2,10 +2,11 @@ import * as fs from 'fs'; import * as iconv from 'iconv-lite'; import * as path from 'path'; +import { GlyphChars } from '../constants'; import { Logger } from '../logger'; import { Objects, Strings } from '../system'; import { findGitPath, IGitInfo } from './gitLocator'; -import { CommandOptions, runCommand } from './shell'; +import { run, RunOptions } from './shell'; export { IGitInfo }; export * from './models/models'; @@ -63,28 +64,18 @@ const GitWarnings = { unknownRevision: /ambiguous argument \'.*?\': unknown revision or path not in the working tree|not stored as a remote-tracking branch/i }; -async function gitCommand( - options: CommandOptions & { readonly correlationKey?: string }, - ...args: any[] -): Promise { - try { - return await gitCommandCore(options, ...args); - } - catch (ex) { - return gitCommandDefaultErrorHandler(ex, options, ...args); - } +interface GitCommandOptions extends RunOptions { + readonly correlationKey?: string; + exceptionHandler?(ex: Error): string; } // A map of running git commands -- avoids running duplicate overlaping commands const pendingCommands: Map> = new Map(); -async function gitCommandCore( - options: CommandOptions & { readonly correlationKey?: string }, - ...args: any[] -): Promise { +async function git(options: GitCommandOptions, ...args: any[]): Promise { const start = process.hrtime(); - const { correlationKey, ...opts } = options; + const { correlationKey, exceptionHandler, ...opts } = options; const encoding = options.encoding || 'utf8'; const runOpts = { @@ -93,52 +84,61 @@ async function gitCommandCore( // Adds GCM environment variables to avoid any possible credential issues -- from https://github.com/Microsoft/vscode/issues/26573#issuecomment-338686581 // Shouldn't *really* be needed but better safe than sorry env: { ...(options.env || process.env), GCM_INTERACTIVE: 'NEVER', GCM_PRESERVE_CREDS: 'TRUE', LC_ALL: 'C' } - } as CommandOptions; + } as RunOptions; - const gitCommand = `git ${args.join(' ')}`; - const command = `(${runOpts.cwd}${correlationKey !== undefined ? correlationKey : ''}): ${gitCommand}`; + const gitCommand = `[${runOpts.cwd}] git ${args.join(' ')}`; + const command = `${correlationKey !== undefined ? `${correlationKey}:` : ''}${gitCommand}`; + + let waiting; let promise = pendingCommands.get(command); if (promise === undefined) { - Logger.log(`Running${command}`); + waiting = false; // Fixes https://github.com/eamodio/vscode-gitlens/issues/73 & https://github.com/eamodio/vscode-gitlens/issues/161 // See https://stackoverflow.com/questions/4144417/how-to-handle-asian-characters-in-file-names-in-git-on-os-x args.splice(0, 0, '-c', 'core.quotepath=false', '-c', 'color.ui=false'); - promise = runCommand(gitInfo.path, args, runOpts); + promise = run(gitInfo.path, args, encoding, runOpts); pendingCommands.set(command, promise); } else { - Logger.log(`Awaiting${command}`); + waiting = true; } - let data: string; let exception: Error | undefined; try { - data = await promise; + return await promise; } catch (ex) { exception = ex; - throw ex; + if (exceptionHandler !== undefined) { + const result = exceptionHandler(ex); + exception = undefined; + return result; + } + + const result = defaultExceptionHandler(ex, options, ...args); + exception = undefined; + return result; } finally { pendingCommands.delete(command); - const duration = process.hrtime(start); - const completedIn = `${exception === undefined ? 'Completed' : 'FAILED'} in ${duration[0] * 1000 + - Math.floor(duration[1] / 1000000)} ms`; - - Logger.log(`${exception === undefined ? 'Completed' : 'FAILED'}${command} ${completedIn}`); - Logger.logGitCommand(`${gitCommand} ${completedIn}`, runOpts.cwd!, exception); + const duration = `${Strings.getDurationMilliseconds(start)} ms ${waiting ? '(await) ' : ''}`; + Logger.log( + `${gitCommand} ${GlyphChars.Dot} ${ + exception !== undefined ? `FAILED(${(exception.message || '').trim().split('\n', 1)[0]}) ` : '' + }${duration}` + ); + Logger.logGitCommand( + `${gitCommand} ${GlyphChars.Dot} ${exception !== undefined ? 'FAILED ' : ''}${duration}`, + exception + ); } - - if (encoding === 'utf8' || encoding === 'binary') return data; - - return iconv.decode(Buffer.from(data, 'binary'), encoding); } -function gitCommandDefaultErrorHandler(ex: Error, options: CommandOptions, ...args: any[]): string { +function defaultExceptionHandler(ex: Error, options: GitCommandOptions, ...args: any[]): string { const msg = ex && ex.toString(); if (msg) { for (const warning of Objects.values(GitWarnings)) { @@ -153,6 +153,14 @@ function gitCommandDefaultErrorHandler(ex: Error, options: CommandOptions, ...ar throw ex; } +function ignoreExceptionsHandler() { + return ''; +} + +function throwExceptionHandler(ex: Error) { + throw ex; +} + let gitInfo: IGitInfo; export class Git { @@ -180,10 +188,10 @@ export class Git { gitInfo = await findGitPath(gitPath); - const duration = process.hrtime(start); Logger.log( - `Git found: ${gitInfo.version} @ ${gitInfo.path === 'git' ? 'PATH' : gitInfo.path} in ${duration[0] * 1000 + - Math.floor(duration[1] / 1000000)} ms` + `Git found: ${gitInfo.version} @ ${gitInfo.path === 'git' ? 'PATH' : gitInfo.path} ${ + GlyphChars.Dot + } ${Strings.getDurationMilliseconds(start)} ms` ); } @@ -323,7 +331,7 @@ export class Git { } } - return gitCommand({ cwd: root, stdin: stdin }, ...params, '--', file); + return git({ cwd: root, stdin: stdin }, ...params, '--', file); } static async blame_contents( @@ -355,12 +363,7 @@ export class Git { // Pipe the blame contents to stdin params.push('--contents', '-'); - return gitCommand( - { cwd: root, stdin: contents, correlationKey: options.correlationKey }, - ...params, - '--', - file - ); + return git({ cwd: root, stdin: contents, correlationKey: options.correlationKey }, ...params, '--', file); } static branch(repoPath: string, options: { all: boolean } = { all: false }) { @@ -369,7 +372,7 @@ export class Git { params.push('-a'); } - return gitCommand({ cwd: repoPath }, ...params); + return git({ cwd: repoPath }, ...params); } static branch_contains(repoPath: string, ref: string, options: { remote: boolean } = { remote: false }) { @@ -378,33 +381,33 @@ export class Git { params.push('-r'); } - return gitCommand({ cwd: repoPath }, ...params, ref); + return git({ cwd: repoPath }, ...params, ref); } static checkout(repoPath: string, fileName: string, sha: string) { const [file, root] = Git.splitPath(fileName, repoPath); - return gitCommand({ cwd: root }, 'checkout', sha, '--', file); + return git({ cwd: root }, 'checkout', sha, '--', file); } static async config_get(key: string, repoPath?: string) { - try { - const data = await gitCommandCore({ cwd: repoPath || '' }, 'config', '--get', key); - return data.trim(); - } - catch { - return undefined; - } + const data = await git( + { cwd: repoPath || '', exceptionHandler: ignoreExceptionsHandler }, + 'config', + '--get', + key + ); + return data === '' ? undefined : data.trim(); } static async config_getRegex(pattern: string, repoPath?: string) { - try { - const data = await gitCommandCore({ cwd: repoPath || '' }, 'config', '--get-regex', pattern); - return data.trim(); - } - catch { - return undefined; - } + const data = await git( + { cwd: repoPath || '', exceptionHandler: ignoreExceptionsHandler }, + 'config', + '--get-regex', + pattern + ); + return data === '' ? undefined : data.trim(); } static diff(repoPath: string, fileName: string, sha1?: string, sha2?: string, options: { encoding?: string } = {}) { @@ -417,7 +420,7 @@ export class Git { } const encoding: BufferEncoding = options.encoding === 'utf8' ? 'utf8' : 'binary'; - return gitCommand({ cwd: repoPath, encoding: encoding }, ...params, '--', fileName); + return git({ cwd: repoPath, encoding: encoding }, ...params, '--', fileName); } static diff_nameStatus(repoPath: string, sha1?: string, sha2?: string, options: { filter?: string } = {}) { @@ -432,7 +435,7 @@ export class Git { params.push(sha2); } - return gitCommand({ cwd: repoPath }, ...params); + return git({ cwd: repoPath }, ...params); } static diff_shortstat(repoPath: string, sha?: string) { @@ -440,7 +443,7 @@ export class Git { if (sha) { params.push(sha); } - return gitCommand({ cwd: repoPath }, ...params); + return git({ cwd: repoPath }, ...params); } static difftool_dirDiff(repoPath: string, tool: string, ref1: string, ref2?: string) { @@ -449,7 +452,7 @@ export class Git { params.push(ref2); } - return gitCommand({ cwd: repoPath }, ...params); + return git({ cwd: repoPath }, ...params); } static difftool_fileDiff(repoPath: string, fileName: string, tool: string, staged: boolean) { @@ -459,7 +462,7 @@ export class Git { } params.push('--', fileName); - return gitCommand({ cwd: repoPath }, ...params); + return git({ cwd: repoPath }, ...params); } static log(repoPath: string, options: { maxCount?: number; ref?: string; reverse?: boolean }) { @@ -475,7 +478,7 @@ export class Git { params.push(options.ref); } } - return gitCommand({ cwd: repoPath }, ...params, '--'); + return git({ cwd: repoPath }, ...params, '--'); } static log_file( @@ -514,36 +517,34 @@ export class Git { params.push(`-L ${options.startLine},${options.endLine}:${file}`); } - return gitCommand({ cwd: root }, ...params, '--', file); + return git({ cwd: root }, ...params, '--', file); } static async log_recent(repoPath: string, fileName: string) { - try { - const data = await gitCommandCore({ cwd: repoPath }, 'log', '-M', '-n1', '--format=%H', '--', fileName); - return data.trim(); - } - catch { - return undefined; - } + const data = await git( + { cwd: repoPath, exceptionHandler: ignoreExceptionsHandler }, + 'log', + '-M', + '-n1', + '--format=%H', + '--', + fileName + ); + return data === '' ? undefined : data.trim(); } static async log_resolve(repoPath: string, fileName: string, ref: string) { - try { - const data = await gitCommandCore( - { cwd: repoPath }, - 'log', - '-M', - '-n1', - '--format=%H', - ref, - '--', - fileName - ); - return data.trim(); - } - catch { - return undefined; - } + const data = await git( + { cwd: repoPath, exceptionHandler: ignoreExceptionsHandler }, + 'log', + '-M', + '-n1', + '--format=%H', + ref, + '--', + fileName + ); + return data === '' ? undefined : data.trim(); } static log_search(repoPath: string, search: string[] = [], options: { maxCount?: number } = {}) { @@ -552,7 +553,7 @@ export class Git { params.push(`-n${options.maxCount}`); } - return gitCommand({ cwd: repoPath }, ...params, ...search); + return git({ cwd: repoPath }, ...params, ...search); } static log_shortstat(repoPath: string, options: { ref?: string }) { @@ -560,22 +561,21 @@ export class Git { if (options.ref && !Git.isStagedUncommitted(options.ref)) { params.push(options.ref); } - return gitCommand({ cwd: repoPath }, ...params, '--'); + return git({ cwd: repoPath }, ...params, '--'); } - static async ls_files(repoPath: string, fileName: string, options: { ref?: string } = {}): Promise { + static async ls_files( + repoPath: string, + fileName: string, + options: { ref?: string } = {} + ): Promise { const params = ['ls-files']; if (options.ref && !Git.isStagedUncommitted(options.ref)) { params.push(`--with-tree=${options.ref}`); } - try { - const data = await gitCommandCore({ cwd: repoPath }, ...params, fileName); - return data.trim(); - } - catch { - return ''; - } + const data = await git({ cwd: repoPath, exceptionHandler: ignoreExceptionsHandler }, ...params, fileName); + return data === '' ? undefined : data.trim(); } static merge_base(repoPath: string, ref1: string, ref2: string, options: { forkPoint?: boolean } = {}) { @@ -584,79 +584,73 @@ export class Git { params.push('--fork-point'); } - return gitCommand({ cwd: repoPath }, ...params, ref1, ref2); + return git({ cwd: repoPath }, ...params, ref1, ref2); } static remote(repoPath: string): Promise { - return gitCommand({ cwd: repoPath }, 'remote', '-v'); + return git({ cwd: repoPath }, 'remote', '-v'); } static remote_url(repoPath: string, remote: string): Promise { - return gitCommand({ cwd: repoPath }, 'remote', 'get-url', remote); + return git({ cwd: repoPath }, 'remote', 'get-url', remote); } static async revparse(repoPath: string, ref: string): Promise { - try { - const data = await gitCommandCore({ cwd: repoPath }, 'rev-parse', ref); - return data.trim(); - } - catch { - return undefined; - } + const data = await git({ cwd: repoPath, exceptionHandler: ignoreExceptionsHandler }, 'rev-parse', ref); + return data === '' ? undefined : data.trim(); } static async revparse_currentBranch(repoPath: string): Promise<[string, string?] | undefined> { const params = ['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@', '@{u}']; - const opts = { cwd: repoPath } as CommandOptions; + const opts = { + cwd: repoPath, + exceptionHandler: throwExceptionHandler + } as GitCommandOptions; + try { - const data = await gitCommandCore(opts, ...params); + const data = await git(opts, ...params); return [data, undefined]; } catch (ex) { const msg = ex && ex.toString(); if (GitWarnings.headNotABranch.test(msg)) { - try { - const params = ['log', '-n1', '--format=%H', '--']; - const data = await gitCommandCore(opts, ...params); - if (data === undefined) return undefined; - - // Matches output of `git branch -vv` - const sha = data.trim(); - return [`(HEAD detached at ${this.shortenSha(sha)})`, sha]; - } - catch { - return undefined; - } + const data = await git( + { ...opts, exceptionHandler: ignoreExceptionsHandler }, + 'log', + '-n1', + '--format=%H', + '--' + ); + if (data === '') return undefined; + + // Matches output of `git branch -vv` + const sha = data.trim(); + return [`(HEAD detached at ${this.shortenSha(sha)})`, sha]; } const result = GitWarnings.noUpstream.exec(msg); if (result !== null) return [result[1], undefined]; if (GitWarnings.unknownRevision.test(msg)) { - try { - const params = ['symbolic-ref', '-q', '--short', 'HEAD']; - const data = await gitCommandCore(opts, ...params); - return [data, undefined]; - } - catch { - return undefined; - } + const data = await git( + { ...opts, exceptionHandler: ignoreExceptionsHandler }, + 'symbolic-ref', + '-q', + '--short', + 'HEAD' + ); + return data === '' ? undefined : [data.trim(), undefined]; } - gitCommandDefaultErrorHandler(ex, opts, ...params); + defaultExceptionHandler(ex, opts, ...params); return undefined; } } static async revparse_toplevel(cwd: string): Promise { - try { - const data = await gitCommandCore({ cwd: cwd }, 'rev-parse', '--show-toplevel'); - return data.trim(); - } - catch { - return undefined; - } + const data = await git({ cwd: cwd, exceptionHandler: ignoreExceptionsHandler }, 'rev-parse', '--show-toplevel'); + return data === '' ? undefined : data.trim(); } static async show( @@ -672,11 +666,15 @@ export class Git { } if (Git.isUncommitted(ref)) throw new Error(`sha=${ref} is uncommitted`); - const opts = { cwd: root, encoding: options.encoding || 'utf8' } as CommandOptions; + const opts = { + cwd: root, + encoding: options.encoding || 'utf8', + exceptionHandler: throwExceptionHandler + } as GitCommandOptions; const args = ref.endsWith(':') ? `${ref}./${file}` : `${ref}:./${file}`; try { - const data = await gitCommandCore(opts, 'show', args, '--'); + const data = await git(opts, 'show', args, '--'); return data; } catch (ex) { @@ -693,22 +691,22 @@ export class Git { return undefined; } - return gitCommandDefaultErrorHandler(ex, opts, args); + return defaultExceptionHandler(ex, opts, args); } } static stash_apply(repoPath: string, stashName: string, deleteAfter: boolean) { if (!stashName) return undefined; - return gitCommand({ cwd: repoPath }, 'stash', deleteAfter ? 'pop' : 'apply', stashName); + return git({ cwd: repoPath }, 'stash', deleteAfter ? 'pop' : 'apply', stashName); } static stash_delete(repoPath: string, stashName: string) { if (!stashName) return undefined; - return gitCommand({ cwd: repoPath }, 'stash', 'drop', stashName); + return git({ cwd: repoPath }, 'stash', 'drop', stashName); } static stash_list(repoPath: string) { - return gitCommand({ cwd: repoPath }, ...defaultStashParams); + return git({ cwd: repoPath }, ...defaultStashParams); } static stash_push(repoPath: string, pathspecs: string[], message?: string) { @@ -717,7 +715,7 @@ export class Git { params.push('-m', message); } params.splice(params.length, 0, '--', ...pathspecs); - return gitCommand({ cwd: repoPath }, ...params); + return git({ cwd: repoPath }, ...params); } static stash_save(repoPath: string, message?: string) { @@ -725,12 +723,12 @@ export class Git { if (message) { params.push(message); } - return gitCommand({ cwd: repoPath }, ...params); + return git({ cwd: repoPath }, ...params); } static status(repoPath: string, porcelainVersion: number = 1): Promise { const porcelain = porcelainVersion >= 2 ? `--porcelain=v${porcelainVersion}` : '--porcelain'; - return gitCommand( + return git( { cwd: repoPath, env: { ...process.env, GIT_OPTIONAL_LOCKS: '0' } }, '-c', 'color.status=false', @@ -745,7 +743,7 @@ export class Git { const [file, root] = Git.splitPath(fileName, repoPath); const porcelain = porcelainVersion >= 2 ? `--porcelain=v${porcelainVersion}` : '--porcelain'; - return gitCommand( + return git( { cwd: root, env: { ...process.env, GIT_OPTIONAL_LOCKS: '0' } }, '-c', 'color.status=false', @@ -757,8 +755,6 @@ export class Git { } static tag(repoPath: string) { - const params = ['tag', '-l', '-n1']; - - return gitCommand({ cwd: repoPath }, ...params); + return git({ cwd: repoPath }, 'tag', '-l', '-n1'); } } diff --git a/src/git/gitLocator.ts b/src/git/gitLocator.ts index 531a9df..2bcf8c6 100644 --- a/src/git/gitLocator.ts +++ b/src/git/gitLocator.ts @@ -1,7 +1,7 @@ 'use strict'; // import { findActualExecutable, spawnPromise } from 'spawn-rx'; import * as path from 'path'; -import { findExecutable, runCommand } from './shell'; +import { findExecutable, run } from './shell'; export interface IGitInfo { path: string; @@ -13,7 +13,7 @@ function parseVersion(raw: string): string { } async function findSpecificGit(path: string): Promise { - const version = await runCommand(path, ['--version']); + const version = await run(path, ['--version'], 'utf8'); // If needed, let's update our path to avoid the search on every command if (!path || path === 'git') { path = findExecutable(path, ['--version']).cmd; @@ -27,7 +27,7 @@ async function findSpecificGit(path: string): Promise { async function findGitDarwin(): Promise { try { - let path = await runCommand('which', ['git']); + let path = await run('which', ['git'], 'utf8'); path = path.replace(/^\s+|\s+$/g, ''); if (path !== '/usr/bin/git') { @@ -35,7 +35,7 @@ async function findGitDarwin(): Promise { } try { - await runCommand('xcode-select', ['-p']); + await run('xcode-select', ['-p'], 'utf8'); return findSpecificGit(path); } catch (ex) { diff --git a/src/git/shell.ts b/src/git/shell.ts index d66c90a..69756b7 100644 --- a/src/git/shell.ts +++ b/src/git/shell.ts @@ -1,6 +1,7 @@ 'use strict'; import { execFile } from 'child_process'; import * as fs from 'fs'; +import * as iconv from 'iconv-lite'; import * as path from 'path'; import { Logger } from '../logger'; @@ -89,7 +90,18 @@ export function findExecutable(exe: string, args: string[]): { cmd: string; args return { cmd: exe, args: args }; } -export interface CommandOptions { +export class RunError extends Error { + constructor( + public readonly exitCode: number, + ...args: any[] + ) { + super(...args); + + Error.captureStackTrace(this, RunError); + } +} + +export interface RunOptions { readonly cwd?: string; readonly env?: Object; readonly encoding?: BufferEncoding; @@ -114,33 +126,41 @@ export interface CommandOptions { readonly stdinEncoding?: string; } -export function runCommand(command: string, args: any[], options: CommandOptions = {}) { - const { stdin, stdinEncoding, ...opts } = { maxBuffer: 100 * 1024 * 1024, ...options } as CommandOptions; +export function run(command: string, args: any[], encoding: BufferEncoding, options: RunOptions = {}): Promise { + const { stdin, stdinEncoding, ...opts } = { maxBuffer: 100 * 1024 * 1024, ...options } as RunOptions; return new Promise((resolve, reject) => { - const proc = execFile(command, args, opts, (err: Error & { code?: string | number } | null, stdout, stderr) => { - if (!err) { + const proc = execFile( + command, + args, + opts, + (err: (Error & { code?: string | number }) | null, stdout, stderr) => { + if (err != null) { + reject( + new RunError( + err.code ? Number(err.code) : 0, + err.message === 'stdout maxBuffer exceeded' + ? `Command output exceeded the allocated stdout buffer. Set 'options.maxBuffer' to a larger value than ${ + opts.maxBuffer + } bytes` + : stderr + ) + ); + + return; + } + if (stderr) { Logger.warn(`Warning(${command} ${args.join(' ')}): ${stderr}`); } - resolve(stdout); - - return; - } - if (err.message === 'stdout maxBuffer exceeded') { - reject( - new Error( - `Command output exceeded the allocated stdout buffer. Set 'options.maxBuffer' to a larger value than ${ - opts.maxBuffer - } bytes` - ) + resolve( + encoding === 'utf8' || encoding === 'binary' + ? stdout + : iconv.decode(Buffer.from(stdout, 'binary'), encoding) ); } - - // Logger.warn(`Error(${opts.cwd}): ${command} ${args.join(' ')})\n (${err.code}) ${err.message}\n${stderr}`); - reject(err); - }); + ); if (stdin) { proc.stdin.end(stdin, stdinEncoding || 'utf8'); diff --git a/src/gitService.ts b/src/gitService.ts index 4d77e5f..cf795d0 100644 --- a/src/gitService.ts +++ b/src/gitService.ts @@ -18,7 +18,7 @@ import { } from 'vscode'; import { GitExtension } from './@types/git'; import { configuration, IRemotesConfig } from './configuration'; -import { CommandContext, DocumentSchemes, setCommandContext } from './constants'; +import { CommandContext, DocumentSchemes, GlyphChars, setCommandContext } from './constants'; import { Container } from './container'; import { CommitFormatting, @@ -243,10 +243,10 @@ export class GitService implements Disposable { } if (depth <= 0) { - const duration = process.hrtime(start); Logger.log( - `Searching for repositories (depth=${depth}) in '${folderUri.fsPath}' took ${duration[0] * 1000 + - Math.floor(duration[1] / 1000000)} ms` + `Completed repository search (depth=${depth}) in '${folderUri.fsPath}' ${ + GlyphChars.Dot + } ${Strings.getDurationMilliseconds(start)} ms` ); return repositories; @@ -281,13 +281,13 @@ export class GitService implements Disposable { catch (ex) { if (RepoSearchWarnings.doesNotExist.test(ex.message || '')) { Logger.log( - `Searching for repositories (depth=${depth}) in '${folderUri.fsPath}' FAILED${ - ex.message ? ` (${ex.message})` : '' + `Repository search (depth=${depth}) in '${folderUri.fsPath}' FAILED${ + ex.message ? `(${ex.message})` : '' }` ); } else { - Logger.error(ex, `Searching for repositories (depth=${depth}) in '${folderUri.fsPath}' FAILED`); + Logger.error(ex, `Repository search (depth=${depth}) in '${folderUri.fsPath}' FAILED`); } return repositories; @@ -305,10 +305,10 @@ export class GitService implements Disposable { repositories.push(new Repository(folder, rp, false, anyRepoChangedFn, this._suspended)); } - const duration = process.hrtime(start); Logger.log( - `Searching for repositories (depth=${depth}) in '${folderUri.fsPath}' took ${duration[0] * 1000 + - Math.floor(duration[1] / 1000000)} ms` + `Completed repository search (depth=${depth}) in '${folderUri.fsPath}' ${ + GlyphChars.Dot + } ${Strings.getDurationMilliseconds(start)} ms` ); return repositories; diff --git a/src/logger.ts b/src/logger.ts index 58510e1..236b2cf 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -94,15 +94,13 @@ export class Logger { static gitOutput: OutputChannel | undefined; - static logGitCommand(command: string, cwd: string, ex?: Error): void { + static logGitCommand(command: string, ex?: Error): void { if (this.level !== OutputLevel.Debug) return; if (this.gitOutput === undefined) { this.gitOutput = window.createOutputChannel(`${extensionOutputChannelName} (Git)`); } - this.gitOutput.appendLine( - `${this.timestamp} ${command} (${cwd})${ex === undefined ? '' : `\n\n${ex.toString()}`}` - ); + this.gitOutput.appendLine(`${this.timestamp} ${command}${ex === undefined ? '' : `\n\n${ex.toString()}`}`); } private static _isDebugging: boolean | undefined; diff --git a/src/system/string.ts b/src/system/string.ts index bbafc40..9437f68 100644 --- a/src/system/string.ts +++ b/src/system/string.ts @@ -2,6 +2,11 @@ import { createHash, HexBase64Latin1Encoding } from 'crypto'; export namespace Strings { + export function getDurationMilliseconds(start: [number, number]) { + const [secs, nanosecs] = process.hrtime(start); + return secs * 1000 + Math.floor(nanosecs / 1000000); + } + const pathNormalizer = /\\/g; const TokenRegex = /\$\{([^|]*?)(?:\|(\d+)(\-|\?)?)?\}/g; const TokenSanitizeRegex = /\$\{(\w*?)(?:\W|\d)*?\}/g; diff --git a/src/telemetry.ts b/src/telemetry.ts index 8903c54..bf1d5ba 100644 --- a/src/telemetry.ts +++ b/src/telemetry.ts @@ -1,8 +1,9 @@ // 'use strict'; +// import * as os from 'os'; // import { env, version, workspace } from 'vscode'; // import { configuration } from './configuration'; // import { Logger } from './logger'; -// import * as os from 'os'; +// import { Strings } from './system'; // let _reporter: TelemetryReporter | undefined; @@ -18,8 +19,7 @@ // _reporter = new TelemetryReporter(key); -// const duration = process.hrtime(start); -// Logger.log(`Telemetry.configure took ${(duration[0] * 1000) + Math.floor(duration[1] / 1000000)} ms`); +// Logger.log(`Telemetry.configure ${GlyphChars.Dot} ${Strings.getDurationMilliseconds(start)} ms`); // } // static setContext(context?: { [key: string]: string }) {