Skip to content

Commit

Permalink
perf(git): optimize git scan when exclude but no include filter is set
Browse files Browse the repository at this point in the history
This can be a major optimization in certain cases.

The reason this wasn't done previously was due to a git ls-files issue
but we missed out on this path when no includes are used, in which case
the issue doesn't come up.

Usually includes are much more specific than excludes in configs, so this
approach should cover a lot of user scenarios.

It is still more optimal to use `.gardenignore` files over exclude patterns
where possible, but using excludes in configs is obviously  a common and
supported use case, and not always possible to use the ignore files (say,
if many actions/modules share a directory and need different exclusions).
  • Loading branch information
edvald authored and vvagaytsev committed Jul 20, 2023
1 parent 45a5526 commit 7361fc9
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 9 deletions.
43 changes: 35 additions & 8 deletions core/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,17 +300,40 @@ export class GitHandler extends VcsHandler {

const absExcludes = exclude.map((p) => resolve(path, p))

// Apply the include patterns to the ls-files queries. We use the 'glob' "magic word" (in git parlance)
// Apply the include patterns to the ls-files queries. We use the --glob-pathspecs flag
// to make sure the path handling is consistent with normal POSIX-style globs used generally by Garden.
// Note: We unfortunately can't exclude at this level because it simply doesn't work in git, for reasons unknown.
const patterns = [...(include || []).map((p) => ":(glob)" + p)]

// Due to an issue in git, we can unfortunately only use _either_ include or exclude patterns in the
// ls-files commands, but not both. Trying both just ignores the exclude patterns.

if (include?.includes("**/*")) {
// This is redundant
include = undefined
}

const hasIncludes = !!include?.length

const globalArgs = ["--glob-pathspecs"]
const lsFilesCommonArgs = ["--cached", "--exclude", this.gardenDirPath]

if (!hasIncludes) {
for (const p of exclude) {
lsFilesCommonArgs.push("--exclude", p)
}
}

// List tracked but ignored files (we currently exclude those as well, so we need to query that specially)
const trackedButIgnored = new Set(
!this.ignoreFile
? []
: await git("ls-files", "--ignored", ...lsFilesCommonArgs, "--exclude-per-directory", this.ignoreFile)
: await git(
...globalArgs,
"ls-files",
"--ignored",
...lsFilesCommonArgs,
"--exclude-per-directory",
this.ignoreFile
)
)

// List all submodule paths in the current path
Expand Down Expand Up @@ -366,8 +389,12 @@ export class GitHandler extends VcsHandler {

const resolvedPath = resolve(path, filePath)

// Filter on excludes
if (!matchPath(filePath, undefined, exclude) || submodulePaths.includes(resolvedPath)) {
// Filter on excludes and submodules
if (submodulePaths.includes(resolvedPath)) {
return
}

if (hasIncludes && !matchPath(filePath, undefined, exclude)) {
return
}

Expand Down Expand Up @@ -428,11 +455,11 @@ export class GitHandler extends VcsHandler {

await new Promise<void>((_resolve, _reject) => {
// Prepare args
const args = ["ls-files", "-s", "--others", ...lsFilesCommonArgs]
const args = [...globalArgs, "ls-files", "-s", "--others", ...lsFilesCommonArgs]
if (this.ignoreFile) {
args.push("--exclude-per-directory", this.ignoreFile)
}
args.push(...patterns)
args.push(...(include || []))

// Start git process
gitLog.silly(`Calling git with args '${args.join(" ")}' in ${path}`)
Expand Down
2 changes: 1 addition & 1 deletion core/test/unit/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ export const commonGitHandlerTests = (handlerCls: new (params: VcsHandlerParams)
).to.eql([])
})

it("should respect include and exclude patterns, if both are specified", async () => {
it("should respect include and exclude filters, if both are specified", async () => {
const moduleDir = resolve(tmpPath, "module-a")
const pathA = resolve(moduleDir, "yes.txt")
const pathB = resolve(tmpPath, "no.txt")
Expand Down

0 comments on commit 7361fc9

Please sign in to comment.