fix: windows file tree (#5364)

* test: reproduce issue for file tree cache on windows

* fix: make file tree work on windows

* fix: ensure that glob matches also work under windows

* fix: correctly transform paths into globs in GitRepoHandler

core/src/vcs/file-tree.ts

@@ -7,31 +7,36 @@
  */
 
 import type { VcsFile } from "./vcs.js"
-import * as path from "path"
+import type { PlatformPath } from "path"
+import * as posixPath from "path/posix"
+import * as winPath from "path/win32"
 
 type FileTreeNodeOptions = {
   ownPath: string
+  pathUtils: PlatformPath
 }
 
 export class FileTreeNode {
-  private ownPath: string
+  private pathUtils: PlatformPath
+  public ownPath: string
   private files: VcsFile[] = []
   private childrenBySubpath: Map<string, FileTreeNode> = new Map()
 
-  constructor({ ownPath }: FileTreeNodeOptions) {
+  constructor({ ownPath, pathUtils }: FileTreeNodeOptions) {
     this.ownPath = ownPath
+    this.pathUtils = pathUtils
   }
 
   addFile(file: VcsFile): boolean {
-    if (!file.path.startsWith(`/${this.ownPath}`)) {
+    if (!file.path.startsWith(this.ownPath)) {
       return false
     }
 
     this.files.push(file)
 
     const relativePath = file.path.slice(this.ownPath.length)
     // We use absolute paths so the first part of the split is always an empty string
-    const [_, subpathSegment, nextSegment] = relativePath.split(path.sep)
+    const [subpathSegment, nextSegment] = relativePath.split(this.pathUtils.sep)
 
     // We have reached the end of this path
     // and arrived at the file.
@@ -42,7 +47,10 @@ export class FileTreeNode {
 
     let child: FileTreeNode
     if (!this.childrenBySubpath.has(subpathSegment)) {
-      child = new FileTreeNode({ ownPath: path.join(this.ownPath, subpathSegment) })
+      child = new FileTreeNode({
+        ownPath: `${this.pathUtils.join(this.ownPath, subpathSegment)}${this.pathUtils.sep}`,
+        pathUtils: this.pathUtils,
+      })
       this.childrenBySubpath.set(subpathSegment, child)
     } else {
       child = this.childrenBySubpath.get(subpathSegment)!
@@ -64,14 +72,18 @@ export class FileTreeNode {
 
 export class FileTree {
   private root: FileTreeNode
+  private pathUtils: PlatformPath
 
-  constructor(root: FileTreeNode) {
+  constructor(root: FileTreeNode, pathUtils: PlatformPath) {
     this.root = root
+    this.pathUtils = pathUtils
   }
 
   private getNodeAtPath(filesPath: string): FileTreeNode | undefined {
-    // TODO: leading slash makes things annoying in many places
-    const segments = filesPath.slice(1).split(path.sep)
+    // Since we're always rooted at the same root directory, we remove that from the path we iterate down
+    // and start out iteration on the root node.
+    const rootPath = this.pathUtils.parse(filesPath).root
+    const segments = filesPath.slice(rootPath.length).split(this.pathUtils.sep)
 
     let currentNode: FileTreeNode | undefined = this.root
     while (currentNode) {
@@ -87,6 +99,10 @@ export class FileTree {
   }
 
   getFilesAtPath(filesPath: string): VcsFile[] {
+    if (this.root.ownPath !== this.pathUtils.parse(filesPath).root) {
+      return []
+    }
+
     const node = this.getNodeAtPath(filesPath)
 
     if (!node) {
@@ -104,17 +120,38 @@ export class FileTree {
     return node !== undefined
   }
 
-  static fromFiles(files: VcsFile[]): FileTree {
+  static fromFiles(files: VcsFile[], platform?: "win32" | "posix"): FileTree {
+    // In theory, node picks the right utils automatically depending on platform
+    // However, for testing, we need to be able to specify the platform explicitly here
+    // else we cannot test for example for Windows on a Unix machine.
+    const pathUtils = (platform ?? process.platform) === "win32" ? winPath : posixPath
+    if (files.length === 0) {
+      return new FileTree(
+        new FileTreeNode({
+          ownPath: "",
+          pathUtils,
+        }),
+        pathUtils
+      )
+    }
+
     // We assume all files are rooted from the same directory
     // which is the root filesystem directory for most (all?) use cases here.
+    // If we ever have a case where somehow we have files from different roots,
+    // we will need to handle that here.
+    // That probably won't ever be the case on Unix, but on Windows with its drive letter based system,
+    // it could happen at least theoretically.
+    // Practically, garden runs and scans from one directory, and things in that cannot be rooted on a different drive.
+    const rootPath = pathUtils.parse(files[0].path).root
     const node = new FileTreeNode({
-      ownPath: "",
+      ownPath: rootPath,
+      pathUtils,
     })
 
     for (const file of files) {
       node.addFile(file)
     }
 
-    return new FileTree(node)
+    return new FileTree(node, pathUtils)
   }
 }

core/src/vcs/git-repo.ts

@@ -6,14 +6,14 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { join } from "path"
 import { GitHandler, augmentGlobs } from "./git.js"
 import type { GetFilesParams, VcsFile } from "./vcs.js"
-import { isDirectory, joinWithPosix, matchPath } from "../util/fs.js"
+import { isDirectory, matchPath } from "../util/fs.js"
 import fsExtra from "fs-extra"
 const { pathExists } = fsExtra
 import { pathToCacheContext } from "../cache.js"
 import { FileTree } from "./file-tree.js"
+import { sep } from "path"
 
 type ScanRepoParams = Pick<GetFilesParams, "log" | "path" | "pathDescription" | "failOnPrompt">
 
@@ -58,11 +58,13 @@ export class GitRepoHandler extends GitHandler {
 
     const moduleFiles = fileTree.getFilesAtPath(path)
 
-    const include = params.include ? await absGlobs(path, params.include) : [path, join(path, "**", "*")]
-    const exclude = await absGlobs(path, params.exclude || [])
+    // We allow just passing a path like `foo` as include and exclude params
+    // Those need to be converted to globs, but we don't want to touch existing globs
+    const include = params.include ? await augmentGlobs(path, params.include) : ["**/*"]
+    const exclude = await augmentGlobs(path, params.exclude || [])
 
     if (scanRoot === this.garden?.projectRoot) {
-      exclude.push(join(scanRoot, ".garden", "**", "*"))
+      exclude.push("**/.garden/**/*")
     }
 
     log.debug(
@@ -71,7 +73,18 @@ export class GitRepoHandler extends GitHandler {
     log.silly(`Include globs: ${include.join(", ")}`)
     log.silly(exclude.length > 0 ? `Exclude globs: ${exclude.join(", ")}` : "No exclude globs")
 
-    const filtered = moduleFiles.filter(({ path: p }) => (!filter || filter(p)) && matchPath(p, include, exclude))
+    const filtered = moduleFiles.filter(({ path: p }) => {
+      if (filter && !filter(p)) {
+        return false
+      }
+      // We remove the subpath from the file path before matching
+      // so that the globs can be relative to the module path
+      // Previously we prepended the module path to the globs
+      // but that caused issues with the glob matching on windows due to backslashes
+      const relativePath = p.replace(`${path}${sep}`, "")
+      log.silly(`Checking if ${relativePath} matches include/exclude globs`)
+      return matchPath(relativePath, include, exclude)
+    })
 
     log.debug(`Found ${filtered.length} files in module path after glob matching`)
 
@@ -112,8 +125,3 @@ export class GitRepoHandler extends GitHandler {
     })
   }
 }
-
-async function absGlobs(basePath: string, globs: string[]): Promise<string[]> {
-  const augmented = await augmentGlobs(basePath, globs)
-  return augmented?.map((p) => joinWithPosix(basePath, p)) || []
-}

core/src/vcs/git.ts

@@ -700,7 +700,9 @@ const notInRepoRootErrorMessage = (path: string) => deline`
  * Given a list of POSIX-style globs/paths and a `basePath`, find paths that point to a directory and append `**\/*`
  * to them, such that they'll be matched consistently between git and our internal pattern matching.
  */
-export async function augmentGlobs(basePath: string, globs?: string[]) {
+export async function augmentGlobs(basePath: string, globs: string[]): Promise<string[]>
+export async function augmentGlobs(basePath: string, globs?: string[]): Promise<string[] | undefined>
+export async function augmentGlobs(basePath: string, globs?: string[]): Promise<string[] | undefined> {
   if (!globs || globs.length === 0) {
     return globs
   }

core/test/unit/src/vcs/file-tree.ts

@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 2018-2023 Garden Technologies, Inc. <info@garden.io>
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+import { expect } from "chai"
+import { FileTree } from "../../../../src/vcs/file-tree.js"
+
+describe("file-tree", () => {
+  describe("on unix", () => {
+    it("should get the files at a path", () => {
+      const fileTree = FileTree.fromFiles(
+        [
+          { path: "/Users/developer/code/garden-project/project-1/frontend", hash: "" },
+          { path: "/Users/developer/code/garden-project/project-1/backend", hash: "" },
+          { path: "/Users/developer/code/garden-project/project-2/frontend", hash: "" },
+          { path: "/Users/developer/code/garden-project/project-2/backend", hash: "" },
+        ],
+        "posix"
+      )
+
+      const filesAtProjectPath = fileTree.getFilesAtPath("/Users/developer/code/garden-project")
+
+      expect(filesAtProjectPath).to.eql([
+        { path: "/Users/developer/code/garden-project/project-1/frontend", hash: "" },
+        { path: "/Users/developer/code/garden-project/project-1/backend", hash: "" },
+        { path: "/Users/developer/code/garden-project/project-2/frontend", hash: "" },
+        { path: "/Users/developer/code/garden-project/project-2/backend", hash: "" },
+      ])
+
+      const filesAtFirstProjectPath = fileTree.getFilesAtPath("/Users/developer/code/garden-project/project-1")
+
+      expect(filesAtFirstProjectPath).to.eql([
+        { path: "/Users/developer/code/garden-project/project-1/frontend", hash: "" },
+        { path: "/Users/developer/code/garden-project/project-1/backend", hash: "" },
+      ])
+    })
+  })
+
+  describe("on windows", () => {
+    it("should get the files at a path", () => {
+      const fileTree = FileTree.fromFiles(
+        [
+          { path: "C:\\Users\\developer\\code\\garden-project\\project-1\\frontend", hash: "" },
+          { path: "C:\\Users\\developer\\code\\garden-project\\project-1\\backend", hash: "" },
+          { path: "C:\\Users\\developer\\code\\garden-project\\project-2\\frontend", hash: "" },
+          { path: "C:\\Users\\developer\\code\\garden-project\\project-2\\backend", hash: "" },
+        ],
+        "win32"
+      )
+
+      const filesAtProjectPath = fileTree.getFilesAtPath("C:\\Users\\developer\\code\\garden-project")
+
+      expect(filesAtProjectPath).to.eql([
+        { path: "C:\\Users\\developer\\code\\garden-project\\project-1\\frontend", hash: "" },
+        { path: "C:\\Users\\developer\\code\\garden-project\\project-1\\backend", hash: "" },
+        { path: "C:\\Users\\developer\\code\\garden-project\\project-2\\frontend", hash: "" },
+        { path: "C:\\Users\\developer\\code\\garden-project\\project-2\\backend", hash: "" },
+      ])
+
+      const filesAtFirstProjectPath = fileTree.getFilesAtPath("C:\\Users\\developer\\code\\garden-project\\project-1")
+
+      expect(filesAtFirstProjectPath).to.eql([
+        { path: "C:\\Users\\developer\\code\\garden-project\\project-1\\frontend", hash: "" },
+        { path: "C:\\Users\\developer\\code\\garden-project\\project-1\\backend", hash: "" },
+      ])
+    })
+  })
+})