Skip to content

Commit

Permalink
fix(git): fix file list caching bug for repo mode (#5710)
Browse files Browse the repository at this point in the history
This fixes an error that would come up e.g. when an action config in the
project root excluded a subdirectory containing another action config.

In that setup, the file list for the action in the subdirectory would be
empty because the file list had been cached using the exclude config
applied when computing the file list for the action config at the root
level (which included no files for the excluded subdirectory).

This was fixed by adding another level of caching to `GitRepoHandler`
that accounts for different includes/excludes and filter functions for
the same path.
  • Loading branch information
thsig committed Feb 7, 2024
1 parent 9e64163 commit ca7f997
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 79 deletions.
53 changes: 31 additions & 22 deletions core/src/vcs/git-repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import fsExtra from "fs-extra"
import { pathToCacheContext } from "../cache.js"
import { FileTree } from "./file-tree.js"
import { normalize, sep } from "path"
import { stableStringify } from "../util/string.js"
import { hashString } from "../util/util.js"

const { pathExists } = fsExtra

Expand Down Expand Up @@ -62,10 +64,7 @@ export class GitRepoHandler extends GitHandler {
override async getFiles(params: GetFilesParams): Promise<VcsFile[]> {
const { log, path, pathDescription, filter, failOnPrompt = false } = params

if (params.include && params.include.length === 0) {
// No need to proceed, nothing should be included
return []
}
let scanRoot = params.scanRoot || path

if (!(await pathExists(path))) {
log.warn(`${pathDescription} ${path} could not be found.`)
Expand All @@ -77,42 +76,50 @@ export class GitRepoHandler extends GitHandler {
return []
}

let scanRoot = params.scanRoot || path

if (!params.scanRoot && params.pathDescription !== "submodule") {
scanRoot = await this.getRepoRoot(log, path, failOnPrompt)
}

const scanFromProjectRoot = scanRoot === this.garden?.projectRoot
const { augmentedExcludes, augmentedIncludes } = await getIncludeExcludeFiles({ ...params, scanFromProjectRoot })

const hashedFilterParams = hashString(
stableStringify({
filter: filter ? filter.toString() : undefined, // We hash the source code of the filter function if provided.
augmentedIncludes,
augmentedExcludes,
})
)
const filteredFilesCacheKey = ["git-repo-files", path, hashedFilterParams]

const cached = this.cache.get(log, filteredFilesCacheKey) as VcsFile[] | undefined
if (cached) {
return cached
}

if (params.include && params.include.length === 0) {
// No need to proceed, nothing should be included
return []
}

const fileTree = await this.scanRepo({
log,
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)

const scanFromProjectRoot = scanRoot === this.garden?.projectRoot
const { augmentedExcludes, augmentedIncludes } = await getIncludeExcludeFiles({ ...params, scanFromProjectRoot })
const filesAtPath = fileTree.getFilesAtPath(path)

log.debug(
`Found ${moduleFiles.length} files in module path, filtering by ${augmentedIncludes.length} include and ${augmentedExcludes.length} exclude globs`
`Found ${filesAtPath.length} files in path, filtering by ${augmentedIncludes.length} include and ${augmentedExcludes.length} exclude globs`
)
log.silly(() => `Include globs: ${augmentedIncludes.join(", ")}`)
log.silly(() =>
augmentedExcludes.length > 0 ? `Exclude globs: ${augmentedExcludes.join(", ")}` : "No exclude globs"
)

const filtered = moduleFiles.filter(({ path: p }) => {
const filtered = filesAtPath.filter(({ path: p }) => {
if (filter && !filter(p)) {
return false
}
Expand All @@ -127,6 +134,8 @@ export class GitRepoHandler extends GitHandler {

log.debug(`Found ${filtered.length} files in module path after glob matching`)

this.cache.set(log, filteredFilesCacheKey, filtered, pathToCacheContext(path))

return filtered
}

Expand All @@ -140,7 +149,7 @@ export class GitRepoHandler extends GitHandler {
const { log, path } = params

const key = ["git-repo-files", path]
let existing = this.cache.get(log, key) as FileTree
let existing = this.cache.get(log, key) as FileTree | undefined

if (existing) {
params.log.silly(() => `Found cached repository match at ${path}`)
Expand Down
6 changes: 3 additions & 3 deletions core/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,12 @@ export class GitHandler extends VcsHandler {
}

const { log, path, pathDescription = "directory", filter, failOnPrompt = false } = params
const { exclude, hasIncludes, include } = await getIncludeExcludeFiles(params)

const gitLog = log
.createLog({ name: "git" })
.debug(
`Scanning ${pathDescription} at ${path}\n → Includes: ${include || "(none)"}\n → Excludes: ${
exclude || "(none)"
`Scanning ${pathDescription} at ${path}\n → Includes: ${params.include || "(none)"}\n → Excludes: ${
params.exclude || "(none)"
}`
)

Expand All @@ -225,6 +224,7 @@ export class GitHandler extends VcsHandler {
throw err
}
}
const { exclude, hasIncludes, include } = await getIncludeExcludeFiles(params)

let files: VcsFile[] = []

Expand Down

0 comments on commit ca7f997

Please sign in to comment.