diff --git a/core/src/vcs/git-repo.ts b/core/src/vcs/git-repo.ts index e3ffe9d3bf..d497d27e8b 100644 --- a/core/src/vcs/git-repo.ts +++ b/core/src/vcs/git-repo.ts @@ -22,10 +22,11 @@ import { FileTree } from "./file-tree.js" import { normalize, sep } from "path" import { stableStringify } from "../util/string.js" import { hashString } from "../util/util.js" +import { Profile } from "../util/profiling.js" const { pathExists } = fsExtra -type ScanRepoParams = Pick +type ScanRepoParams = Pick interface GitRepoGetFilesParams extends GetFilesParams { scanFromProjectRoot: boolean @@ -44,7 +45,7 @@ const getIncludeExcludeFiles: IncludeExcludeFilesHandler boolean) | undefined + augmentedIncludes: string[] + augmentedExcludes: string[] +}) { + return hashString( + stableStringify({ + filter: filter ? filter.toString() : undefined, // We hash the source code of the filter function if provided. + augmentedIncludes: augmentedIncludes.sort(), + augmentedExcludes: augmentedExcludes.sort(), + }) + ) +} + +@Profile() export class GitRepoHandler extends AbstractGitHandler { private readonly gitHandlerDelegate: GitSubTreeHandler override readonly name = "git-repo" @@ -71,7 +90,7 @@ export class GitRepoHandler extends AbstractGitHandler { } /** - * This has the same signature as the GitHandler super class method but instead of scanning the individual directory + * This has the same signature as the `GitSubTreeHandler` class method but instead of scanning the individual directory * path directly, we scan the entire enclosing git repository, cache that file list and then filter down to the * sub-path. This results in far fewer git process calls but in turn collects more data in memory. */ @@ -97,13 +116,11 @@ export class GitRepoHandler extends AbstractGitHandler { const scanFromProjectRoot = scanRoot === this.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 hashedFilterParams = getHashedFilterParams({ + filter, + augmentedIncludes, + augmentedExcludes, + }) const filteredFilesCacheKey = ["git-repo-files", path, hashedFilterParams] const cached = this.cache.get(log, filteredFilesCacheKey) as VcsFile[] | undefined diff --git a/core/src/vcs/vcs.ts b/core/src/vcs/vcs.ts index 45bd9c1913..1968efa28c 100644 --- a/core/src/vcs/vcs.ts +++ b/core/src/vcs/vcs.ts @@ -470,7 +470,7 @@ export function describeConfig(config: ModuleConfig | BaseActionConfig): ActionD * Checks if the {@code subPathCandidate} is a sub-path of {@code basePath}. * Sub-path means that a candidate must be located inside a reference path. * - * Both {@basePath} and {@ subPathCandidate} must be absolute paths + * Both {@code basePath} and {@code subPathCandidate} must be absolute paths * * @param basePath the reference path (absolute) * @param subPathCandidate the path to be checked (absolute) diff --git a/core/test/unit/src/vcs/vcs.ts b/core/test/unit/src/vcs/vcs.ts index cca0a08cfd..faab67d1a2 100644 --- a/core/test/unit/src/vcs/vcs.ts +++ b/core/test/unit/src/vcs/vcs.ts @@ -41,6 +41,7 @@ import { defaultDotIgnoreFile, fixedProjectExcludes } from "../../../../src/util import { createActionLog } from "../../../../src/logger/log-entry.js" import type { BaseActionConfig } from "../../../../src/actions/types.js" import { TreeCache } from "../../../../src/cache.js" +import { getHashedFilterParams } from "../../../../src/vcs/git-repo.js" export class TestVcsHandler extends VcsHandler { override readonly name = "test" @@ -664,4 +665,44 @@ describe("helpers", () => { expect(subPath).to.be.true }) }) + + describe("getHashedFilterParams", () => { + it("should return the same hashes for fully equal objects", () => { + const params1 = { filter: undefined, augmentedIncludes: ["yes.txt"], augmentedExcludes: ["no.txt"] } + const hash1 = getHashedFilterParams(params1) + + const params2 = { filter: undefined, augmentedIncludes: ["yes.txt"], augmentedExcludes: ["no.txt"] } + const hash2 = getHashedFilterParams(params2) + + expect(hash1).to.eql(hash2) + }) + + it("should return the different hashes for non-equal objects", () => { + const params1 = { filter: undefined, augmentedIncludes: ["yes1.txt"], augmentedExcludes: ["no1.txt"] } + const hash1 = getHashedFilterParams(params1) + + const params2 = { filter: undefined, augmentedIncludes: ["yes2.txt"], augmentedExcludes: ["no2.txt"] } + const hash2 = getHashedFilterParams(params2) + + expect(hash1).not.to.eql(hash2) + }) + + it("should not depend on the order of the include/exclude file lists", () => { + const params1 = { + filter: undefined, + augmentedIncludes: ["yes1.txt", "yes2.txt"], + augmentedExcludes: ["no1.txt", "no2.txt"], + } + const hash1 = getHashedFilterParams(params1) + + const params2 = { + filter: undefined, + augmentedIncludes: ["yes2.txt", "yes1.txt"], + augmentedExcludes: ["no2.txt", "no1.txt"], + } + const hash2 = getHashedFilterParams(params2) + + expect(hash1).to.eql(hash2) + }) + }) })