Skip to content

Commit

Permalink
fix(git): fix exclude filter in repo Git scan mode
Browse files Browse the repository at this point in the history
Align `exclude` filter behavior between `subtree` and `repo` Git repository scan modes.
Now both `GitHandler` and `GitRepoHandler` behave identically if any exclusions are configured.

chore: amend
  • Loading branch information
vvagaytsev committed Dec 19, 2023
1 parent 0587044 commit c15d394
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 23 deletions.
13 changes: 12 additions & 1 deletion core/src/vcs/git-repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { normalize, sep } from "path"

const { pathExists } = fsExtra

type ScanRepoParams = Pick<GetFilesParams, "log" | "path" | "pathDescription" | "failOnPrompt">
type ScanRepoParams = Pick<GetFilesParams, "log" | "path" | "pathDescription" | "failOnPrompt" | "exclude">

interface GitRepoGetFilesParams extends GetFilesParams {
scanFromProjectRoot: boolean
Expand Down Expand Up @@ -88,6 +88,15 @@ export class GitRepoHandler extends GitHandler {
path: scanRoot,
pathDescription: pathDescription || "repository",
failOnPrompt,
// This method delegates to the old "subtree" Git scan mode that use `--exclude` and `--glob-pathspecs`.
// We need to pass the exclude-filter to ensure that all excluded files and dirs are excluded properly,
// i.e. with the properly expanded globs.
// Otherwise, the "repo" Git scan mode may return some excluded files
// which should be skipped by design and are skipped by the "subtree" mode.
// Thus, some excluded files can appear in the resulting fileTree.
// It happens because the new "repo" mode does not use `--glob-pathspecs` flag
// and does some explicit glob patterns augmentation that misses some edge-cases.
exclude: params.exclude,
})

const moduleFiles = fileTree.getFilesAtPath(path)
Expand Down Expand Up @@ -124,6 +133,8 @@ export class GitRepoHandler extends GitHandler {
/**
* Scans the given repo root and caches the list of files in the tree cache.
* Uses an async lock to ensure a repo root is only scanned once.
*
* Delegates to {@link GitHandler.getFiles}.
*/
async scanRepo(params: ScanRepoParams): Promise<FileTree> {
const { log, path } = params
Expand Down
25 changes: 3 additions & 22 deletions core/test/unit/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
},
{
name: "when directory is included by name with globs",
// FIXME: shouldn't just '**/deepdir' work well too?
// FIXME-GITREPOHANDLER: shouldn't just '**/deepdir' work well too?
inclusionBuilder: (_subDirName: string, deepDirName: string) => join("**", deepDirName, "**", "*"),
},
]
Expand Down Expand Up @@ -452,13 +452,6 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {

for (const testParam of testParams) {
it(testParam.name, async () => {
// FIXME
// if (handler.name === "git-repo") {
// if (testParam.name === "without globs") {
// return
// }
// }

// matches file exclusion pattern -> should be excluded
const excludedByFilename = resolve(tmpPath, "foo.txt")
await createFile(excludedByFilename)
Expand Down Expand Up @@ -529,13 +522,6 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
*/
for (const testParam of testParams) {
it(testParam.name, async () => {
// FIXME
// if (handler.name === "git-repo") {
// if (testParam.name === "with prefix globs") {
// return
// }
// }

// doesn't match file exclusion pattern -> should be included
const notExcludedByFilename = resolve(tmpPath, "bar.txt")

Expand Down Expand Up @@ -607,13 +593,6 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
*/
for (const testParam of testParams) {
it(testParam.name, async () => {
// FIXME
// if (handler.name === "git-repo") {
// if (testParam.name === "without globs" || testParam.name === "with prefix globs") {
// return
// }
// }

// doesn't match file exclusion pattern -> should be included
const notExcludedByFilename = resolve(tmpPath, "bar.txt")

Expand Down Expand Up @@ -1392,6 +1371,8 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
})
}

// FIXME-GITREPOHANDLER: revisit these tests and disk-based configs,
// inspect the scenarios when both include and exclude filters are defined.s
const getTreeVersionTests = (gitScanMode: GitScanMode) => {
const gitHandlerCls = getGitHandlerCls(gitScanMode)
describe("getTreeVersion", () => {
Expand Down

0 comments on commit c15d394

Please sign in to comment.