-
Notifications
You must be signed in to change notification settings - Fork 9
feat: submodule discovery #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
40af4db
9917f47
4445033
6e910bc
a66f016
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||||||||||||||
| import * as path from "node:path"; | ||||||||||||||||||
| import * as cp from "node:child_process"; | ||||||||||||||||||
|
|
||||||||||||||||||
| import { escapeRefName, getPathFromStr } from "./backend/utils"; | ||||||||||||||||||
|
|
@@ -203,9 +204,9 @@ export class DataSource { | |||||||||||||||||
| }); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| public isGitRepository(path: string) { | ||||||||||||||||||
| public isGitRepository(repoPath: string) { | ||||||||||||||||||
| return new Promise<boolean>((resolve) => { | ||||||||||||||||||
| this.execGit("rev-parse --git-dir", path, (err) => { | ||||||||||||||||||
| this.execGit("rev-parse --git-dir", repoPath, (err) => { | ||||||||||||||||||
| resolve(!err); | ||||||||||||||||||
| }); | ||||||||||||||||||
| }); | ||||||||||||||||||
|
|
@@ -462,4 +463,49 @@ export class DataSource { | |||||||||||||||||
| }); | ||||||||||||||||||
| }); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| public getSubmodules(repo: string) { | ||||||||||||||||||
| return new Promise<string[]>((resolve) => { | ||||||||||||||||||
| cp.execFile( | ||||||||||||||||||
| this.gitPath, | ||||||||||||||||||
| ["config", "-f", ".gitmodules", "--get-regexp", "^submodule\\..*\\.path$"], | ||||||||||||||||||
| { cwd: repo }, | ||||||||||||||||||
| async (err, stdout) => { | ||||||||||||||||||
| const submodules: string[] = []; | ||||||||||||||||||
| if (err) { | ||||||||||||||||||
| // Missing .gitmodules or no submodule.path entries. | ||||||||||||||||||
| resolve(submodules); | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const lines = stdout.split(eolRegex); | ||||||||||||||||||
|
|
||||||||||||||||||
| for (let i = 0; i < lines.length; i++) { | ||||||||||||||||||
| const line = lines[i]; | ||||||||||||||||||
| if (line.trim() === "") continue; | ||||||||||||||||||
|
|
||||||||||||||||||
| const separatorPos = line.search(/\s/); | ||||||||||||||||||
| if (separatorPos === -1) continue; | ||||||||||||||||||
|
|
||||||||||||||||||
| const rawPath = line.slice(separatorPos + 1).trim(); | ||||||||||||||||||
| if (rawPath === "") continue; | ||||||||||||||||||
|
|
||||||||||||||||||
| const resolvedPath = path.resolve(repo, rawPath); | ||||||||||||||||||
| const relativeToRepo = path.relative(repo, resolvedPath); | ||||||||||||||||||
| // Ignore entries that escape the repository root. | ||||||||||||||||||
| if (relativeToRepo.startsWith("..") || path.isAbsolute(relativeToRepo)) continue; | ||||||||||||||||||
|
||||||||||||||||||
| if (relativeToRepo.startsWith("..") || path.isAbsolute(relativeToRepo)) continue; | |
| if ( | |
| relativeToRepo === ".." || | |
| relativeToRepo.startsWith(".." + path.sep) || | |
| path.isAbsolute(relativeToRepo) | |
| ) { | |
| continue; | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,6 +95,7 @@ export class RepoManager { | |
| if (!(await this.checkReposExist())) this.sendRepos(); | ||
| await this.searchWorkspaceForRepos(); | ||
| this.startWatchingFolders(); | ||
| if (await this.checkReposForNewSubmodules()) this.sendRepos(); | ||
| } | ||
|
|
||
| private removeReposNotInWorkspace() { | ||
|
|
@@ -123,9 +124,12 @@ export class RepoManager { | |
| public getRepos() { | ||
| return sortRepos(this.repos); | ||
| } | ||
| private addRepo(repo: string) { | ||
| private async addRepo(repo: string) { | ||
| if (typeof this.repos[repo] !== "undefined") return false; | ||
| this.repos[repo] = { columnWidths: null }; | ||
| await this.searchRepoForSubmodules(repo); | ||
| this.extensionState.saveRepos(this.repos); | ||
| return true; | ||
| } | ||
| private removeRepo(repo: string) { | ||
| delete this.repos[repo]; | ||
|
|
@@ -174,6 +178,39 @@ export class RepoManager { | |
| this.repos[repo] = state; | ||
| this.extensionState.saveRepos(this.repos); | ||
| } | ||
| private async searchRepoForSubmodules(repo: string) { | ||
| let changes = false; | ||
| const submodules = await this.dataSource.getSubmodules(repo); | ||
|
|
||
| for (let i = 0; i < submodules.length; i++) { | ||
| if (typeof this.repos[submodules[i]] !== "undefined") continue; | ||
|
|
||
| this.repos[submodules[i]] = { columnWidths: null }; | ||
| changes = true; | ||
| if (await this.searchRepoForSubmodules(submodules[i])) changes = true; | ||
| } | ||
|
|
||
| return changes; | ||
| } | ||
| private async checkReposForNewSubmodules() { | ||
| const repoPaths = Object.keys(this.repos); | ||
| if (repoPaths.length === 0) return false; | ||
|
|
||
| const results = await evalPromises(repoPaths, 2, (repoPath) => | ||
| this.searchRepoForSubmodules(repoPath) | ||
| ); | ||
| const changes = results.indexOf(true) > -1; | ||
|
|
||
| if (changes) this.extensionState.saveRepos(this.repos); | ||
| return changes; | ||
| } | ||
| private async checkRepoForNewSubmodules(repo: string) { | ||
| if (typeof this.repos[repo] === "undefined") return false; | ||
|
|
||
| const changes = await this.searchRepoForSubmodules(repo); | ||
| if (changes) this.extensionState.saveRepos(this.repos); | ||
| return changes; | ||
| } | ||
|
|
||
| /* Repo Searching */ | ||
| private async searchWorkspaceForRepos() { | ||
|
|
@@ -204,8 +241,7 @@ export class RepoManager { | |
| .isGitRepository(directory) | ||
| .then((isRepo) => { | ||
| if (isRepo) { | ||
| this.addRepo(directory); | ||
| resolve(true); | ||
| this.addRepo(directory).then(resolve); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason to change this logic? |
||
| } else if (maxDepth > 0) { | ||
| fs.readdir(directory, async (err, dirContents) => { | ||
| if (err) { | ||
|
|
@@ -289,6 +325,9 @@ export class RepoManager { | |
| while ((path = this.createEventPaths.shift())) { | ||
| if (await isDirectory(path)) { | ||
| if (await this.searchDirectoryForRepos(path, this.maxDepthOfRepoSearch)) changes = true; | ||
| } else if (path.endsWith("/.gitmodules")) { | ||
| const repo = path.slice(0, -"/.gitmodules".length); | ||
| if (await this.checkRepoForNewSubmodules(repo)) changes = true; | ||
| } | ||
| } | ||
| this.processCreateEventsTimeout = null; | ||
|
|
@@ -298,7 +337,10 @@ export class RepoManager { | |
| let path, | ||
| changes = false; | ||
| while ((path = this.changeEventPaths.shift())) { | ||
| if (!(await doesPathExist(path))) { | ||
| if (path.endsWith("/.gitmodules")) { | ||
| const repo = path.slice(0, -"/.gitmodules".length); | ||
| if (await this.checkRepoForNewSubmodules(repo)) changes = true; | ||
| } else if (!(await doesPathExist(path))) { | ||
|
Comment on lines
326
to
+343
|
||
| if (this.removeReposWithinFolder(path)) changes = true; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import * as cp from "node:child_process"; | ||
| import * as fs from "node:fs"; | ||
| import * as os from "node:os"; | ||
| import * as path from "node:path"; | ||
|
|
||
| import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| vi.mock("vscode", () => ({ | ||
| workspace: { | ||
| getConfiguration: () => ({ | ||
| get: (_key: string, defaultValue: unknown) => defaultValue | ||
| }) | ||
| } | ||
| })); | ||
|
|
||
| import { DataSource } from "../../src/dataSource"; | ||
|
|
||
| function git(args: string[], cwd: string) { | ||
| cp.execFileSync("git", args, { cwd, stdio: "pipe" }); | ||
| } | ||
|
|
||
| function makeRepo(prefix: string): string { | ||
| const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); | ||
| try { | ||
| git(["init", "-b", "main"], dir); | ||
| } catch { | ||
| git(["init"], dir); | ||
| git(["checkout", "-b", "main"], dir); | ||
| } | ||
| git(["config", "user.email", "t@t.com"], dir); | ||
| git(["config", "user.name", "T"], dir); | ||
| fs.writeFileSync(path.join(dir, "f"), "x"); | ||
| git(["add", "."], dir); | ||
| git(["-c", "commit.gpgsign=false", "commit", "-m", "init"], dir); | ||
| return dir; | ||
| } | ||
|
|
||
| let parentRepo = ""; | ||
| let submoduleRepo = ""; | ||
| let standaloneRepo = ""; | ||
|
|
||
| beforeAll(() => { | ||
| submoduleRepo = makeRepo("ngg-submodule-"); | ||
| parentRepo = makeRepo("ngg-parent-"); | ||
| standaloneRepo = makeRepo("ngg-plain-"); | ||
|
|
||
| git( | ||
| [ | ||
| "-c", | ||
| "protocol.file.allow=always", | ||
| "submodule", | ||
| "add", | ||
| submoduleRepo, | ||
| "modules/child" | ||
| ], | ||
| parentRepo | ||
| ); | ||
| git(["-c", "commit.gpgsign=false", "commit", "-am", "add submodule"], parentRepo); | ||
| }); | ||
|
|
||
| afterAll(() => { | ||
| if (parentRepo !== "") fs.rmSync(parentRepo, { recursive: true, force: true }); | ||
| if (submoduleRepo !== "") fs.rmSync(submoduleRepo, { recursive: true, force: true }); | ||
| if (standaloneRepo !== "") fs.rmSync(standaloneRepo, { recursive: true, force: true }); | ||
| }); | ||
|
Comment on lines
+61
to
+65
|
||
|
|
||
| describe("getSubmodules", () => { | ||
| it("returns initialized submodules declared in .gitmodules", async () => { | ||
| const dataSource = new DataSource(); | ||
|
|
||
| const submodules = await dataSource.getSubmodules(parentRepo); | ||
|
|
||
| expect(submodules).toEqual([path.join(parentRepo, "modules/child").replace(/\\/g, "/")]); | ||
| }); | ||
|
Comment on lines
+67
to
+74
|
||
|
|
||
| it("returns an empty array when a repository has no .gitmodules file", async () => { | ||
| const dataSource = new DataSource(); | ||
|
|
||
| await expect(dataSource.getSubmodules(standaloneRepo)).resolves.toEqual([]); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Importing
node:pathaspathintroduces identifier shadowing with existing parameters namedpath(e.g.,isGitRepository(path: string)). Consider renaming those parameters (e.g.,repoPath/dir) to avoid confusion and make future use of thepathmodule inside those methods possible.