fix(git): fix exclude filter in repo Git scan mode

core/src/vcs/git-repo.ts

@@ -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
@@ -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)
@@ -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

core/src/vcs/git.ts

@@ -367,9 +367,11 @@ export class GitHandler extends VcsHandler {
         return
       }
 
-      const passesExclusionFilter = matchPath(filePath, undefined, exclude)
-      if (hasIncludes && !passesExclusionFilter) {
-        return
+      if (hasIncludes) {
+        const passesExclusionFilter = matchPath(filePath, undefined, exclude)
+        if (!passesExclusionFilter) {
+          return
+        }
       }
 
       // We push to the output array if it passes through the exclude filters.

core/test/unit/src/plugins/container/container.ts

@@ -40,9 +40,9 @@ import {
   DEFAULT_TEST_TIMEOUT_SEC,
   GardenApiVersion,
 } from "../../../../../src/constants.js"
-import { resolve } from "path"
+import { join, resolve } from "path"
 import type { ConvertModuleParams } from "../../../../../src/plugin/handlers/Module/convert.js"
-import { omit } from "lodash-es"
+import { omit, remove } from "lodash-es"
 import type { GardenTask } from "../../../../../src/types/task.js"
 import { taskFromConfig } from "../../../../../src/types/task.js"
 import type { GardenService } from "../../../../../src/types/service.js"
@@ -151,7 +151,11 @@ describe("plugins.container", () => {
 
     it("returns the dummy Build action if no Dockerfile and an exec Build is needed", async () => {
       const module = graph.getModule("module-a") as ContainerModule
-      module.version.files.pop() // remove automatically picked up Dockerfile
+
+      // remove automatically picked up Dockerfile
+      const defaultDockerfilePath = join(module.path, defaultDockerfileName)
+      remove(module.version.files, (f) => f === defaultDockerfilePath)
+
       const dummyBuild: ExecBuildConfig = {
         internal: {
           basePath: ".",

core/test/unit/src/vcs/git.ts

@@ -362,16 +362,17 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
                 const testParams = [
                   {
                     name: "when directory is included by exact relative path",
-                    pathBuilder: (subDirName: string, deepDirName) => join(subDirName, deepDirName),
+                    inclusionBuilder: (subDirName: string, deepDirName: string) => join(subDirName, deepDirName),
                   },
                   {
                     name: "when directory is included by relative path with globs",
-                    pathBuilder: (subDirName: string, deepDirName) => join(subDirName, deepDirName, "**", "*"),
+                    inclusionBuilder: (subDirName: string, deepDirName: string) =>
+                      join(subDirName, deepDirName, "**", "*"),
                   },
                   {
                     name: "when directory is included by name with globs",
-                    // FIXME: shouldn't just '**/deepdir' work well too?
-                    pathBuilder: (_subDirName: string, deepDirName) => join("**", deepDirName, "**", "*"),
+                    // FIXME-GITREPOHANDLER: shouldn't just '**/deepdir' work well too?
+                    inclusionBuilder: (_subDirName: string, deepDirName: string) => join("**", deepDirName, "**", "*"),
                   },
                 ]
 
@@ -386,7 +387,7 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
                     const path = resolve(deepDir, "foo.txt")
                     await createFile(path)
 
-                    const include = [testParam.pathBuilder(subdirName, deepDirName)]
+                    const include = [testParam.inclusionBuilder(subdirName, deepDirName)]
                     const files = (
                       await handler.getFiles({
                         path: tmpPath,
@@ -427,26 +428,30 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
       // the exclusion paths with and without glob prefix **/ works in the same way.
       context("when only exclude filter is specified", () => {
         context("should filter out files that match the exclude filter", () => {
+          const excludedFilenameNoExt = "foo"
+          const excludedFilenameTxt = `${excludedFilenameNoExt}.txt`
+          const excludedFilenameWildcard = `${excludedFilenameNoExt}.*`
           const testParams = [
             {
-              name: "without globs",
-              pathBuilder: (path: string) => path,
+              name: "by exact filename without globs",
+              exclusionBuilder: () => excludedFilenameTxt,
             },
             {
-              name: "with globs",
-              pathBuilder: (path: string) => join("**", path),
+              name: "by exact filename with prefix globs",
+              exclusionBuilder: () => join("**", excludedFilenameTxt),
+            },
+            {
+              name: "by filename with wildcard extension without prefix globs",
+              exclusionBuilder: () => excludedFilenameWildcard,
+            },
+            {
+              name: "by filename with wildcard extension with prefix globs",
+              exclusionBuilder: () => join("**", excludedFilenameWildcard),
             },
           ]
 
           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)
@@ -472,7 +477,7 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
                   path: tmpPath,
                   scanRoot: undefined,
                   include: undefined,
-                  exclude: [testParam.pathBuilder("foo.*")],
+                  exclude: [testParam.exclusionBuilder()],
                   log,
                 })
               )
@@ -490,19 +495,19 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
             const testParams = [
               {
                 name: "without globs",
-                pathBuilder: (subDirName: string) => subDirName,
+                exclusionBuilder: (subDirName: string) => subDirName,
               },
               {
                 name: "with prefix globs",
-                pathBuilder: (subDirName: string) => join("**", subDirName),
+                exclusionBuilder: (subDirName: string) => join("**", subDirName),
               },
               {
                 name: "with full globs",
-                pathBuilder: (subDirName: string) => join("**", subDirName, "**", "*"),
+                exclusionBuilder: (subDirName: string) => join("**", subDirName, "**", "*"),
               },
               {
                 name: "with redundant relative path",
-                pathBuilder: (subDirName: string) => `./${subDirName}`,
+                exclusionBuilder: (subDirName: string) => `./${subDirName}`,
               },
             ]
 
@@ -517,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")
 
@@ -551,7 +549,7 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
                     path: tmpPath,
                     scanRoot: undefined,
                     include: undefined, // when include: [], getFiles() always returns an empty result
-                    exclude: [testParam.pathBuilder(excludedDirName)],
+                    exclude: [testParam.exclusionBuilder(excludedDirName)],
                     log,
                   })
                 )
@@ -568,19 +566,19 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
             const testParams = [
               {
                 name: "without globs",
-                pathBuilder: (...subDirNames: string[]) => subDirNames.at(-1)!,
+                exclusionBuilder: (...subDirNames: string[]) => subDirNames.at(-1)!,
               },
               {
                 name: "with prefix globs",
-                pathBuilder: (...subDirNames: string[]) => join("**", subDirNames.at(-1)!),
+                exclusionBuilder: (...subDirNames: string[]) => join("**", subDirNames.at(-1)!),
               },
               {
                 name: "with full globs",
-                pathBuilder: (...subDirNames: string[]) => join("**", subDirNames.at(-1)!, "**", "*"),
+                exclusionBuilder: (...subDirNames: string[]) => join("**", subDirNames.at(-1)!, "**", "*"),
               },
               {
                 name: "with redundant relative path",
-                pathBuilder: (...subDirNames: string[]) => `./${subDirNames.join("/")}`,
+                exclusionBuilder: (...subDirNames: string[]) => `./${subDirNames.join("/")}`,
               },
             ]
 
@@ -595,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")
 
@@ -629,7 +620,7 @@ const commonGitHandlerTests = (gitScanMode: GitScanMode) => {
                     path: tmpPath,
                     scanRoot: undefined,
                     include: undefined, // when include: [], getFiles() always returns an empty result
-                    exclude: [testParam.pathBuilder(notExcludedDirName, excludedSubDirectoryName)],
+                    exclude: [testParam.exclusionBuilder(notExcludedDirName, excludedSubDirectoryName)],
                     log,
                   })
                 )
@@ -1380,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", () => {