Skip to content

Commit

Permalink
Cody Ignore: remove git CLI repo-name resolver (#4132)
Browse files Browse the repository at this point in the history
Removes the git CLI repo-name resolver because it performs worse than other existing alternatives (VS Code Git extension and file system tree-walk). There are no cases where it is preferred over alternatives, so we can simplify the codebase by deleting it.
  • Loading branch information
valerybugakov committed May 10, 2024
1 parent b7364df commit e01888c
Show file tree
Hide file tree
Showing 10 changed files with 2,003 additions and 2,578 deletions.
4,408 changes: 1,985 additions & 2,423 deletions agent/recordings/enterpriseClient_3965582033/recording.har.yaml

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion lib/shared/src/sourcegraph-api/graphql/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ query Repositories($first: Int!, $after: String) {
nodes {
id
name
url
}
pageInfo {
endCursor
Expand Down
26 changes: 14 additions & 12 deletions vscode/src/context/workspace-repo-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ const GIT_REFRESH_DELAY = 2000
// IDs. This depends on the vscode.git extension for mapping git repositories
// to their remotes.
export class WorkspaceRepoMapper implements vscode.Disposable, CodebaseRepoIdMapper {
private changeEmitter = new vscode.EventEmitter<{ name: string; id: string }[]>()
private changeEmitter = new vscode.EventEmitter<Repo[]>()
private disposables: vscode.Disposable[] = [this.changeEmitter]
// The workspace repos.
private repos: { name: string; id: string }[] = []
private repos: Repo[] = []
// A cache of results for non-workspace repos. This caches repos that are
// not found, as well as repo IDs.
private nonWorkspaceRepos = new Map<string, string | undefined>()
Expand Down Expand Up @@ -108,11 +108,11 @@ export class WorkspaceRepoMapper implements vscode.Disposable, CodebaseRepoIdMap
return this.started
}

public get workspaceRepos(): { name: string; id: string }[] {
public get workspaceRepos(): Repo[] {
return [...this.repos]
}

public get onChange(): vscode.Event<{ name: string; id: string }[]> {
public get onChange(): vscode.Event<Repo[]> {
return this.changeEmitter.event
}

Expand All @@ -126,7 +126,7 @@ export class WorkspaceRepoMapper implements vscode.Disposable, CodebaseRepoIdMap
.map(f => f.uri.toString())
.join()}`
)
this.repos = await this.findRepoIds(folders)
this.repos = await this.findRepos(folders)
} catch (error) {
logDebug('WorkspaceRepoMapper', `Error mapping workspace folders to repo IDs: ${error}`)
throw error
Expand All @@ -136,9 +136,7 @@ export class WorkspaceRepoMapper implements vscode.Disposable, CodebaseRepoIdMap

// Given a set of workspace folders, looks up their git remotes and finds the related repo IDs,
// if any.
private async findRepoIds(
folders: readonly vscode.WorkspaceFolder[]
): Promise<{ name: string; id: string }[]> {
private async findRepos(folders: readonly vscode.WorkspaceFolder[]): Promise<Repo[]> {
const repoNames = new Set(
folders.flatMap(folder => {
const codebase = getCodebaseFromWorkspaceUri(folder.uri)
Expand All @@ -149,10 +147,14 @@ export class WorkspaceRepoMapper implements vscode.Disposable, CodebaseRepoIdMap
// Otherwise we fetch the first 10 repos from the Sourcegraph instance
return []
}
const ids = await graphqlClient.getRepoIds([...repoNames.values()], RemoteSearch.MAX_REPO_COUNT)
if (isError(ids)) {
throw ids
const repos = await graphqlClient.getRepoIds(
[...repoNames.values()],
RemoteSearch.MAX_REPO_COUNT
)
if (isError(repos)) {
throw repos
}
return ids

return repos
}
}
2 changes: 0 additions & 2 deletions vscode/src/extension.common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import type { ContextRankerConfig, ContextRankingController } from './local-cont
import type { LocalEmbeddingsConfig, LocalEmbeddingsController } from './local-context/local-embeddings'
import type { SymfRunner } from './local-context/symf'
import { start } from './main'
import type { RemoteUrlGetter } from './repository/enterprise-repo-name-resolver'
import type {
OpenTelemetryService,
OpenTelemetryServiceConfig,
Expand All @@ -48,7 +47,6 @@ export interface PlatformContext {
) => SourcegraphCompletionsClient
createSentryService?: (config: Pick<ConfigurationWithAccessToken, 'serverEndpoint'>) => SentryService
createOpenTelemetryService?: (config: OpenTelemetryServiceConfig) => OpenTelemetryService
getRemoteUrlGetters?: () => RemoteUrlGetter[]
startTokenReceiver?: typeof startTokenReceiver
onConfigurationChange?: (configuration: Configuration) => void
extensionClient: ExtensionClient
Expand Down
2 changes: 0 additions & 2 deletions vscode/src/extension.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
createLocalEmbeddingsController,
} from './local-context/local-embeddings'
import { SymfRunner } from './local-context/symf'
import { gitRemoteUrlsFromGitCli } from './repository/repo-name-getter.node'
import { OpenTelemetryService } from './services/open-telemetry/OpenTelemetryService.node'

/**
Expand Down Expand Up @@ -58,7 +57,6 @@ export function activate(
createBfgRetriever: () => new BfgRetriever(context),
createSentryService: (...args) => new NodeSentryService(...args),
createOpenTelemetryService: (...args) => new OpenTelemetryService(...args),
getRemoteUrlGetters: () => [gitRemoteUrlsFromGitCli],
startTokenReceiver: (...args) => startTokenReceiver(...args),

onConfigurationChange: setCustomAgent,
Expand Down
1 change: 0 additions & 1 deletion vscode/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,6 @@ const register = async (

const commandsManager = platform.createCommandsProvider?.()
setCommandController(commandsManager)
enterpriseRepoNameResolver.init(platform.getRemoteUrlGetters?.())

// Execute Cody Commands and Cody Custom Commands
const executeCommand = (
Expand Down
18 changes: 0 additions & 18 deletions vscode/src/repository/enterprise-repo-name-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,9 @@ type RemoteUrl = string
type UriFsPath = string

export class EnterpriseRepoNameResolver {
private platformSpecificGitRemoteGetters: RemoteUrlGetter[] = []
private fsPathToRepoNameCache = new LRUCache<UriFsPath, RepoName[]>({ max: 1000 })
private remoteUrlToRepoNameCache = new LRUCache<RemoteUrl, Promise<RepoName | null>>({ max: 1000 })

/**
* Currently is used to set node specific remote url getters on the extension init.
*/
public init(platformSpecificGitRemoteGetters: RemoteUrlGetter[] = []) {
this.platformSpecificGitRemoteGetters = platformSpecificGitRemoteGetters
}

/**
* Gets the repo names for a file URI.
*
Expand All @@ -50,16 +42,6 @@ export class EnterpriseRepoNameResolver {
remoteUrls = await gitRemoteUrlsFromTreeWalk(uri)
}

if (remoteUrls === undefined || remoteUrls.length === 0) {
for (const getter of this.platformSpecificGitRemoteGetters) {
remoteUrls = await getter(uri)

if (remoteUrls?.length !== 0) {
break
}
}
}

if (remoteUrls) {
const uniqueRemoteUrls = Array.from(new Set(remoteUrls))
const repoNames = await Promise.all(
Expand Down
71 changes: 0 additions & 71 deletions vscode/src/repository/repo-name-getter.node.test.ts

This file was deleted.

47 changes: 0 additions & 47 deletions vscode/src/repository/repo-name-getter.node.ts

This file was deleted.

5 changes: 4 additions & 1 deletion vscode/src/repository/repo-name-resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { URI } from 'vscode-uri'
import { graphqlClient } from '@sourcegraph/cody-shared'
import dedent from 'dedent'
import { EnterpriseRepoNameResolver, gitRemoteUrlsFromTreeWalk } from './enterprise-repo-name-resolver'
import * as gitExtensionAPI from './git-extension-api'

interface MockFsCallsParams {
filePath: string
Expand Down Expand Up @@ -258,7 +259,9 @@ describe('gitRemoteUrlFromTreeWalk', () => {
describe('getRepoNamesFromWorkspaceUri', () => {
it('resolves the repo name using graphql', async () => {
const resolver = new EnterpriseRepoNameResolver()
resolver.init([() => Promise.resolve(['git@github.com:sourcegraph/cody.git'])])
vi.spyOn(gitExtensionAPI, 'gitRemoteUrlsFromGitExtension').mockReturnValue([
'git@github.com:sourcegraph/cody.git',
])

const { fileUri } = mockFsCalls({
filePath: '/repo/submodule/foo.ts',
Expand Down

0 comments on commit e01888c

Please sign in to comment.