fix(diff): some false positives in diff command (#7806)

* fix(diff): error on disabled actions

* fix(diff): some false positives in diff command

---------

Co-authored-by: Jon Edvald <edvald@gmail.com>

Author: edvald-garden

core/src/commands/diff.ts

@@ -420,7 +420,7 @@ async function actionDiffPreliminary({
   node: RenderedNode
   resolveAction: boolean
 }): Promise<ActionDiffPreliminary> {
-  const actionA = graphA.getActionByRef({ kind: node.kind, name: node.name })
+  const actionA = graphA.getActionByRef({ kind: node.kind, name: node.name }, { includeDisabled: true })
 
   log = actionA.createLog(log)
   log.debug({ msg: "Comparison (phase 1)" })
@@ -573,7 +573,7 @@ export async function actionDiffFinal(
     }
   }
 
-  const actionA = graphA.getActionByRef({ kind: diff.kind, name: diff.name })
+  const actionA = graphA.getActionByRef({ kind: diff.kind, name: diff.name }, { includeDisabled: true })
   // const actionB = graphB.getActionByRef({ kind: diffB.kind, name: diffB.name })
 
   const status = diff.status
@@ -774,27 +774,29 @@ async function computeFiles({
 
   if (actionA) {
     const sourcePath = actionA.sourcePath()
-    filesA = await gardenA.vcs.getFiles({
+    const treeVersionWithHashes = await gardenA.vcs.getTreeVersionWithHashes({
       log,
-      path: sourcePath,
+      projectName: gardenA.projectName,
+      config: actionA.getConfig(),
       // FIXME: We should use the minimal roots, as when resolving the graph
       scanRoot: sourcePath,
     })
-    filesA = filesA.map((file) => ({
+    filesA = treeVersionWithHashes.files.map((file) => ({
       ...file,
       path: relative(sourcePath, file.path),
     }))
   }
 
   if (actionB) {
     const sourcePath = actionB.sourcePath()
-    filesB = await gardenB.vcs.getFiles({
+    const treeVersionWithHashes = await gardenB.vcs.getTreeVersionWithHashes({
       log,
-      path: sourcePath,
+      projectName: gardenB.projectName,
+      config: actionB.getConfig(),
       // FIXME: We should use the minimal roots, as when resolving the graph
       scanRoot: sourcePath,
     })
-    filesB = filesB.map((file) => ({
+    filesB = treeVersionWithHashes.files.map((file) => ({
       ...file,
       path: relative(sourcePath, file.path),
     }))

core/src/vcs/vcs.ts

@@ -70,6 +70,11 @@ export interface TreeVersion {
   files: string[]
 }
 
+export interface TreeVersionWithHashes {
+  contentHash: string
+  files: VcsFile[]
+}
+
 export interface TreeVersions {
   [moduleName: string]: TreeVersion
 }
@@ -208,13 +213,13 @@ export abstract class VcsHandler {
     this.cache.clear()
   }
 
-  async getTreeVersion({
+  async getTreeVersionWithHashes({
     log,
     projectName,
     config,
     force = false,
     scanRoot,
-  }: GetTreeVersionParams): Promise<TreeVersion> {
+  }: GetTreeVersionParams): Promise<TreeVersionWithHashes> {
     const cacheKey = getResourceTreeCacheKey(config)
     const description = describeConfig(config)
 
@@ -231,7 +236,7 @@ export abstract class VcsHandler {
     const configPath = getConfigFilePath(config)
     const path = getSourcePath(config)
 
-    let result: TreeVersion = { contentHash: NEW_RESOURCE_VERSION, files: [] }
+    let result: TreeVersionWithHashes = { contentHash: NEW_RESOURCE_VERSION, files: [] }
 
     // Make sure we don't concurrently scan the exact same context
     await scanLock.acquire(cacheKey.join(":"), async () => {
@@ -296,7 +301,7 @@ export abstract class VcsHandler {
           stringsForContentHash = filesToHash.map((f) => f.hash)
         }
         result.contentHash = hashStrings(stringsForContentHash)
-        result.files = files.map((f) => f.path)
+        result.files = files
       }
 
       this.cache.set(log, cacheKey, result, pathToCacheContext(path))
@@ -306,6 +311,14 @@ export abstract class VcsHandler {
     return result
   }
 
+  async getTreeVersion(params: GetTreeVersionParams): Promise<TreeVersion> {
+    const treeVersionWithHashes = await this.getTreeVersionWithHashes(params)
+    return {
+      contentHash: treeVersionWithHashes.contentHash,
+      files: treeVersionWithHashes.files.map((f) => f.path),
+    }
+  }
+
   /**
    * Write a file and ensure relevant caches are invalidated after writing.
    */

core/test/integ/src/commands/diff.ts

@@ -135,7 +135,7 @@ describe("DiffCommand", () => {
     expect(summary).to.include("Source files changed")
     expect(summary).to.include("M Dockerfile")
     expect(summary).to.include("+ test.txt")
-    expect(summary).to.include("8 files unchanged")
+    expect(summary).to.include("7 files unchanged")
 
     const files = result.actions["build.api"].files
     const changedFiles = files.filter((file) => file.status === "modified")

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

@@ -215,15 +215,15 @@ describe("VcsHandler", () => {
       const moduleConfig = await gardenA.resolveModule("module-a")
       const cacheKey = getResourceTreeCacheKey(moduleConfig)
 
-      const cachedResult = { contentHash: "abcdef", files: ["foo"] }
+      const cachedResult = { contentHash: "abcdef", files: [{ path: "foo", hash: "abcdef" }] }
       handlerA["cache"].set(gardenA.log, cacheKey, cachedResult, ["foo", "bar"])
 
       const result = await handlerA.getTreeVersion({
         log: gardenA.log,
         projectName: gardenA.projectName,
         config: moduleConfig,
       })
-      expect(result).to.eql(cachedResult)
+      expect(result).to.eql({ contentHash: "abcdef", files: ["foo"] })
     })
 
     it("should cache the resolved version", async () => {