refactor(git): extract template method for files retrieval

To avoid duplicate sanity checks and other repeatable code.

core/src/vcs/git-repo.ts

@@ -8,9 +8,8 @@
 
 import { GitHandler, augmentGlobs } from "./git.js"
 import type { GetFilesParams, VcsFile } from "./vcs.js"
-import { isDirectory, matchPath } from "../util/fs.js"
-import fsExtra from "fs-extra"
-const { pathExists } = fsExtra
+import { matchPath } from "../util/fs.js"
+
 import { pathToCacheContext } from "../cache.js"
 import { FileTree } from "./file-tree.js"
 import { sep } from "path"
@@ -25,24 +24,9 @@ export class GitRepoHandler extends GitHandler {
    * 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.
    */
-  override async getFiles(params: GetFilesParams): Promise<VcsFile[]> {
+  override async findFiles(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 []
-    }
-
-    if (!(await pathExists(path))) {
-      log.warn(`${pathDescription} ${path} could not be found.`)
-      return []
-    }
-
-    if (!(await isDirectory(path))) {
-      log.warn(`Path ${path} is not a directory.`)
-      return []
-    }
-
     let scanRoot = params.scanRoot || path
 
     if (!params.scanRoot && params.pathDescription !== "submodule") {
@@ -115,7 +99,7 @@ export class GitRepoHandler extends GitHandler {
       }
 
       log.silly(() => `Scanning repository at ${path}`)
-      const files = await super.getFiles({ ...params, scanRoot: undefined })
+      const files = await super.findFiles({ ...params, scanRoot: undefined })
 
       const fileTree = FileTree.fromFiles(files)
 

core/src/vcs/git.ts

@@ -143,16 +143,16 @@ export class GitHandler extends VcsHandler {
    * Returns a list of files, along with file hashes, under the given path, taking into account the configured
    * .ignore files, and the specified include/exclude filters.
    */
-  override async getFiles(params: GetFilesParams): Promise<VcsFile[]> {
-    return this._getFiles(params)
+  override async findFiles(params: GetFilesParams): Promise<VcsFile[]> {
+    return this._findFiles(params)
   }
 
   /**
    * In order for {@link GitRepoHandler} not to enter infinite recursion when scanning submodules,
-   * we need to name the function that recurses in here differently from {@link getFiles}
-   * so that {@link getFiles} won't refer to the method in the subclass.
+   * we need to name the function that recurses in here differently from {@link findFiles}
+   * so that {@link findFiles} won't refer to the method in the subclass.
    */
-  async _getFiles({
+  async _findFiles({
     log,
     path,
     pathDescription = "directory",
@@ -180,6 +180,7 @@ export class GitHandler extends VcsHandler {
         }`
       )
 
+    // This code is required to handle submodules
     try {
       const pathStats = await stat(path)
 
@@ -196,8 +197,6 @@ export class GitHandler extends VcsHandler {
       }
     }
 
-    let files: VcsFile[] = []
-
     const git = this.gitCli(gitLog, path, failOnPrompt)
     const gitRoot = await this.getRepoRoot(gitLog, path, failOnPrompt)
 
@@ -292,7 +291,7 @@ export class GitHandler extends VcsHandler {
           }
         }
 
-        return this._getFiles({
+        return this._findFiles({
           log: gitLog,
           path: submodulePath,
           pathDescription: "submodule",
@@ -307,6 +306,7 @@ export class GitHandler extends VcsHandler {
 
     // Make sure we have a fresh hash for each file
     let count = 0
+    let files: VcsFile[] = []
 
     const ensureHash = async (file: VcsFile, stats: fsExtra.Stats | undefined): Promise<void> => {
       if (file.hash === "" || modified.has(file.path)) {

core/src/vcs/vcs.ts

@@ -12,14 +12,14 @@ import { relative, sep } from "path"
 import { DOCS_BASE_URL } from "../constants.js"
 import fsExtra from "fs-extra"
 
-const { writeFile } = fsExtra
+const { pathExists, writeFile } = fsExtra
 import type { ExternalSourceType } from "../util/ext-source-util.js"
 import { getRemoteSourceLocalPath, getRemoteSourcesPath } from "../util/ext-source-util.js"
 import type { ModuleConfig } from "../config/module.js"
 import { serializeConfig } from "../config/module.js"
 import type { Log } from "../logger/log-entry.js"
 import { dedent, splitLast } from "../util/string.js"
-import { fixedProjectExcludes } from "../util/fs.js"
+import { fixedProjectExcludes, isDirectory } from "../util/fs.js"
 import type { TreeCache } from "../cache.js"
 import { pathToCacheContext } from "../cache.js"
 import type { ServiceConfig } from "../config/service.js"
@@ -34,6 +34,7 @@ import type { Garden } from "../garden.js"
 import { Profile } from "../util/profiling.js"
 
 import AsyncLock from "async-lock"
+
 const scanLock = new AsyncLock()
 
 export const versionStringPrefix = "v-"
@@ -169,10 +170,34 @@ export abstract class VcsHandler {
 
   abstract getRepoRoot(log: Log, path: string): Promise<string>
 
+  /**
+   * Template method for finding files in different Git repo scanning modes.
+   */
+  async getFiles(params: GetFilesParams): Promise<VcsFile[]> {
+    const { log, path, pathDescription } = params
+
+    if (params.include && params.include.length === 0) {
+      // No need to proceed, nothing should be included
+      return []
+    }
+
+    if (!(await pathExists(path))) {
+      log.warn(`${pathDescription} ${path} could not be found.`)
+      return []
+    }
+
+    if (!(await isDirectory(path))) {
+      log.warn(`Path ${path} is not a directory.`)
+      return []
+    }
+
+    return this.findFiles(params)
+  }
+
   /**
    * Abstract method for finding files in concrete Git repo scanning modes.
    */
-  abstract getFiles(params: GetFilesParams): Promise<VcsFile[]>
+  abstract findFiles(params: GetFilesParams): Promise<VcsFile[]>
 
   abstract ensureRemoteSource(params: RemoteSourceParams): Promise<string>
 

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

@@ -54,6 +54,10 @@ export class TestVcsHandler extends VcsHandler {
     return []
   }
 
+  override async findFiles(_: GetFilesParams): Promise<VcsFile[]> {
+    return []
+  }
+
   async getPathInfo() {
     return {
       branch: "main",