From bcd9659f9d0cf61684bd62a33c3b40f1c154ff78 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 22 Aug 2022 11:15:47 +1000 Subject: [PATCH 1/7] add git executor class --- extension/src/cli/{ => dvc}/index.test.ts | 31 +++---- extension/src/cli/dvc/index.ts | 33 +++++++ extension/src/cli/executor.ts | 9 +- extension/src/cli/git/constants.ts | 18 ++++ extension/src/cli/git/executor.ts | 75 ++++++++++++++++ extension/src/cli/git/index.ts | 88 +++++++++++++++++++ extension/src/cli/git/reader.ts | 15 ++++ extension/src/cli/index.ts | 37 +++----- extension/src/cli/reader.ts | 15 ++-- extension/src/commands/internal.ts | 4 +- extension/src/extension.ts | 8 +- extension/src/repository/commands/index.ts | 23 +++-- extension/src/repository/commands/register.ts | 4 +- extension/src/test/suite/status.test.ts | 7 +- .../test/suite/vscode/outputChannel.test.ts | 7 +- 15 files changed, 306 insertions(+), 68 deletions(-) rename extension/src/cli/{ => dvc}/index.test.ts (82%) create mode 100644 extension/src/cli/dvc/index.ts create mode 100644 extension/src/cli/git/constants.ts create mode 100644 extension/src/cli/git/executor.ts create mode 100644 extension/src/cli/git/index.ts create mode 100644 extension/src/cli/git/reader.ts diff --git a/extension/src/cli/index.test.ts b/extension/src/cli/dvc/index.test.ts similarity index 82% rename from extension/src/cli/index.test.ts rename to extension/src/cli/dvc/index.test.ts index 9bba577c1c..9446c7ac1e 100644 --- a/extension/src/cli/index.test.ts +++ b/extension/src/cli/dvc/index.test.ts @@ -1,17 +1,18 @@ import { EventEmitter } from 'vscode' import { Disposable, Disposer } from '@hediet/std/disposable' -import { Cli, CliResult, CliStarted, typeCheckCommands } from '.' -import { Command } from './constants' -import { getProcessEnv } from '../env' -import { createProcess } from '../processExecution' -import { getFailingMockedProcess, getMockedProcess } from '../test/util/jest' -import { Config } from '../config' -import { joinEnvPath } from '../util/env' +import { DvcCli } from '.' +import { CliResult, CliStarted, typeCheckCommands } from '..' +import { Command } from '../constants' +import { getProcessEnv } from '../../env' +import { createProcess } from '../../processExecution' +import { getFailingMockedProcess, getMockedProcess } from '../../test/util/jest' +import { Config } from '../../config' +import { joinEnvPath } from '../../util/env' jest.mock('vscode') jest.mock('@hediet/std/disposable') -jest.mock('../env') -jest.mock('../processExecution') +jest.mock('../../env') +jest.mock('../../processExecution') const mockedDisposable = jest.mocked(Disposable) @@ -31,7 +32,7 @@ beforeEach(() => { }) describe('typeCheckCommands', () => { - const cli = { func: jest.fn() } as unknown as Cli + const cli = { func: jest.fn() } as unknown as DvcCli it('should throw an error when the command is not on the class', () => { expect(() => typeCheckCommands( @@ -51,7 +52,7 @@ describe('typeCheckCommands', () => { }) }) -describe('executeProcess', () => { +describe('executeDvcProcess', () => { it('should pass the correct details to the underlying process given no path to the cli or python binary path', async () => { const existingPath = joinEnvPath( '/Users/robot/some/path', @@ -62,7 +63,7 @@ describe('executeProcess', () => { const args = [Command.CHECKOUT] mockedGetEnv.mockReturnValueOnce(processEnv) mockedCreateProcess.mockReturnValueOnce(getMockedProcess('done')) - const cli = new Cli( + const cli = new DvcCli( { getCliPath: () => undefined, pythonBinPath: undefined @@ -79,7 +80,7 @@ describe('executeProcess', () => { } ) - await cli.executeProcess(cwd, ...args) + await cli.executeDvcProcess(cwd, ...args) expect(mockedCreateProcess).toBeCalledWith({ args, @@ -101,7 +102,7 @@ describe('executeProcess', () => { const args = [Command.CHECKOUT] mockedGetEnv.mockReturnValueOnce(processEnv) mockedCreateProcess.mockReturnValueOnce(getFailingMockedProcess('I DEED')) - const cli = new Cli( + const cli = new DvcCli( { getCliPath: () => '/some/path/to/dvc', pythonBinPath @@ -118,7 +119,7 @@ describe('executeProcess', () => { } ) - await expect(cli.executeProcess(cwd, ...args)).rejects.toThrow() + await expect(cli.executeDvcProcess(cwd, ...args)).rejects.toThrow() expect(mockedCreateProcess).toBeCalledWith({ args, diff --git a/extension/src/cli/dvc/index.ts b/extension/src/cli/dvc/index.ts new file mode 100644 index 0000000000..2ab8777165 --- /dev/null +++ b/extension/src/cli/dvc/index.ts @@ -0,0 +1,33 @@ +import { EventEmitter } from 'vscode' +import { Cli, CliResult, CliStarted } from '..' +import { Config } from '../../config' +import { Args } from '../constants' +import { getOptions } from '../options' + +export class DvcCli extends Cli { + public autoRegisteredCommands: string[] = [] + + protected readonly config: Config + + constructor( + config: Config, + emitters?: { + processStarted: EventEmitter + processCompleted: EventEmitter + } + ) { + super(emitters) + + this.config = config + } + + public executeDvcProcess(cwd: string, ...args: Args): Promise { + const { command, ...options } = getOptions( + this.config.pythonBinPath, + this.config.getCliPath(), + cwd, + ...args + ) + return this.executeProcess(cwd, command, options) + } +} diff --git a/extension/src/cli/executor.ts b/extension/src/cli/executor.ts index 0e25abae7b..9c992e206b 100644 --- a/extension/src/cli/executor.ts +++ b/extension/src/cli/executor.ts @@ -1,4 +1,5 @@ -import { Cli, typeCheckCommands } from '.' +import { typeCheckCommands } from '.' +import { DvcCli } from './dvc' import { Args, Command, @@ -27,7 +28,7 @@ export const autoRegisteredCommands = { REMOVE: 'remove' } as const -export class CliExecutor extends Cli { +export class CliExecutor extends DvcCli { public readonly autoRegisteredCommands = typeCheckCommands( autoRegisteredCommands, this @@ -130,12 +131,12 @@ export class CliExecutor extends Cli { } private executeExperimentProcess(cwd: string, ...args: Args) { - return this.executeProcess(cwd, Command.EXPERIMENT, ...args) + return this.executeDvcProcess(cwd, Command.EXPERIMENT, ...args) } private async blockAndExecuteProcess(cwd: string, ...args: Args) { this.setRunning(true) - const output = await this.executeProcess(cwd, ...args) + const output = await this.executeDvcProcess(cwd, ...args) this.setRunning(false) return output } diff --git a/extension/src/cli/git/constants.ts b/extension/src/cli/git/constants.ts new file mode 100644 index 0000000000..a4b7fd98ab --- /dev/null +++ b/extension/src/cli/git/constants.ts @@ -0,0 +1,18 @@ +export enum Command { + ADD = 'add', + CLEAN = 'clean', + DIFF = 'diff', + LS_FILES = 'ls-files', + PUSH = 'push', + RESET = 'reset', + REV_PARSE = 'rev-parse' +} + +export enum Flag { + DIRECTORIES = '-d', + DOT = '.', + FORCE = '-f', + HARD = '--hard', + SHOW_TOPLEVEL = '--show-toplevel', + QUIET = '-q' +} diff --git a/extension/src/cli/git/executor.ts b/extension/src/cli/git/executor.ts new file mode 100644 index 0000000000..bf63a8721f --- /dev/null +++ b/extension/src/cli/git/executor.ts @@ -0,0 +1,75 @@ +import { Command, Flag } from './constants' +import { typeCheckCommands, Cli } from '..' +import { executeProcess } from '../../processExecution' +import { getGitRepositoryRoot } from '../../git' + +export const autoRegisteredCommands = { + GIT_PUSH_BRANCH: 'pushBranch', + GIT_RESET_WORKSPACE: 'resetWorkspace', + GIT_STAGE_ALL: 'stageAll', + GIT_UNSTAGE_ALL: 'reset' +} as const + +export class GitExecutor extends Cli { + public readonly autoRegisteredCommands = typeCheckCommands( + autoRegisteredCommands, + this + ) + + public reset(cwd: string, ...args: string[]) { + const options = { + args: [Command.RESET, ...args], + cwd, + env: process.env, + executable: 'git' + } + + return this.executeProcess( + cwd, + [options.executable, ...options.args].join(' '), + options + ) + } + + public async resetWorkspace(cwd: string) { + await this.reset(cwd, Flag.HARD, 'HEAD') + + return executeProcess({ + args: [Command.CLEAN, Flag.FORCE, Flag.DIRECTORIES, Flag.QUIET], + cwd, + executable: 'git' + }) + } + + public async stageAll(cwd: string) { + const gitRoot = await getGitRepositoryRoot(cwd) + + const options = { + args: [Command.ADD, Flag.DOT], + cwd: gitRoot, + env: process.env, + executable: 'git' + } + + return this.executeProcess( + cwd, + [options.executable, ...options.args].join(' '), + options + ) + } + + public pushBranch(cwd: string, branchName: string) { + const options = { + args: ['push', '-u', 'origin', branchName], + cwd, + env: process.env, + executable: 'git' + } + + return this.executeProcess( + cwd, + [options.executable, ...options.args].join(' '), + options + ) + } +} diff --git a/extension/src/cli/git/index.ts b/extension/src/cli/git/index.ts new file mode 100644 index 0000000000..acf2bf68b7 --- /dev/null +++ b/extension/src/cli/git/index.ts @@ -0,0 +1,88 @@ +import { Event, EventEmitter } from 'vscode' +import { CliEvent, CliResult, CliStarted, ICli } from '..' +import { Disposable } from '../../class/dispose' +import { createProcess } from '../../processExecution' +import { StopWatch } from '../../util/time' +import { CliError, MaybeConsoleError } from '../error' +import { ExecutionOptions } from '../options' + +export class Cli extends Disposable implements ICli { + public autoRegisteredCommands: string[] = [] + + public readonly processCompleted: EventEmitter + public readonly onDidCompleteProcess: Event + + public readonly processStarted: EventEmitter + public readonly onDidStartProcess: Event + + constructor(emitters?: { + processStarted: EventEmitter + processCompleted: EventEmitter + }) { + super() + + this.processCompleted = + emitters?.processCompleted || + this.dispose.track(new EventEmitter()) + this.onDidCompleteProcess = this.processCompleted.event + + this.processStarted = + emitters?.processStarted || + this.dispose.track(new EventEmitter()) + this.onDidStartProcess = this.processStarted.event + } + + public async executeProcess( + cwd: string, + command: string, + options: ExecutionOptions + ): Promise { + const baseEvent: CliEvent = { command, cwd, pid: undefined } + const stopWatch = new StopWatch() + try { + const process = this.dispose.track(createProcess(options)) + + baseEvent.pid = process.pid + this.processStarted.fire(baseEvent) + + process.on('close', () => { + this.dispose.untrack(process) + }) + + const { stdout, exitCode } = await process + + this.processCompleted.fire({ + ...baseEvent, + duration: stopWatch.getElapsedTime(), + exitCode + }) + return stdout + } catch (error: unknown) { + throw this.processCliError( + error as MaybeConsoleError, + options, + baseEvent, + stopWatch.getElapsedTime() + ) + } + } + + private processCliError( + error: MaybeConsoleError, + options: ExecutionOptions, + baseEvent: CliEvent, + duration: number + ) { + const cliError = new CliError({ + baseError: error as MaybeConsoleError, + options + }) + this.processCompleted.fire({ + ...baseEvent, + duration, + exitCode: cliError.exitCode, + stderr: cliError.stderr + }) + return cliError + } +} diff --git a/extension/src/cli/git/reader.ts b/extension/src/cli/git/reader.ts new file mode 100644 index 0000000000..dcd23976b8 --- /dev/null +++ b/extension/src/cli/git/reader.ts @@ -0,0 +1,15 @@ +import { Cli, typeCheckCommands } from '..' + +export const autoRegisteredCommands = { + GIT_PUSH: 'push', + GIT_RESET: 'reset', + GIT_RESET_WORKSPACE: 'resetWorkspace', + GIT_STAGE_ALL: 'stageAll' +} as const + +export class GitReader extends Cli { + public readonly autoRegisteredCommands = typeCheckCommands( + autoRegisteredCommands, + this + ) +} diff --git a/extension/src/cli/index.ts b/extension/src/cli/index.ts index 8249455b69..5f6e59733f 100644 --- a/extension/src/cli/index.ts +++ b/extension/src/cli/index.ts @@ -1,13 +1,11 @@ import { Event, EventEmitter } from 'vscode' -import { Args } from './constants' -import { ExecutionOptions, getOptions } from './options' +import { ExecutionOptions } from './options' import { CliError, MaybeConsoleError } from './error' import { createProcess } from '../processExecution' -import { Config } from '../config' import { StopWatch } from '../util/time' import { Disposable } from '../class/dispose' -type CliEvent = { +export type CliEvent = { command: string cwd: string pid: number | undefined @@ -54,19 +52,12 @@ export class Cli extends Disposable implements ICli { public readonly processStarted: EventEmitter public readonly onDidStartProcess: Event - protected readonly config: Config - - constructor( - config: Config, - emitters?: { - processStarted: EventEmitter - processCompleted: EventEmitter - } - ) { + constructor(emitters?: { + processStarted: EventEmitter + processCompleted: EventEmitter + }) { super() - this.config = config - this.processCompleted = emitters?.processCompleted || this.dispose.track(new EventEmitter()) @@ -78,8 +69,11 @@ export class Cli extends Disposable implements ICli { this.onDidStartProcess = this.processStarted.event } - public async executeProcess(cwd: string, ...args: Args): Promise { - const { command, ...options } = this.getOptions(cwd, ...args) + protected async executeProcess( + cwd: string, + command: string, + options: ExecutionOptions + ): Promise { const baseEvent: CliEvent = { command, cwd, pid: undefined } const stopWatch = new StopWatch() try { @@ -110,15 +104,6 @@ export class Cli extends Disposable implements ICli { } } - private getOptions(cwd: string, ...args: Args) { - return getOptions( - this.config.pythonBinPath, - this.config.getCliPath(), - cwd, - ...args - ) - } - private processCliError( error: MaybeConsoleError, options: ExecutionOptions, diff --git a/extension/src/cli/reader.ts b/extension/src/cli/reader.ts index 87fd6be267..5a619fdd8b 100644 --- a/extension/src/cli/reader.ts +++ b/extension/src/cli/reader.ts @@ -1,5 +1,6 @@ import { join } from 'path' -import { Cli, typeCheckCommands } from '.' +import { typeCheckCommands } from '.' +import { DvcCli } from './dvc' import { Args, Command, @@ -126,7 +127,7 @@ export const autoRegisteredCommands = { STATUS: 'status' } as const -export class CliReader extends Cli { +export class CliReader extends DvcCli { public readonly autoRegisteredCommands = typeCheckCommands( autoRegisteredCommands, this @@ -175,7 +176,7 @@ export class CliReader extends Cli { public async root(cwd: string): Promise { try { - return await this.executeProcess(cwd, Command.ROOT) + return await this.executeDvcProcess(cwd, Command.ROOT) } catch {} } @@ -184,7 +185,7 @@ export class CliReader extends Cli { } public version(cwd: string): Promise { - return this.executeProcess(cwd, Flag.VERSION) + return this.executeDvcProcess(cwd, Flag.VERSION) } private async readProcess( @@ -194,8 +195,10 @@ export class CliReader extends Cli { ...args: Args ): Promise { const output = - (await retry(() => this.executeProcess(cwd, ...args), args.join(' '))) || - defaultValue + (await retry( + () => this.executeDvcProcess(cwd, ...args), + args.join(' ') + )) || defaultValue if (!formatter) { return output as unknown as T } diff --git a/extension/src/commands/internal.ts b/extension/src/commands/internal.ts index fc0ca48749..e6fc4a4a8d 100644 --- a/extension/src/commands/internal.ts +++ b/extension/src/commands/internal.ts @@ -5,6 +5,7 @@ import { Args } from '../cli/constants' import { autoRegisteredCommands as CliExecutorCommands } from '../cli/executor' import { autoRegisteredCommands as CliReaderCommands } from '../cli/reader' import { autoRegisteredCommands as CliRunnerCommands } from '../cli/runner' +import { autoRegisteredCommands as GitExecutorCommands } from '../cli/git/executor' import { sendTelemetryEvent, sendTelemetryEventAndThrow } from '../telemetry' import { StopWatch } from '../util/time' import { OutputChannel } from '../vscode/outputChannel' @@ -18,7 +19,8 @@ export const AvailableCommands = Object.assign( {} as const, CliExecutorCommands, CliReaderCommands, - CliRunnerCommands + CliRunnerCommands, + GitExecutorCommands ) export type CommandId = typeof AvailableCommands[keyof typeof AvailableCommands] diff --git a/extension/src/extension.ts b/extension/src/extension.ts index 9b73fe9d29..02f21ec390 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -49,6 +49,7 @@ import { PlotsPathsTree } from './plots/paths/tree' import { Disposable } from './class/dispose' import { collectWorkspaceScale } from './telemetry/collect' import { createFileSystemWatcher } from './fileSystem/watcher' +import { GitExecutor } from './cli/git/executor' export class Extension extends Disposable implements IExtension { protected readonly internalCommands: InternalCommands @@ -63,6 +64,7 @@ export class Extension extends Disposable implements IExtension { private readonly cliExecutor: CliExecutor private readonly cliReader: CliReader private readonly cliRunner: CliRunner + private readonly gitExecutor: GitExecutor private readonly status: Status private cliAccessible = false private cliCompatible: boolean | undefined @@ -97,10 +99,11 @@ export class Extension extends Disposable implements IExtension { this.cliExecutor = this.dispose.track(new CliExecutor(this.config)) this.cliReader = this.dispose.track(new CliReader(this.config)) this.cliRunner = this.dispose.track(new CliRunner(this.config)) + this.gitExecutor = this.dispose.track(new GitExecutor()) const outputChannel = this.dispose.track( new OutputChannel( - [this.cliExecutor, this.cliReader, this.cliRunner], + [this.cliExecutor, this.cliReader, this.cliRunner, this.gitExecutor], context.extension.packageJSON.version ) ) @@ -110,7 +113,8 @@ export class Extension extends Disposable implements IExtension { outputChannel, this.cliExecutor, this.cliReader, - this.cliRunner + this.cliRunner, + this.gitExecutor ) ) diff --git a/extension/src/repository/commands/index.ts b/extension/src/repository/commands/index.ts index 8cdd6d54a8..b74e025699 100644 --- a/extension/src/repository/commands/index.ts +++ b/extension/src/repository/commands/index.ts @@ -7,7 +7,6 @@ import { InternalCommands } from '../../commands/internal' import { relativeWithUri } from '../../fileSystem' -import { gitReset, gitResetWorkspace, gitStageAll } from '../../git' import { warnOfConsequences } from '../../vscode/modal' import { Response } from '../../vscode/response' import { WorkspaceRepositories } from '../workspace' @@ -71,7 +70,10 @@ export const getRootCommand = } export const getStageAllCommand = - (repositories: WorkspaceRepositories): RootCommand => + ( + repositories: WorkspaceRepositories, + internalCommands: InternalCommands + ): RootCommand => async context => { const cwd = await repositories.getCwd(context?.rootUri) @@ -79,12 +81,15 @@ export const getStageAllCommand = return } - await gitStageAll(cwd) + await internalCommands.executeCommand(AvailableCommands.GIT_STAGE_ALL, cwd) return commands.executeCommand('workbench.scm.focus') } export const getUnstageAllCommand = - (repositories: WorkspaceRepositories): RootCommand => + ( + repositories: WorkspaceRepositories, + internalCommands: InternalCommands + ): RootCommand => async context => { const cwd = await repositories.getCwd(context?.rootUri) @@ -92,7 +97,10 @@ export const getUnstageAllCommand = return } - return gitReset(cwd) + return internalCommands.executeCommand( + AvailableCommands.GIT_UNSTAGE_ALL, + cwd + ) } export const getCommitRootCommand = @@ -138,7 +146,10 @@ export const getResetRootCommand = return } - await gitResetWorkspace(cwd as string) + await internalCommands.executeCommand( + AvailableCommands.GIT_RESET_WORKSPACE, + cwd as string + ) return internalCommands.executeCommand( AvailableCommands.CHECKOUT, diff --git a/extension/src/repository/commands/register.ts b/extension/src/repository/commands/register.ts index f84d1b21da..bbae7a0fee 100644 --- a/extension/src/repository/commands/register.ts +++ b/extension/src/repository/commands/register.ts @@ -39,12 +39,12 @@ const registerResourceGroupCommands = ( ) => { internalCommands.registerExternalCommand( RegisteredCommands.GIT_STAGE_ALL, - getStageAllCommand(repositories) + getStageAllCommand(repositories, internalCommands) ) internalCommands.registerExternalCommand( RegisteredCommands.GIT_UNSTAGE_ALL, - getUnstageAllCommand(repositories) + getUnstageAllCommand(repositories, internalCommands) ) } diff --git a/extension/src/test/suite/status.test.ts b/extension/src/test/suite/status.test.ts index 4b2eb56182..99dfea6b5c 100644 --- a/extension/src/test/suite/status.test.ts +++ b/extension/src/test/suite/status.test.ts @@ -5,7 +5,8 @@ import { window, workspace, EventEmitter, StatusBarItem } from 'vscode' import { closeAllEditors } from './util' import { Disposable } from '../../extension' import { Status } from '../../status' -import { Cli, CliResult, CliStarted } from '../../cli' +import { CliResult, CliStarted } from '../../cli' +import { DvcCli } from '../../cli/dvc' import { Config } from '../../config' import { RegisteredCommands } from '../../commands/external' import { Title } from '../../vscode/title' @@ -36,7 +37,7 @@ suite('Status Test Suite', () => { const processStarted = disposable.track(new EventEmitter()) const cli = disposable.track( - new Cli({} as Config, { processCompleted, processStarted }) + new DvcCli({} as Config, { processCompleted, processStarted }) ) const mockStatusBarItem = { command: undefined, @@ -117,7 +118,7 @@ suite('Status Test Suite', () => { const cwd = __dirname const cli = disposable.track( - new Cli({} as Config, { processCompleted, processStarted }) + new DvcCli({} as Config, { processCompleted, processStarted }) ) const mockStatusBarItem = { dispose: fake(), diff --git a/extension/src/test/suite/vscode/outputChannel.test.ts b/extension/src/test/suite/vscode/outputChannel.test.ts index eee757b167..5a298dfdf5 100644 --- a/extension/src/test/suite/vscode/outputChannel.test.ts +++ b/extension/src/test/suite/vscode/outputChannel.test.ts @@ -4,7 +4,8 @@ import { EventEmitter, window, OutputChannel as VSOutputChannel } from 'vscode' import { restore, stub, fake } from 'sinon' import { OutputChannel } from '../../../vscode/outputChannel' import { Disposable } from '../../../extension' -import { Cli, CliResult, CliStarted } from '../../../cli' +import { CliResult, CliStarted } from '../../../cli' +import { DvcCli } from '../../../cli/dvc' import { Config } from '../../../config' suite('Output Channel Test Suite', () => { @@ -27,7 +28,7 @@ suite('Output Channel Test Suite', () => { const processStarted = disposable.track(new EventEmitter()) const cli = disposable.track( - new Cli({} as Config, { processCompleted, processStarted }) + new DvcCli({} as Config, { processCompleted, processStarted }) ) const mockAppend = fake() const mockOutputChannel = stub(window, 'createOutputChannel').returns({ @@ -65,7 +66,7 @@ suite('Output Channel Test Suite', () => { const processStarted = disposable.track(new EventEmitter()) const cli = disposable.track( - new Cli({} as Config, { processCompleted, processStarted }) + new DvcCli({} as Config, { processCompleted, processStarted }) ) const mockAppend = fake() const mockOutputChannel = stub(window, 'createOutputChannel').returns({ From e08cf2c016313572483566e6436043f5982cb917 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 22 Aug 2022 12:58:43 +1000 Subject: [PATCH 2/7] add git reader class --- extension/src/cli/command.test.ts | 32 ------- extension/src/cli/command.ts | 17 ++-- extension/src/cli/dvc/index.ts | 4 +- extension/src/cli/error.ts | 6 +- extension/src/cli/git/constants.ts | 17 +++- extension/src/cli/git/executor.ts | 64 +++++-------- extension/src/cli/git/index.ts | 88 ------------------ extension/src/cli/git/options.ts | 11 +++ extension/src/cli/git/reader.ts | 63 ++++++++++++- extension/src/cli/index.ts | 15 ++- extension/src/cli/options.test.ts | 3 - extension/src/cli/options.ts | 11 +-- extension/src/cli/runner.ts | 4 +- extension/src/commands/internal.ts | 4 +- extension/src/experiments/commands/index.ts | 4 +- extension/src/extension.ts | 25 ++--- extension/src/git.ts | 93 +------------------ extension/src/repository/data/index.ts | 17 ++-- .../src/test/suite/experiments/index.test.ts | 4 +- .../test/suite/experiments/workspace.test.ts | 4 +- extension/src/test/suite/extension.test.ts | 6 ++ .../sourceControlManagement.test.ts | 52 ++++++----- extension/src/test/suite/repository/util.ts | 8 +- extension/src/test/suite/util.ts | 15 ++- 24 files changed, 215 insertions(+), 352 deletions(-) delete mode 100644 extension/src/cli/command.test.ts delete mode 100644 extension/src/cli/git/index.ts create mode 100644 extension/src/cli/git/options.ts diff --git a/extension/src/cli/command.test.ts b/extension/src/cli/command.test.ts deleted file mode 100644 index dccc75552b..0000000000 --- a/extension/src/cli/command.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { join } from 'path' -import { getCommandString } from './command' -import { Command, Flag } from './constants' - -describe('getCommandString', () => { - it('should give the correct command string given a basic environment', () => { - const commandString = getCommandString( - undefined, - 'dvc', - Command.CHECKOUT, - Flag.FORCE - ) - expect(commandString).toStrictEqual('dvc checkout -f') - }) - - it('should give the correct command string given an isolated python env is in use', () => { - const pythonBinPath = join('path', 'to', 'python', '.venv') - const commandString = getCommandString(pythonBinPath, 'dvc', Command.DIFF) - expect(commandString).toStrictEqual( - `${join(pythonBinPath, 'python')} dvc diff` - ) - }) - - it('should give the correct command string given both an isolated python env and direct path to dvc are in use', () => { - const pythonBinPath = join('path', 'to', 'conda', '.venv') - const cliPath = join('custom', 'path', 'to', 'dvc') - const commandString = getCommandString(pythonBinPath, cliPath, Command.PUSH) - expect(commandString).toStrictEqual( - `${join(pythonBinPath, 'python')} ${cliPath} push` - ) - }) -}) diff --git a/extension/src/cli/command.ts b/extension/src/cli/command.ts index f38c130001..a8c243164f 100644 --- a/extension/src/cli/command.ts +++ b/extension/src/cli/command.ts @@ -1,12 +1,11 @@ -import { join } from 'path' -import { Args } from './constants' import { joinTruthyItems } from '../util/array' -export const getCommandString = ( - pythonBinPath: string | undefined, - executable: string, - ...args: Args -): string => { - const prefix = pythonBinPath ? join(pythonBinPath, 'python') : undefined - return `${joinTruthyItems([prefix, executable])} ${args.join(' ')}` +export const getCommandString = ({ + args, + executable +}: { + args: string[] + executable: string +}): string => { + return `${joinTruthyItems([executable, ...args])}` } diff --git a/extension/src/cli/dvc/index.ts b/extension/src/cli/dvc/index.ts index 2ab8777165..b39ebe4597 100644 --- a/extension/src/cli/dvc/index.ts +++ b/extension/src/cli/dvc/index.ts @@ -22,12 +22,12 @@ export class DvcCli extends Cli { } public executeDvcProcess(cwd: string, ...args: Args): Promise { - const { command, ...options } = getOptions( + const options = getOptions( this.config.pythonBinPath, this.config.getCliPath(), cwd, ...args ) - return this.executeProcess(cwd, command, options) + return this.executeProcess(options) } } diff --git a/extension/src/cli/error.ts b/extension/src/cli/error.ts index ce04c0680f..9e52e65e43 100644 --- a/extension/src/cli/error.ts +++ b/extension/src/cli/error.ts @@ -1,4 +1,4 @@ -import { ExecutionOptions } from './options' +import { ProcessOptions } from '../processExecution' export interface MaybeConsoleError extends Error { stderr?: string @@ -6,13 +6,13 @@ export interface MaybeConsoleError extends Error { } interface CliProcessErrorArgs { - options: ExecutionOptions + options: ProcessOptions baseError: MaybeConsoleError message?: string } export class CliError extends Error { - public readonly options?: ExecutionOptions + public readonly options?: ProcessOptions public readonly baseError: Error public readonly stderr?: string public readonly exitCode: number | null diff --git a/extension/src/cli/git/constants.ts b/extension/src/cli/git/constants.ts index a4b7fd98ab..50b53f6650 100644 --- a/extension/src/cli/git/constants.ts +++ b/extension/src/cli/git/constants.ts @@ -10,9 +10,22 @@ export enum Command { export enum Flag { DIRECTORIES = '-d', + DIRECTORY = '--directory', DOT = '.', + EXCLUDE_STANDARD = '--exclude-standard', FORCE = '-f', HARD = '--hard', - SHOW_TOPLEVEL = '--show-toplevel', - QUIET = '-q' + NAME_ONLY = '--name-only', + NO_EMPTY_DIRECTORY = '--no-empty-directory', + OTHERS = '--others', + QUIET = '-q', + RAW_WITH_NUL = '-z', + SET_UPSTREAM = '--set-upstream', + SHOW_TOPLEVEL = '--show-toplevel' } + +export enum Commit { + HEAD = 'HEAD' +} + +export const DEFAULT_REMOTE = 'origin' diff --git a/extension/src/cli/git/executor.ts b/extension/src/cli/git/executor.ts index bf63a8721f..9b25c59124 100644 --- a/extension/src/cli/git/executor.ts +++ b/extension/src/cli/git/executor.ts @@ -1,6 +1,6 @@ -import { Command, Flag } from './constants' +import { Command, Commit, DEFAULT_REMOTE, Flag } from './constants' +import { getOptions } from './options' import { typeCheckCommands, Cli } from '..' -import { executeProcess } from '../../processExecution' import { getGitRepositoryRoot } from '../../git' export const autoRegisteredCommands = { @@ -16,60 +16,42 @@ export class GitExecutor extends Cli { this ) - public reset(cwd: string, ...args: string[]) { - const options = { - args: [Command.RESET, ...args], - cwd, - env: process.env, - executable: 'git' - } + public reset(cwd: string, ...args: (Flag | Commit)[]) { + const options = getOptions(cwd, Command.RESET, ...args) - return this.executeProcess( - cwd, - [options.executable, ...options.args].join(' '), - options - ) + return this.executeProcess(options) } public async resetWorkspace(cwd: string) { - await this.reset(cwd, Flag.HARD, 'HEAD') + await this.reset(cwd, Flag.HARD, Commit.HEAD) - return executeProcess({ - args: [Command.CLEAN, Flag.FORCE, Flag.DIRECTORIES, Flag.QUIET], + const options = getOptions( cwd, - executable: 'git' - }) + Command.CLEAN, + Flag.FORCE, + Flag.DIRECTORIES, + Flag.QUIET + ) + + return this.executeProcess(options) } public async stageAll(cwd: string) { const gitRoot = await getGitRepositoryRoot(cwd) + const options = getOptions(gitRoot, Command.ADD, Flag.DOT) - const options = { - args: [Command.ADD, Flag.DOT], - cwd: gitRoot, - env: process.env, - executable: 'git' - } - - return this.executeProcess( - cwd, - [options.executable, ...options.args].join(' '), - options - ) + return this.executeProcess(options) } public pushBranch(cwd: string, branchName: string) { - const options = { - args: ['push', '-u', 'origin', branchName], + const options = getOptions( cwd, - env: process.env, - executable: 'git' - } - - return this.executeProcess( - cwd, - [options.executable, ...options.args].join(' '), - options + Command.PUSH, + Flag.SET_UPSTREAM, + DEFAULT_REMOTE, + branchName as Commit ) + + return this.executeProcess(options) } } diff --git a/extension/src/cli/git/index.ts b/extension/src/cli/git/index.ts deleted file mode 100644 index acf2bf68b7..0000000000 --- a/extension/src/cli/git/index.ts +++ /dev/null @@ -1,88 +0,0 @@ -import { Event, EventEmitter } from 'vscode' -import { CliEvent, CliResult, CliStarted, ICli } from '..' -import { Disposable } from '../../class/dispose' -import { createProcess } from '../../processExecution' -import { StopWatch } from '../../util/time' -import { CliError, MaybeConsoleError } from '../error' -import { ExecutionOptions } from '../options' - -export class Cli extends Disposable implements ICli { - public autoRegisteredCommands: string[] = [] - - public readonly processCompleted: EventEmitter - public readonly onDidCompleteProcess: Event - - public readonly processStarted: EventEmitter - public readonly onDidStartProcess: Event - - constructor(emitters?: { - processStarted: EventEmitter - processCompleted: EventEmitter - }) { - super() - - this.processCompleted = - emitters?.processCompleted || - this.dispose.track(new EventEmitter()) - this.onDidCompleteProcess = this.processCompleted.event - - this.processStarted = - emitters?.processStarted || - this.dispose.track(new EventEmitter()) - this.onDidStartProcess = this.processStarted.event - } - - public async executeProcess( - cwd: string, - command: string, - options: ExecutionOptions - ): Promise { - const baseEvent: CliEvent = { command, cwd, pid: undefined } - const stopWatch = new StopWatch() - try { - const process = this.dispose.track(createProcess(options)) - - baseEvent.pid = process.pid - this.processStarted.fire(baseEvent) - - process.on('close', () => { - this.dispose.untrack(process) - }) - - const { stdout, exitCode } = await process - - this.processCompleted.fire({ - ...baseEvent, - duration: stopWatch.getElapsedTime(), - exitCode - }) - return stdout - } catch (error: unknown) { - throw this.processCliError( - error as MaybeConsoleError, - options, - baseEvent, - stopWatch.getElapsedTime() - ) - } - } - - private processCliError( - error: MaybeConsoleError, - options: ExecutionOptions, - baseEvent: CliEvent, - duration: number - ) { - const cliError = new CliError({ - baseError: error as MaybeConsoleError, - options - }) - this.processCompleted.fire({ - ...baseEvent, - duration, - exitCode: cliError.exitCode, - stderr: cliError.stderr - }) - return cliError - } -} diff --git a/extension/src/cli/git/options.ts b/extension/src/cli/git/options.ts new file mode 100644 index 0000000000..20782297a8 --- /dev/null +++ b/extension/src/cli/git/options.ts @@ -0,0 +1,11 @@ +import { Command, Commit, DEFAULT_REMOTE, Flag } from './constants' +import { ProcessOptions } from '../../processExecution' + +export const getOptions = ( + cwd: string, + ...args: (Command | Flag | Commit | typeof DEFAULT_REMOTE)[] +): ProcessOptions => ({ + args, + cwd, + executable: 'git' +}) diff --git a/extension/src/cli/git/reader.ts b/extension/src/cli/git/reader.ts index dcd23976b8..14d623e8db 100644 --- a/extension/src/cli/git/reader.ts +++ b/extension/src/cli/git/reader.ts @@ -1,10 +1,13 @@ +import { resolve } from 'path' +import { Command, Flag } from './constants' +import { getOptions } from './options' import { Cli, typeCheckCommands } from '..' +import { trimAndSplit } from '../../util/stdout' +import { isDirectory } from '../../fileSystem' export const autoRegisteredCommands = { - GIT_PUSH: 'push', - GIT_RESET: 'reset', - GIT_RESET_WORKSPACE: 'resetWorkspace', - GIT_STAGE_ALL: 'stageAll' + GIT_HAS_CHANGES: 'hasChanges', + GIT_LIST_UNTRACKED: 'listUntracked' } as const export class GitReader extends Cli { @@ -12,4 +15,56 @@ export class GitReader extends Cli { autoRegisteredCommands, this ) + + public async hasChanges(cwd: string) { + const options = getOptions( + cwd, + Command.DIFF, + Flag.NAME_ONLY, + Flag.RAW_WITH_NUL + ) + const output = await this.executeProcess(options) + + return !!output + } + + public async listUntracked(cwd: string) { + const [files, dirs] = await Promise.all([ + this.getUntrackedFiles(cwd), + this.getUntrackedDirectories(cwd) + ]) + return new Set([...files, ...dirs]) + } + + private getUntrackedDirectories = async (cwd: string): Promise => { + const options = getOptions( + cwd, + Command.LS_FILES, + Flag.OTHERS, + Flag.EXCLUDE_STANDARD, + Flag.DIRECTORY, + Flag.NO_EMPTY_DIRECTORY + ) + + const output = await this.executeProcess(options) + return this.getUris(cwd, trimAndSplit(output)).filter(path => + isDirectory(path) + ) + } + + private getUntrackedFiles = async (cwd: string): Promise => { + const options = getOptions( + cwd, + Command.LS_FILES, + Flag.OTHERS, + Flag.EXCLUDE_STANDARD + ) + + const output = await this.executeProcess(options) + return this.getUris(cwd, trimAndSplit(output)) + } + + private getUris(repositoryRoot: string, relativePaths: string[]) { + return relativePaths.map(path => resolve(repositoryRoot, path)) + } } diff --git a/extension/src/cli/index.ts b/extension/src/cli/index.ts index 5f6e59733f..9d77b98c64 100644 --- a/extension/src/cli/index.ts +++ b/extension/src/cli/index.ts @@ -1,7 +1,7 @@ import { Event, EventEmitter } from 'vscode' -import { ExecutionOptions } from './options' import { CliError, MaybeConsoleError } from './error' -import { createProcess } from '../processExecution' +import { getCommandString } from './command' +import { createProcess, ProcessOptions } from '../processExecution' import { StopWatch } from '../util/time' import { Disposable } from '../class/dispose' @@ -69,12 +69,9 @@ export class Cli extends Disposable implements ICli { this.onDidStartProcess = this.processStarted.event } - protected async executeProcess( - cwd: string, - command: string, - options: ExecutionOptions - ): Promise { - const baseEvent: CliEvent = { command, cwd, pid: undefined } + protected async executeProcess(options: ProcessOptions): Promise { + const command = getCommandString(options) + const baseEvent: CliEvent = { command, cwd: options.cwd, pid: undefined } const stopWatch = new StopWatch() try { const process = this.dispose.track(createProcess(options)) @@ -106,7 +103,7 @@ export class Cli extends Disposable implements ICli { private processCliError( error: MaybeConsoleError, - options: ExecutionOptions, + options: ProcessOptions, baseEvent: CliEvent, duration: number ) { diff --git a/extension/src/cli/options.test.ts b/extension/src/cli/options.test.ts index 3eafc2aeed..755085429b 100644 --- a/extension/src/cli/options.test.ts +++ b/extension/src/cli/options.test.ts @@ -27,7 +27,6 @@ describe('getOptions', () => { const options = getOptions(undefined, '', cwd, Command.CHECKOUT, Flag.FORCE) expect(options).toStrictEqual({ args: ['checkout', '-f'], - command: 'dvc checkout -f', cwd, env: mockedEnv, executable: 'dvc' @@ -39,7 +38,6 @@ describe('getOptions', () => { const options = getOptions(pythonBinPath, '', cwd, Command.DIFF) expect(options).toStrictEqual({ args: ['-m', 'dvc', 'diff'], - command: `${pythonBinPath} -m dvc diff`, cwd, env: { DVCLIVE_OPEN: 'false', @@ -56,7 +54,6 @@ describe('getOptions', () => { const options = getOptions(pythonBinPath, cliPath, cwd, Command.DIFF) expect(options).toStrictEqual({ args: ['diff'], - command: `${cliPath} diff`, cwd, env: { DVCLIVE_OPEN: 'false', diff --git a/extension/src/cli/options.ts b/extension/src/cli/options.ts index e76deabbc1..5a7c0440ff 100644 --- a/extension/src/cli/options.ts +++ b/extension/src/cli/options.ts @@ -3,16 +3,12 @@ import { Args } from './constants' import { getCaseSensitiveCwd } from './cwd' import { getProcessEnv } from '../env' import { joinEnvPath } from '../util/env' +import { ProcessOptions } from '../processExecution' -export type ExecutionOptions = { - executable: string - args: Args - cwd: string +export type ExecutionOptions = ProcessOptions & { env: NodeJS.ProcessEnv } -export type ExecutionDetails = ExecutionOptions & { command: string } - const getPATH = (existingPath: string, pythonBinPath?: string): string => { const python = pythonBinPath ? dirname(pythonBinPath) : '' return joinEnvPath(python, existingPath) @@ -43,12 +39,11 @@ export const getOptions = ( cliPath: string, cwd: string, ...originalArgs: Args -): ExecutionDetails => { +): ExecutionOptions => { const executable = getExecutable(pythonBinPath, cliPath) const args = getArgs(pythonBinPath, cliPath, ...originalArgs) return { args, - command: [executable, ...args].join(' '), cwd: getCaseSensitiveCwd(cwd), env: getEnv(pythonBinPath), executable diff --git a/extension/src/cli/runner.ts b/extension/src/cli/runner.ts index 7220657c7b..783601823d 100644 --- a/extension/src/cli/runner.ts +++ b/extension/src/cli/runner.ts @@ -7,6 +7,7 @@ import { ExperimentSubCommand } from './constants' import { getOptions } from './options' +import { getCommandString } from './command' import { Config } from '../config' import { PseudoTerminal } from '../vscode/pseudoTerminal' import { createProcess, Process } from '../processExecution' @@ -166,7 +167,8 @@ export class CliRunner extends Disposable implements ICli { } private createProcess({ cwd, args }: { cwd: string; args: Args }): Process { - const { command, ...options } = this.getOptions(cwd, args) + const options = this.getOptions(cwd, args) + const command = getCommandString(options) const stopWatch = new StopWatch() const process = this.dispose.track(createProcess(options)) const baseEvent = { command, cwd, pid: process.pid } diff --git a/extension/src/commands/internal.ts b/extension/src/commands/internal.ts index e6fc4a4a8d..c2ef707635 100644 --- a/extension/src/commands/internal.ts +++ b/extension/src/commands/internal.ts @@ -6,6 +6,7 @@ import { autoRegisteredCommands as CliExecutorCommands } from '../cli/executor' import { autoRegisteredCommands as CliReaderCommands } from '../cli/reader' import { autoRegisteredCommands as CliRunnerCommands } from '../cli/runner' import { autoRegisteredCommands as GitExecutorCommands } from '../cli/git/executor' +import { autoRegisteredCommands as GitReaderCommands } from '../cli/git/reader' import { sendTelemetryEvent, sendTelemetryEventAndThrow } from '../telemetry' import { StopWatch } from '../util/time' import { OutputChannel } from '../vscode/outputChannel' @@ -20,7 +21,8 @@ export const AvailableCommands = Object.assign( CliExecutorCommands, CliReaderCommands, CliRunnerCommands, - GitExecutorCommands + GitExecutorCommands, + GitReaderCommands ) export type CommandId = typeof AvailableCommands[keyof typeof AvailableCommands] diff --git a/extension/src/experiments/commands/index.ts b/extension/src/experiments/commands/index.ts index cd4b60bf1a..367be050fb 100644 --- a/extension/src/experiments/commands/index.ts +++ b/extension/src/experiments/commands/index.ts @@ -1,6 +1,4 @@ import { AvailableCommands } from '../../commands/internal' -import { gitPushBranch } from '../../git' -import { Toast } from '../../vscode/toast' import { WorkspaceExperiments } from '../workspace' export const getBranchExperimentCommand = @@ -23,5 +21,5 @@ export const getShareExperimentAsBranchCommand = await experiments.runCommand(AvailableCommands.PUSH, cwd) - return Toast.showOutput(gitPushBranch(cwd, input)) + return experiments.runCommand(AvailableCommands.GIT_PUSH_BRANCH, cwd, input) } diff --git a/extension/src/extension.ts b/extension/src/extension.ts index 02f21ec390..2fbf39ebd1 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -50,6 +50,7 @@ import { Disposable } from './class/dispose' import { collectWorkspaceScale } from './telemetry/collect' import { createFileSystemWatcher } from './fileSystem/watcher' import { GitExecutor } from './cli/git/executor' +import { GitReader } from './cli/git/reader' export class Extension extends Disposable implements IExtension { protected readonly internalCommands: InternalCommands @@ -65,6 +66,7 @@ export class Extension extends Disposable implements IExtension { private readonly cliReader: CliReader private readonly cliRunner: CliRunner private readonly gitExecutor: GitExecutor + private readonly gitReader: GitReader private readonly status: Status private cliAccessible = false private cliCompatible: boolean | undefined @@ -99,23 +101,24 @@ export class Extension extends Disposable implements IExtension { this.cliExecutor = this.dispose.track(new CliExecutor(this.config)) this.cliReader = this.dispose.track(new CliReader(this.config)) this.cliRunner = this.dispose.track(new CliRunner(this.config)) + this.gitExecutor = this.dispose.track(new GitExecutor()) + this.gitReader = this.dispose.track(new GitReader()) + + const clis = [ + this.cliExecutor, + this.cliReader, + this.cliRunner, + this.gitExecutor, + this.gitReader + ] const outputChannel = this.dispose.track( - new OutputChannel( - [this.cliExecutor, this.cliReader, this.cliRunner, this.gitExecutor], - context.extension.packageJSON.version - ) + new OutputChannel(clis, context.extension.packageJSON.version) ) this.internalCommands = this.dispose.track( - new InternalCommands( - outputChannel, - this.cliExecutor, - this.cliReader, - this.cliRunner, - this.gitExecutor - ) + new InternalCommands(outputChannel, ...clis) ) this.status = this.dispose.track( diff --git a/extension/src/git.ts b/extension/src/git.ts index 45970b09ed..a154787979 100644 --- a/extension/src/git.ts +++ b/extension/src/git.ts @@ -1,7 +1,5 @@ -import { join, resolve } from 'path' +import { join } from 'path' import { executeProcess } from './processExecution' -import { trimAndSplit } from './util/stdout' -import { isDirectory } from './fileSystem' export const DOT_GIT = '.git' export const DOT_GIT_HEAD = join(DOT_GIT, 'HEAD') @@ -10,98 +8,9 @@ export const GIT_REFS = join(DOT_GIT, 'refs') export const GIT_LOGS_REFS = join(DOT_GIT, 'logs', 'refs') export const HEADS_GIT_REFS = join(GIT_REFS, 'heads') -const getUris = (repositoryRoot: string, relativePaths: string[]) => - relativePaths.map(path => resolve(repositoryRoot, path)) - -const getUntrackedDirectories = async ( - repositoryRoot: string -): Promise => { - const output = await executeProcess({ - args: [ - 'ls-files', - '--others', - '--exclude-standard', - '--directory', - '--no-empty-directory' - ], - cwd: repositoryRoot, - executable: 'git' - }) - return getUris(repositoryRoot, trimAndSplit(output)).filter(path => - isDirectory(path) - ) -} - -const getUntrackedFiles = async (repositoryRoot: string): Promise => { - const output = await executeProcess({ - args: ['ls-files', '--others', '--exclude-standard'], - cwd: repositoryRoot, - executable: 'git' - }) - return getUris(repositoryRoot, trimAndSplit(output)) -} - -export const getAllUntracked = async ( - repositoryRoot: string -): Promise> => { - const [files, dirs] = await Promise.all([ - getUntrackedFiles(repositoryRoot), - getUntrackedDirectories(repositoryRoot) - ]) - return new Set([...files, ...dirs]) -} - -export const getHasChanges = async ( - repositoryRoot: string -): Promise => { - const output = await executeProcess({ - args: ['diff', '--name-only', '-z'], - cwd: repositoryRoot, - executable: 'git' - }) - return !!output -} - -export const gitReset = (cwd: string, ...args: string[]): Promise => - executeProcess({ - args: ['reset', ...args], - cwd, - executable: 'git' - }) - -export const gitResetWorkspace = async (cwd: string): Promise => { - await gitReset(cwd, '--hard', 'HEAD') - - await executeProcess({ - args: ['clean', '-f', '-d', '-q'], - cwd, - executable: 'git' - }) -} - export const getGitRepositoryRoot = (cwd: string): Promise => executeProcess({ args: ['rev-parse', '--show-toplevel'], cwd, executable: 'git' }) - -export const gitStageAll = async (cwd: string) => { - const repositoryRoot = await getGitRepositoryRoot(cwd) - - return executeProcess({ - args: ['add', '.'], - cwd: repositoryRoot, - executable: 'git' - }) -} - -export const gitPushBranch = ( - cwd: string, - branchName: string -): Promise => - executeProcess({ - args: ['push', '-u', 'origin', branchName], - cwd, - executable: 'git' - }) diff --git a/extension/src/repository/data/index.ts b/extension/src/repository/data/index.ts index f6a33625ad..489094ba01 100644 --- a/extension/src/repository/data/index.ts +++ b/extension/src/repository/data/index.ts @@ -3,12 +3,7 @@ import { Event, EventEmitter } from 'vscode' import { AvailableCommands, InternalCommands } from '../../commands/internal' import { DiffOutput, ListOutput, StatusOutput } from '../../cli/reader' import { isAnyDvcYaml } from '../../fileSystem' -import { - DOT_GIT, - getAllUntracked, - getGitRepositoryRoot, - getHasChanges -} from '../../git' +import { DOT_GIT, getGitRepositoryRoot } from '../../git' import { ProcessManager } from '../../processManager' import { createFileSystemWatcher, @@ -139,8 +134,14 @@ export class RepositoryData extends DeferredDisposable { ) const [untracked, hasGitChanges] = await Promise.all([ - getAllUntracked(this.dvcRoot), - getHasChanges(this.dvcRoot) + this.internalCommands.executeCommand>( + AvailableCommands.GIT_LIST_UNTRACKED, + this.dvcRoot + ), + this.internalCommands.executeCommand( + AvailableCommands.GIT_HAS_CHANGES, + this.dvcRoot + ) ]) return { diffFromCache, diffFromHead, hasGitChanges, untracked } diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index 168402925b..597881347e 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -63,8 +63,8 @@ import { Title } from '../../../vscode/title' import { ExperimentFlag } from '../../../cli/constants' import { WorkspaceExperiments } from '../../../experiments/workspace' import { CliExecutor } from '../../../cli/executor' -import * as Git from '../../../git' import { shortenForLabel } from '../../../util/string' +import { GitExecutor } from '../../../cli/git/executor' suite('Experiments Test Suite', () => { const disposable = Disposable.fn() @@ -503,7 +503,7 @@ suite('Experiments Test Suite', () => { const mockPush = stub(CliExecutor.prototype, 'push').resolves( '10 files updated.' ) - const mockGitPush = stub(Git, 'gitPushBranch') + const mockGitPush = stub(GitExecutor.prototype, 'pushBranch') const branchPushedToRemote = new Promise(resolve => mockGitPush.callsFake(() => { resolve(undefined) diff --git a/extension/src/test/suite/experiments/workspace.test.ts b/extension/src/test/suite/experiments/workspace.test.ts index c3f2fdb7f8..57f98a2aac 100644 --- a/extension/src/test/suite/experiments/workspace.test.ts +++ b/extension/src/test/suite/experiments/workspace.test.ts @@ -26,7 +26,7 @@ import { WEBVIEW_TEST_TIMEOUT } from '../timeouts' import { Title } from '../../../vscode/title' import { join } from '../../util/path' import { AvailableCommands } from '../../../commands/internal' -import * as Git from '../../../git' +import { GitExecutor } from '../../../cli/git/executor' suite('Workspace Experiments Test Suite', () => { const disposable = Disposable.fn() @@ -636,7 +636,7 @@ suite('Workspace Experiments Test Suite', () => { const mockPush = stub(CliExecutor.prototype, 'push').resolves( '10 files updated.' ) - const mockGitPush = stub(Git, 'gitPushBranch') + const mockGitPush = stub(GitExecutor.prototype, 'pushBranch') const branchPushedToRemote = new Promise(resolve => mockGitPush.callsFake(() => { resolve(undefined) diff --git a/extension/src/test/suite/extension.test.ts b/extension/src/test/suite/extension.test.ts index 8ef40f9111..e0b269d77e 100644 --- a/extension/src/test/suite/extension.test.ts +++ b/extension/src/test/suite/extension.test.ts @@ -31,6 +31,7 @@ import { QuickPickItemWithValue } from '../../vscode/quickPick' import { MIN_CLI_VERSION } from '../../cli/constants' import * as WorkspaceFolders from '../../vscode/workspaceFolders' import { CliExecutor } from '../../cli/executor' +import { GitReader } from '../../cli/git/reader' suite('Extension Test Suite', () => { const dvcPathOption = 'dvc.dvcPath' @@ -285,6 +286,9 @@ suite('Extension Test Suite', () => { 'isReady' ) + stub(GitReader.prototype, 'hasChanges').resolves(false) + stub(GitReader.prototype, 'listUntracked').resolves(new Set()) + const workspaceExperimentsAreReady = new Promise(resolve => mockWorkspaceExperimentsReady.callsFake(async () => { await mockWorkspaceExperimentsReady.wrappedMethod() @@ -460,6 +464,8 @@ suite('Extension Test Suite', () => { stub(CliReader.prototype, 'diff').resolves({}) stub(CliReader.prototype, 'plotsDiff').resolves({}) stub(CliReader.prototype, 'status').resolves({}) + stub(GitReader.prototype, 'hasChanges').resolves(false) + stub(GitReader.prototype, 'listUntracked').resolves(new Set()) const mockVersion = stub(CliReader.prototype, 'version') .onFirstCall() diff --git a/extension/src/test/suite/repository/sourceControlManagement.test.ts b/extension/src/test/suite/repository/sourceControlManagement.test.ts index 8b30305315..53c1454d57 100644 --- a/extension/src/test/suite/repository/sourceControlManagement.test.ts +++ b/extension/src/test/suite/repository/sourceControlManagement.test.ts @@ -11,8 +11,9 @@ import { RegisteredCliCommands, RegisteredCommands } from '../../../commands/external' -import * as ProcessExecution from '../../../processExecution' import { WorkspaceRepositories } from '../../../repository/workspace' +import { Cli } from '../../../cli' +import * as Git from '../../../git' suite('Source Control Management Test Suite', () => { const disposable = Disposable.fn() @@ -204,23 +205,20 @@ suite('Source Control Management Test Suite', () => { it('should stage all git tracked files', async () => { const gitRoot = resolve(dvcDemoPath, '..') - const mockGit = stub(ProcessExecution, 'executeProcess') - .onFirstCall() - .resolves(gitRoot) - .onSecondCall() - .resolves('') + const mockGit = stub(Git, 'getGitRepositoryRoot').resolves(gitRoot) + const mockExecuteProcess = stub( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Cli.prototype as any, + 'executeProcess' + ).resolves('') await commands.executeCommand(RegisteredCommands.GIT_STAGE_ALL, { rootUri }) - expect(mockGit).to.be.calledTwice - expect(mockGit).to.be.calledWith({ - args: ['rev-parse', '--show-toplevel'], - cwd: dvcDemoPath, - executable: 'git' - }) - expect(mockGit).to.be.calledWith({ + expect(mockGit).to.be.calledOnce + expect(mockExecuteProcess).to.be.calledOnce + expect(mockExecuteProcess).to.be.calledWith({ args: ['add', '.'], cwd: gitRoot, executable: 'git' @@ -228,14 +226,15 @@ suite('Source Control Management Test Suite', () => { }) it('should unstage all git tracked files', async () => { - const mockGit = stub(ProcessExecution, 'executeProcess').resolves('') + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mockExecuteProcess = stub(Cli.prototype as any, 'executeProcess') await commands.executeCommand(RegisteredCommands.GIT_UNSTAGE_ALL, { rootUri }) - expect(mockGit).to.be.calledOnce - expect(mockGit).to.be.calledWith({ + expect(mockExecuteProcess).to.be.calledOnce + expect(mockExecuteProcess).to.be.calledWith({ args: ['reset'], cwd: dvcDemoPath, executable: 'git' @@ -244,7 +243,9 @@ suite('Source Control Management Test Suite', () => { it('should not reset the workspace if the user does not confirm', async () => { const mockCheckout = stub(CliExecutor.prototype, 'checkout').resolves('') - const mockGitReset = stub(ProcessExecution, 'executeProcess').resolves('') + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mockExecuteProcess = stub(Cli.prototype as any, 'executeProcess') + // eslint-disable-next-line @typescript-eslint/no-explicit-any stub(WorkspaceRepositories.prototype as any, 'hasChanges').returns(true) @@ -262,12 +263,14 @@ suite('Source Control Management Test Suite', () => { expect(mockShowWarningMessage).to.be.calledOnce expect(mockCheckout).not.to.be.called - expect(mockGitReset).not.to.be.called + expect(mockExecuteProcess).not.to.be.called }) it('should reset the workspace if the user confirms they want to', async () => { const mockCheckout = stub(CliExecutor.prototype, 'checkout').resolves('') - const mockGitReset = stub(ProcessExecution, 'executeProcess').resolves('') + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mockExecuteProcess = stub(Cli.prototype as any, 'executeProcess') + // eslint-disable-next-line @typescript-eslint/no-explicit-any stub(WorkspaceRepositories.prototype as any, 'hasChanges').returns(true) @@ -285,13 +288,13 @@ suite('Source Control Management Test Suite', () => { expect(mockShowWarningMessage).to.be.calledOnce expect(mockCheckout).to.be.calledOnce - expect(mockGitReset).to.be.calledTwice - expect(mockGitReset).to.be.calledWith({ + expect(mockExecuteProcess).to.be.calledTwice + expect(mockExecuteProcess).to.be.calledWith({ args: ['reset', '--hard', 'HEAD'], cwd: dvcDemoPath, executable: 'git' }) - expect(mockGitReset).to.be.calledWith({ + expect(mockExecuteProcess).to.be.calledWith({ args: ['clean', '-f', '-d', '-q'], cwd: dvcDemoPath, executable: 'git' @@ -300,7 +303,8 @@ suite('Source Control Management Test Suite', () => { it('should not reset the workspace if there is another user initiated command running', async () => { const mockCheckout = stub(CliExecutor.prototype, 'checkout').resolves('') - const mockGitReset = stub(ProcessExecution, 'executeProcess').resolves('') + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mockExecuteProcess = stub(Cli.prototype as any, 'executeProcess') // eslint-disable-next-line @typescript-eslint/no-explicit-any stub(WorkspaceRepositories.prototype as any, 'hasChanges').returns(true) @@ -314,7 +318,7 @@ suite('Source Control Management Test Suite', () => { ) expect(mockCheckout).not.to.be.called - expect(mockGitReset).not.to.be.called + expect(mockExecuteProcess).not.to.be.called }) }) }) diff --git a/extension/src/test/suite/repository/util.ts b/extension/src/test/suite/repository/util.ts index 231b791b78..f554acb486 100644 --- a/extension/src/test/suite/repository/util.ts +++ b/extension/src/test/suite/repository/util.ts @@ -7,7 +7,6 @@ import { mockDisposable } from '../util' import { Disposer } from '../../../extension' -import * as Git from '../../../git' import { RepositoryData } from '../../../repository/data' import * as Time from '../../../util/time' import * as Watcher from '../../../fileSystem/watcher' @@ -17,7 +16,8 @@ import { DecorationProvider } from '../../../repository/decorationProvider' import { SourceControlManagement } from '../../../repository/sourceControlManagement' export const buildDependencies = (disposer: Disposer) => { - const { cliReader, internalCommands } = buildInternalCommands(disposer) + const { cliReader, gitReader, internalCommands } = + buildInternalCommands(disposer) const mockCreateFileSystemWatcher = stub( Watcher, @@ -27,8 +27,8 @@ export const buildDependencies = (disposer: Disposer) => { const mockListDvcOnlyRecursive = stub(cliReader, 'listDvcOnlyRecursive') const mockStatus = stub(cliReader, 'status') const mockDiff = stub(cliReader, 'diff') - const mockGetAllUntracked = stub(Git, 'getAllUntracked') - const mockGetHasChanges = stub(Git, 'getHasChanges') + const mockGetAllUntracked = stub(gitReader, 'listUntracked') + const mockGetHasChanges = stub(gitReader, 'hasChanges') const mockNow = stub(Time, 'getCurrentEpoch') const treeDataChanged = disposer.track(new EventEmitter()) diff --git a/extension/src/test/suite/util.ts b/extension/src/test/suite/util.ts index b9669a99bf..8b9ff05ed2 100644 --- a/extension/src/test/suite/util.ts +++ b/extension/src/test/suite/util.ts @@ -30,6 +30,7 @@ import { MessageFromWebview } from '../../webview/contract' import { PlotsData } from '../../plots/webview/contract' import { TableData } from '../../experiments/webview/contract' import { CliExecutor } from '../../cli/executor' +import { GitReader } from '../../cli/git/reader' export const mockDisposable = { dispose: stub() @@ -131,16 +132,23 @@ export const buildInternalCommands = (disposer: Disposer) => { const cliReader = disposer.track(new CliReader(config)) const cliRunner = disposer.track(new CliRunner(config)) const cliExecutor = disposer.track(new CliExecutor(config)) + const gitReader = disposer.track(new GitReader()) const outputChannel = disposer.track( new OutputChannel([cliReader], '1', 'test output') ) const internalCommands = disposer.track( - new InternalCommands(outputChannel, cliExecutor, cliReader, cliRunner) + new InternalCommands( + outputChannel, + cliExecutor, + cliReader, + cliRunner, + gitReader + ) ) - return { cliExecutor, cliReader, cliRunner, internalCommands } + return { cliExecutor, cliReader, cliRunner, gitReader, internalCommands } } export const buildMockData = () => @@ -154,7 +162,7 @@ export const buildDependencies = ( expShow = expShowFixture, plotsDiff = plotsDiffFixture ) => { - const { cliExecutor, cliReader, cliRunner, internalCommands } = + const { cliExecutor, cliReader, cliRunner, gitReader, internalCommands } = buildInternalCommands(disposer) const mockCreateFileSystemWatcher = stub( @@ -176,6 +184,7 @@ export const buildDependencies = ( cliExecutor, cliReader, cliRunner, + gitReader, internalCommands, messageSpy, mockCreateFileSystemWatcher, From a14ebd1d66f6a8e0ba0f9bdc5814a9d1a77af5a9 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 22 Aug 2022 13:18:07 +1000 Subject: [PATCH 3/7] register git commands as cli commands (offer to show error) --- extension/src/commands/external.ts | 10 +++++----- extension/src/repository/commands/register.ts | 8 ++++---- .../suite/repository/sourceControlManagement.test.ts | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/extension/src/commands/external.ts b/extension/src/commands/external.ts index 7bb72d236c..5e025138fd 100644 --- a/extension/src/commands/external.ts +++ b/extension/src/commands/external.ts @@ -38,7 +38,10 @@ export enum RegisteredCliCommands { PUSH = 'dvc.push', PUSH_TARGET = 'dvc.pushTarget', REMOVE_TARGET = 'dvc.removeTarget', - RENAME_TARGET = 'dvc.renameTarget' + RENAME_TARGET = 'dvc.renameTarget', + + GIT_STAGE_ALL = 'dvc.gitStageAll', + GIT_UNSTAGE_ALL = 'dvc.gitUnstageAll' } export enum RegisteredCommands { @@ -83,8 +86,5 @@ export enum RegisteredCommands { TRACKED_EXPLORER_COPY_REL_FILE_PATH = 'dvc.copyRelativeFilePath', TRACKED_EXPLORER_FIND_IN_FOLDER = 'dvc.findInFolder', TRACKED_EXPLORER_OPEN_TO_THE_SIDE = 'dvc.openToTheSide', - TRACKED_EXPLORER_SELECT_FOR_COMPARE = 'dvc.selectForCompare', - - GIT_STAGE_ALL = 'dvc.gitStageAll', - GIT_UNSTAGE_ALL = 'dvc.gitUnstageAll' + TRACKED_EXPLORER_SELECT_FOR_COMPARE = 'dvc.selectForCompare' } diff --git a/extension/src/repository/commands/register.ts b/extension/src/repository/commands/register.ts index bbae7a0fee..4cb0b199f1 100644 --- a/extension/src/repository/commands/register.ts +++ b/extension/src/repository/commands/register.ts @@ -37,13 +37,13 @@ const registerResourceGroupCommands = ( repositories: WorkspaceRepositories, internalCommands: InternalCommands ) => { - internalCommands.registerExternalCommand( - RegisteredCommands.GIT_STAGE_ALL, + internalCommands.registerExternalCliCommand( + RegisteredCliCommands.GIT_STAGE_ALL, getStageAllCommand(repositories, internalCommands) ) - internalCommands.registerExternalCommand( - RegisteredCommands.GIT_UNSTAGE_ALL, + internalCommands.registerExternalCliCommand( + RegisteredCliCommands.GIT_UNSTAGE_ALL, getUnstageAllCommand(repositories, internalCommands) ) } diff --git a/extension/src/test/suite/repository/sourceControlManagement.test.ts b/extension/src/test/suite/repository/sourceControlManagement.test.ts index 53c1454d57..afc80b0bf9 100644 --- a/extension/src/test/suite/repository/sourceControlManagement.test.ts +++ b/extension/src/test/suite/repository/sourceControlManagement.test.ts @@ -212,7 +212,7 @@ suite('Source Control Management Test Suite', () => { 'executeProcess' ).resolves('') - await commands.executeCommand(RegisteredCommands.GIT_STAGE_ALL, { + await commands.executeCommand(RegisteredCliCommands.GIT_STAGE_ALL, { rootUri }) @@ -229,7 +229,7 @@ suite('Source Control Management Test Suite', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const mockExecuteProcess = stub(Cli.prototype as any, 'executeProcess') - await commands.executeCommand(RegisteredCommands.GIT_UNSTAGE_ALL, { + await commands.executeCommand(RegisteredCliCommands.GIT_UNSTAGE_ALL, { rootUri }) From 9604300ee7bf6e9c61490ca411e42d5493fca359 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 22 Aug 2022 13:32:19 +1000 Subject: [PATCH 4/7] move constants --- extension/src/cli/git/constants.ts | 9 +++++++++ extension/src/cli/options.ts | 2 +- extension/src/experiments/data/constants.ts | 2 +- extension/src/experiments/data/index.ts | 8 ++------ extension/src/git.ts | 8 -------- extension/src/repository/data/index.ts | 3 ++- extension/src/test/suite/experiments/data/index.test.ts | 3 ++- extension/src/test/suite/repository/data/index.test.ts | 3 ++- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/extension/src/cli/git/constants.ts b/extension/src/cli/git/constants.ts index 50b53f6650..d5c2563964 100644 --- a/extension/src/cli/git/constants.ts +++ b/extension/src/cli/git/constants.ts @@ -1,3 +1,12 @@ +import { join } from 'path' + +export const DOT_GIT = '.git' +export const DOT_GIT_HEAD = join(DOT_GIT, 'HEAD') +export const DOT_GIT_INDEX = join(DOT_GIT, 'index') +export const GIT_REFS = join(DOT_GIT, 'refs') +export const GIT_LOGS_REFS = join(DOT_GIT, 'logs', 'refs') +export const HEADS_GIT_REFS = join(GIT_REFS, 'heads') + export enum Command { ADD = 'add', CLEAN = 'clean', diff --git a/extension/src/cli/options.ts b/extension/src/cli/options.ts index 5a7c0440ff..9f7f00bfc4 100644 --- a/extension/src/cli/options.ts +++ b/extension/src/cli/options.ts @@ -5,7 +5,7 @@ import { getProcessEnv } from '../env' import { joinEnvPath } from '../util/env' import { ProcessOptions } from '../processExecution' -export type ExecutionOptions = ProcessOptions & { +type ExecutionOptions = ProcessOptions & { env: NodeJS.ProcessEnv } diff --git a/extension/src/experiments/data/constants.ts b/extension/src/experiments/data/constants.ts index 656e2172c0..0875cd0383 100644 --- a/extension/src/experiments/data/constants.ts +++ b/extension/src/experiments/data/constants.ts @@ -1,5 +1,5 @@ import { join } from 'path' -import { GIT_LOGS_REFS, GIT_REFS } from '../../git' +import { GIT_LOGS_REFS, GIT_REFS } from '../../cli/git/constants' export const EXPERIMENTS_GIT_REFS = join(GIT_REFS, 'exps') export const EXPERIMENTS_GIT_LOGS_REFS = join(GIT_LOGS_REFS, 'exps') diff --git a/extension/src/experiments/data/index.ts b/extension/src/experiments/data/index.ts index fd9746b665..0ed61aeda4 100644 --- a/extension/src/experiments/data/index.ts +++ b/extension/src/experiments/data/index.ts @@ -10,16 +10,12 @@ import { createFileSystemWatcher, getRelativePattern } from '../../fileSystem/watcher' -import { - DOT_GIT, - DOT_GIT_HEAD, - HEADS_GIT_REFS, - getGitRepositoryRoot -} from '../../git' +import { getGitRepositoryRoot } from '../../git' import { AvailableCommands, InternalCommands } from '../../commands/internal' import { ExperimentsOutput } from '../../cli/reader' import { BaseData } from '../../data' import { ExperimentFlag } from '../../cli/constants' +import { DOT_GIT, DOT_GIT_HEAD, HEADS_GIT_REFS } from '../../cli/git/constants' export const QUEUED_EXPERIMENT_PATH = join('.dvc', 'tmp', 'exps') diff --git a/extension/src/git.ts b/extension/src/git.ts index a154787979..e2a274169e 100644 --- a/extension/src/git.ts +++ b/extension/src/git.ts @@ -1,13 +1,5 @@ -import { join } from 'path' import { executeProcess } from './processExecution' -export const DOT_GIT = '.git' -export const DOT_GIT_HEAD = join(DOT_GIT, 'HEAD') -export const DOT_GIT_INDEX = join(DOT_GIT, 'index') -export const GIT_REFS = join(DOT_GIT, 'refs') -export const GIT_LOGS_REFS = join(DOT_GIT, 'logs', 'refs') -export const HEADS_GIT_REFS = join(GIT_REFS, 'heads') - export const getGitRepositoryRoot = (cwd: string): Promise => executeProcess({ args: ['rev-parse', '--show-toplevel'], diff --git a/extension/src/repository/data/index.ts b/extension/src/repository/data/index.ts index 489094ba01..c0bc96f8a7 100644 --- a/extension/src/repository/data/index.ts +++ b/extension/src/repository/data/index.ts @@ -3,7 +3,7 @@ import { Event, EventEmitter } from 'vscode' import { AvailableCommands, InternalCommands } from '../../commands/internal' import { DiffOutput, ListOutput, StatusOutput } from '../../cli/reader' import { isAnyDvcYaml } from '../../fileSystem' -import { DOT_GIT, getGitRepositoryRoot } from '../../git' +import { getGitRepositoryRoot } from '../../git' import { ProcessManager } from '../../processManager' import { createFileSystemWatcher, @@ -15,6 +15,7 @@ import { EXPERIMENTS_GIT_REFS } from '../../experiments/data/constants' import { DeferredDisposable } from '../../class/deferred' +import { DOT_GIT } from '../../cli/git/constants' export type Data = { diffFromHead: DiffOutput diff --git a/extension/src/test/suite/experiments/data/index.test.ts b/extension/src/test/suite/experiments/data/index.test.ts index 25c07301a2..275f5d7371 100644 --- a/extension/src/test/suite/experiments/data/index.test.ts +++ b/extension/src/test/suite/experiments/data/index.test.ts @@ -16,11 +16,12 @@ import { QUEUED_EXPERIMENT_PATH } from '../../../../experiments/data' import * as Watcher from '../../../../fileSystem/watcher' -import { DOT_GIT_HEAD, getGitRepositoryRoot } from '../../../../git' +import { getGitRepositoryRoot } from '../../../../git' import { InternalCommands } from '../../../../commands/internal' import { buildExperimentsData, buildExperimentsDataDependencies } from '../util' import { ExperimentFlag } from '../../../../cli/constants' import { EXPERIMENTS_GIT_LOGS_REFS } from '../../../../experiments/data/constants' +import { DOT_GIT_HEAD } from '../../../../cli/git/constants' suite('Experiments Data Test Suite', () => { const disposable = Disposable.fn() diff --git a/extension/src/test/suite/repository/data/index.test.ts b/extension/src/test/suite/repository/data/index.test.ts index a8102f64cc..c892370618 100644 --- a/extension/src/test/suite/repository/data/index.test.ts +++ b/extension/src/test/suite/repository/data/index.test.ts @@ -7,9 +7,10 @@ import { buildRepositoryData } from '../util' import { Disposable } from '../../../../extension' import { dvcDemoPath } from '../../../util' import { fireWatcher } from '../../../../fileSystem/watcher' -import { DOT_GIT_HEAD, getGitRepositoryRoot } from '../../../../git' +import { getGitRepositoryRoot } from '../../../../git' import { RepositoryData } from '../../../../repository/data' import { InternalCommands } from '../../../../commands/internal' +import { DOT_GIT_HEAD } from '../../../../cli/git/constants' suite('Repository Data Test Suite', () => { const disposable = Disposable.fn() From c2ed84ecc971957327cd3184cd9f31e114b97987 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 22 Aug 2022 14:36:07 +1000 Subject: [PATCH 5/7] remove git util --- extension/src/cli/git/executor.ts | 8 +++---- extension/src/cli/git/index.ts | 11 ++++++++++ extension/src/cli/git/reader.ts | 6 ++++-- extension/src/experiments/data/index.ts | 6 ++++-- extension/src/git.ts | 8 ------- extension/src/repository/data/index.ts | 6 ++++-- .../test/suite/experiments/data/index.test.ts | 20 ++++++++++++------ extension/src/test/suite/experiments/util.ts | 17 ++++++++++----- .../test/suite/repository/data/index.test.ts | 21 +++++++++++++------ .../sourceControlManagement.test.ts | 9 +++++--- 10 files changed, 74 insertions(+), 38 deletions(-) create mode 100644 extension/src/cli/git/index.ts delete mode 100644 extension/src/git.ts diff --git a/extension/src/cli/git/executor.ts b/extension/src/cli/git/executor.ts index 9b25c59124..026631d12d 100644 --- a/extension/src/cli/git/executor.ts +++ b/extension/src/cli/git/executor.ts @@ -1,7 +1,7 @@ +import { GitCli } from '.' import { Command, Commit, DEFAULT_REMOTE, Flag } from './constants' import { getOptions } from './options' -import { typeCheckCommands, Cli } from '..' -import { getGitRepositoryRoot } from '../../git' +import { typeCheckCommands } from '..' export const autoRegisteredCommands = { GIT_PUSH_BRANCH: 'pushBranch', @@ -10,7 +10,7 @@ export const autoRegisteredCommands = { GIT_UNSTAGE_ALL: 'reset' } as const -export class GitExecutor extends Cli { +export class GitExecutor extends GitCli { public readonly autoRegisteredCommands = typeCheckCommands( autoRegisteredCommands, this @@ -37,7 +37,7 @@ export class GitExecutor extends Cli { } public async stageAll(cwd: string) { - const gitRoot = await getGitRepositoryRoot(cwd) + const gitRoot = await this.getGitRepositoryRoot(cwd) const options = getOptions(gitRoot, Command.ADD, Flag.DOT) return this.executeProcess(options) diff --git a/extension/src/cli/git/index.ts b/extension/src/cli/git/index.ts new file mode 100644 index 0000000000..d54c33a51d --- /dev/null +++ b/extension/src/cli/git/index.ts @@ -0,0 +1,11 @@ +import { Command, Flag } from './constants' +import { getOptions } from './options' +import { Cli } from '..' + +export class GitCli extends Cli { + public getGitRepositoryRoot(cwd: string) { + const options = getOptions(cwd, Command.REV_PARSE, Flag.SHOW_TOPLEVEL) + + return this.executeProcess(options) + } +} diff --git a/extension/src/cli/git/reader.ts b/extension/src/cli/git/reader.ts index 14d623e8db..4c0a9c9f3e 100644 --- a/extension/src/cli/git/reader.ts +++ b/extension/src/cli/git/reader.ts @@ -1,16 +1,18 @@ import { resolve } from 'path' +import { GitCli } from '.' import { Command, Flag } from './constants' import { getOptions } from './options' -import { Cli, typeCheckCommands } from '..' +import { typeCheckCommands } from '..' import { trimAndSplit } from '../../util/stdout' import { isDirectory } from '../../fileSystem' export const autoRegisteredCommands = { + GIT_GET_REPOSITORY_ROOT: 'getGitRepositoryRoot', GIT_HAS_CHANGES: 'hasChanges', GIT_LIST_UNTRACKED: 'listUntracked' } as const -export class GitReader extends Cli { +export class GitReader extends GitCli { public readonly autoRegisteredCommands = typeCheckCommands( autoRegisteredCommands, this diff --git a/extension/src/experiments/data/index.ts b/extension/src/experiments/data/index.ts index 0ed61aeda4..e8d690c415 100644 --- a/extension/src/experiments/data/index.ts +++ b/extension/src/experiments/data/index.ts @@ -10,7 +10,6 @@ import { createFileSystemWatcher, getRelativePattern } from '../../fileSystem/watcher' -import { getGitRepositoryRoot } from '../../git' import { AvailableCommands, InternalCommands } from '../../commands/internal' import { ExperimentsOutput } from '../../cli/reader' import { BaseData } from '../../data' @@ -77,7 +76,10 @@ export class ExperimentsData extends BaseData { } private async watchExpGitRefs(): Promise { - const gitRoot = await getGitRepositoryRoot(this.dvcRoot) + const gitRoot = await this.internalCommands.executeCommand( + AvailableCommands.GIT_GET_REPOSITORY_ROOT, + this.dvcRoot + ) const watchedRelPaths = [ DOT_GIT_HEAD, EXPERIMENTS_GIT_REFS, diff --git a/extension/src/git.ts b/extension/src/git.ts deleted file mode 100644 index e2a274169e..0000000000 --- a/extension/src/git.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { executeProcess } from './processExecution' - -export const getGitRepositoryRoot = (cwd: string): Promise => - executeProcess({ - args: ['rev-parse', '--show-toplevel'], - cwd, - executable: 'git' - }) diff --git a/extension/src/repository/data/index.ts b/extension/src/repository/data/index.ts index c0bc96f8a7..5877da7515 100644 --- a/extension/src/repository/data/index.ts +++ b/extension/src/repository/data/index.ts @@ -3,7 +3,6 @@ import { Event, EventEmitter } from 'vscode' import { AvailableCommands, InternalCommands } from '../../commands/internal' import { DiffOutput, ListOutput, StatusOutput } from '../../cli/reader' import { isAnyDvcYaml } from '../../fileSystem' -import { getGitRepositoryRoot } from '../../git' import { ProcessManager } from '../../processManager' import { createFileSystemWatcher, @@ -153,7 +152,10 @@ export class RepositoryData extends DeferredDisposable { } private async watchWorkspace() { - const gitRoot = await getGitRepositoryRoot(this.dvcRoot) + const gitRoot = await this.internalCommands.executeCommand( + AvailableCommands.GIT_GET_REPOSITORY_ROOT, + this.dvcRoot + ) this.dispose.track( createFileSystemWatcher( diff --git a/extension/src/test/suite/experiments/data/index.test.ts b/extension/src/test/suite/experiments/data/index.test.ts index 275f5d7371..09ccd5e7d6 100644 --- a/extension/src/test/suite/experiments/data/index.test.ts +++ b/extension/src/test/suite/experiments/data/index.test.ts @@ -1,4 +1,4 @@ -import { join, sep } from 'path' +import { join, resolve, sep } from 'path' import { afterEach, beforeEach, describe, it, suite } from 'mocha' import { EventEmitter, FileSystemWatcher } from 'vscode' import { expect } from 'chai' @@ -16,8 +16,11 @@ import { QUEUED_EXPERIMENT_PATH } from '../../../../experiments/data' import * as Watcher from '../../../../fileSystem/watcher' -import { getGitRepositoryRoot } from '../../../../git' -import { InternalCommands } from '../../../../commands/internal' +import { + AvailableCommands, + CommandId, + InternalCommands +} from '../../../../commands/internal' import { buildExperimentsData, buildExperimentsDataDependencies } from '../util' import { ExperimentFlag } from '../../../../cli/constants' import { EXPERIMENTS_GIT_LOGS_REFS } from '../../../../experiments/data/constants' @@ -149,13 +152,20 @@ suite('Experiments Data Test Suite', () => { it('should watch the .git directory for updates', async () => { const mockNow = getMockNow() + const gitRoot = resolve(dvcDemoPath, '..') + + const mockExecuteCommand = (command: CommandId) => { + if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) { + return Promise.resolve(gitRoot) + } + } const data = disposable.track( new ExperimentsData( dvcDemoPath, { dispose: stub(), - executeCommand: stub() + executeCommand: mockExecuteCommand } as unknown as InternalCommands, disposable.track(new EventEmitter()) ) @@ -164,8 +174,6 @@ suite('Experiments Data Test Suite', () => { await data.isReady() bypassProcessManagerDebounce(mockNow) - const gitRoot = await getGitRepositoryRoot(dvcDemoPath) - const managedUpdateSpy = spy(data, 'managedUpdate') const dataUpdatedEvent = new Promise(resolve => data.onDidUpdate(() => resolve(undefined)) diff --git a/extension/src/test/suite/experiments/util.ts b/extension/src/test/suite/experiments/util.ts index ac335a92db..3d6f2c5288 100644 --- a/extension/src/test/suite/experiments/util.ts +++ b/extension/src/test/suite/experiments/util.ts @@ -4,7 +4,6 @@ import omit from 'lodash.omit' import { WorkspaceExperiments } from '../../../experiments/workspace' import { Experiments } from '../../../experiments' import { Disposer } from '../../../extension' -import * as Git from '../../../git' import expShowFixture from '../../fixtures/expShow/output' import { buildMockMemento, dvcDemoPath } from '../../util' import { @@ -47,6 +46,7 @@ export const buildExperiments = ( cliExecutor, cliReader, cliRunner, + gitReader, internalCommands, messageSpy, mockExperimentShow, @@ -79,6 +79,7 @@ export const buildExperiments = ( experiments, // eslint-disable-next-line @typescript-eslint/no-explicit-any experimentsModel: (experiments as any).experiments as ExperimentsModel, + gitReader, internalCommands, messageSpy, mockExperimentShow, @@ -91,12 +92,13 @@ export const buildMultiRepoExperiments = (disposer: Disposer) => { const { internalCommands, experiments: mockExperiments, + gitReader, messageSpy, updatesPaused, resourceLocator } = buildExperiments(disposer, expShowFixture, 'other/dvc/root') - stub(Git, 'getGitRepositoryRoot').resolves(dvcDemoPath) + stub(gitReader, 'getGitRepositoryRoot').resolves(dvcDemoPath) const workspaceExperiments = disposer.track( new WorkspaceExperiments( internalCommands, @@ -117,10 +119,15 @@ export const buildMultiRepoExperiments = (disposer: Disposer) => { } export const buildSingleRepoExperiments = (disposer: Disposer) => { - const { internalCommands, messageSpy, updatesPaused, resourceLocator } = - buildDependencies(disposer) + const { + internalCommands, + gitReader, + messageSpy, + updatesPaused, + resourceLocator + } = buildDependencies(disposer) - stub(Git, 'getGitRepositoryRoot').resolves(dvcDemoPath) + stub(gitReader, 'getGitRepositoryRoot').resolves(dvcDemoPath) const workspaceExperiments = disposer.track( new WorkspaceExperiments( internalCommands, diff --git a/extension/src/test/suite/repository/data/index.test.ts b/extension/src/test/suite/repository/data/index.test.ts index c892370618..9ff6b0ee44 100644 --- a/extension/src/test/suite/repository/data/index.test.ts +++ b/extension/src/test/suite/repository/data/index.test.ts @@ -1,4 +1,4 @@ -import { join } from 'path' +import { join, resolve } from 'path' import { afterEach, beforeEach, describe, it, suite } from 'mocha' import { expect } from 'chai' import { restore, spy, stub } from 'sinon' @@ -7,9 +7,12 @@ import { buildRepositoryData } from '../util' import { Disposable } from '../../../../extension' import { dvcDemoPath } from '../../../util' import { fireWatcher } from '../../../../fileSystem/watcher' -import { getGitRepositoryRoot } from '../../../../git' import { RepositoryData } from '../../../../repository/data' -import { InternalCommands } from '../../../../commands/internal' +import { + AvailableCommands, + CommandId, + InternalCommands +} from '../../../../commands/internal' import { DOT_GIT_HEAD } from '../../../../cli/git/constants' suite('Repository Data Test Suite', () => { @@ -123,20 +126,26 @@ suite('Repository Data Test Suite', () => { }) it('should watch the .git index and HEAD for updates', async () => { + const gitRoot = resolve(dvcDemoPath, '..') + + const mockExecuteCommand = (command: CommandId) => { + if (command === AvailableCommands.GIT_GET_REPOSITORY_ROOT) { + return Promise.resolve(gitRoot) + } + } + const data = disposable.track( new RepositoryData( dvcDemoPath, { dispose: stub(), - executeCommand: stub() + executeCommand: mockExecuteCommand } as unknown as InternalCommands, disposable.track(new EventEmitter()) ) ) await data.isReady() - const gitRoot = await getGitRepositoryRoot(dvcDemoPath) - const managedUpdateSpy = spy(data, 'managedUpdate') const dataUpdatedEvent = new Promise(resolve => data.onDidUpdate(() => resolve(undefined)) diff --git a/extension/src/test/suite/repository/sourceControlManagement.test.ts b/extension/src/test/suite/repository/sourceControlManagement.test.ts index afc80b0bf9..9da2d69b43 100644 --- a/extension/src/test/suite/repository/sourceControlManagement.test.ts +++ b/extension/src/test/suite/repository/sourceControlManagement.test.ts @@ -13,7 +13,7 @@ import { } from '../../../commands/external' import { WorkspaceRepositories } from '../../../repository/workspace' import { Cli } from '../../../cli' -import * as Git from '../../../git' +import { GitCli } from '../../../cli/git' suite('Source Control Management Test Suite', () => { const disposable = Disposable.fn() @@ -205,7 +205,10 @@ suite('Source Control Management Test Suite', () => { it('should stage all git tracked files', async () => { const gitRoot = resolve(dvcDemoPath, '..') - const mockGit = stub(Git, 'getGitRepositoryRoot').resolves(gitRoot) + const mockGetGitRepositoryRoot = stub( + GitCli.prototype, + 'getGitRepositoryRoot' + ).resolves(gitRoot) const mockExecuteProcess = stub( // eslint-disable-next-line @typescript-eslint/no-explicit-any Cli.prototype as any, @@ -216,7 +219,7 @@ suite('Source Control Management Test Suite', () => { rootUri }) - expect(mockGit).to.be.calledOnce + expect(mockGetGitRepositoryRoot).to.be.calledOnce expect(mockExecuteProcess).to.be.calledOnce expect(mockExecuteProcess).to.be.calledWith({ args: ['add', '.'], From 0aaf9285eb577f334b09b09b8ac38d7fc2b565ca Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 22 Aug 2022 15:37:52 +1000 Subject: [PATCH 6/7] remove arrow functions from new class --- extension/src/cli/git/reader.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/src/cli/git/reader.ts b/extension/src/cli/git/reader.ts index 4c0a9c9f3e..07694c57e0 100644 --- a/extension/src/cli/git/reader.ts +++ b/extension/src/cli/git/reader.ts @@ -38,7 +38,7 @@ export class GitReader extends GitCli { return new Set([...files, ...dirs]) } - private getUntrackedDirectories = async (cwd: string): Promise => { + private async getUntrackedDirectories(cwd: string): Promise { const options = getOptions( cwd, Command.LS_FILES, @@ -54,7 +54,7 @@ export class GitReader extends GitCli { ) } - private getUntrackedFiles = async (cwd: string): Promise => { + private async getUntrackedFiles(cwd: string): Promise { const options = getOptions( cwd, Command.LS_FILES, From dfbd2dd0a927999ff1bece62d0702db112cfee04 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 22 Aug 2022 15:39:55 +1000 Subject: [PATCH 7/7] revert exporting of type --- extension/src/cli/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/src/cli/index.ts b/extension/src/cli/index.ts index 9d77b98c64..619625eb2a 100644 --- a/extension/src/cli/index.ts +++ b/extension/src/cli/index.ts @@ -5,7 +5,7 @@ import { createProcess, ProcessOptions } from '../processExecution' import { StopWatch } from '../util/time' import { Disposable } from '../class/dispose' -export type CliEvent = { +type CliEvent = { command: string cwd: string pid: number | undefined