fix(core): versioning fix for remote sources (#5735)

<!--  Thanks for sending a pull request! Here are some tips for you:

1. If this is your first pull request, please read our contributor
guidelines in the
https://github.com/garden-io/garden/blob/main/CONTRIBUTING.md file.
2. Please label this pull request according to what type of issue you
are addressing (see "What type of PR is this?" below)
3. Ensure you have added or run the appropriate tests for your PR.
4. If the PR is unfinished, add `WIP:` at the beginning of the title or
use the GitHub Draft PR feature.
5. Please add at least two reviewers to the PR. Currently active
maintainers are: @edvald, @thsig, @eysi09, @shumailxyz, @stefreak,
@TimBeyer, @mkhq, and @vvagaytsev.
-->

**What this PR does / why we need it**:

Before this fix, we were including the relative path to a file from
project root in the string to be hashed when calculating versions.

This was problematic when linking remote sources, since the chosen name
and path of the locally linked repo would be factored into the version
(resulting in unwanted cache misses).

This was fixed by using the relative path from the directory containing
the file's parent config. For example, an action's directory should be
able to be moved around or renamed without it affecting the version of
that action.

**Which issue(s) this PR fixes**:

Fixes #5501.

core/src/vcs/vcs.ts

@@ -11,6 +11,7 @@ import { createHash } from "node:crypto"
 import { relative, sep } from "path"
 import { DOCS_BASE_URL } from "../constants.js"
 import fsExtra from "fs-extra"
+import { dirname } from "node:path"
 
 const { writeFile } = fsExtra
 import type { ExternalSourceType } from "../util/ext-source-util.js"
@@ -271,8 +272,16 @@ export abstract class VcsHandler {
           // Don't include the config file in the file list
           .filter((f) => !configPath || f.path !== configPath)
 
-        // compute hash using <file-relative-path>-<file-hash> to cater for path changes (e.g. renaming)
-        result.contentHash = hashStrings(files.map((f) => `${relative(this.projectRoot, f.path)}-${f.hash}`))
+        let stringsForContenthash: string[]
+        if (configPath) {
+          // Include the relative path to the file to account for the file being renamed or moved around within the
+          // config path (e.g. renaming).
+          const configDir = dirname(configPath)
+          stringsForContenthash = files.map((f) => `${relative(configDir, f.path)}-${f.hash}`)
+        } else {
+          stringsForContenthash = files.map((f) => f.hash)
+        }
+        result.contentHash = hashStrings(stringsForContenthash)
         result.files = files.map((f) => f.path)
       }
 

core/test/data/test-projects/include-exclude/module-a/garden.yml → core/test/data/test-projects/include-exclude/build-a/garden.yml

@@ -1,5 +1,5 @@
-kind: Module
-name: module-a
+kind: Build
+name: a
 type: test
 include: ["yes.txt", "somedir/*"]
 exclude: ["somedir/nope.txt"]

core/test/data/test-projects/include-exclude/module-b/garden.yml → core/test/data/test-projects/include-exclude/build-b/garden.yml

@@ -1,5 +1,5 @@
-kind: Module
-name: module-b
+kind: Build
+name: b
 type: test
 # test both dir name and relative path
 exclude: ["nope.*", "./dir1", "dir2"]

core/test/data/test-projects/include-exclude/module-c/garden.yml → core/test/data/test-projects/include-exclude/build-c/garden.yml

@@ -1,5 +1,5 @@
-kind: Module
-name: module-c
+kind: Build
+name: c
 type: test
 include: ["*.txt"]
 exclude: ["nope.*"]

core/test/unit/src/garden.ts

@@ -5191,9 +5191,9 @@ describe("Garden", () => {
     })
 
     context("test against fixed version hashes", async () => {
-      const moduleAVersionString = "v-e3eed855ed"
-      const moduleBVersionString = "v-cdfff6b4f2"
-      const moduleCVersionString = "v-9d2afce9e4"
+      const moduleAVersionString = "v-55de0aac5c"
+      const moduleBVersionString = "v-daeabf68fe"
+      const moduleCVersionString = "v-5e9ddea45e"
 
       it("should return the same module versions between runtimes", async () => {
         const projectRoot = getDataDir("test-projects", "fixed-version-hashes-1")

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

@@ -10,7 +10,7 @@ import { execa } from "execa"
 import { expect } from "chai"
 import tmp from "tmp-promise"
 import fsExtra from "fs-extra"
-import { basename, join, relative, resolve } from "path"
+import { basename, dirname, join, relative, resolve } from "path"
 
 import type { TestGarden } from "../../../helpers.js"
 import { expectError, getDataDir, makeTestGarden, makeTestGardenA } from "../../../helpers.js"
@@ -1467,7 +1467,10 @@ const getTreeVersionTests = (gitScanMode: GitScanMode) => {
       it("should respect the include field, if specified", async () => {
         const projectRoot = getDataDir("test-projects", "include-exclude")
         const garden = await makeTestGarden(projectRoot, { gitScanMode })
-        const moduleConfig = await garden.resolveModule("module-a")
+        const log = garden.log
+        const graph = await garden.getConfigGraph({ log, emit: false })
+        const build = graph.getBuild("a")
+        const buildConfig = build.getConfig()
         const handler = new gitHandlerCls({
           garden,
           projectRoot: garden.projectRoot,
@@ -1479,19 +1482,22 @@ const getTreeVersionTests = (gitScanMode: GitScanMode) => {
         const version = await handler.getTreeVersion({
           log: garden.log,
           projectName: garden.projectName,
-          config: moduleConfig,
+          config: buildConfig,
         })
 
         expect(version.files).to.eql([
-          resolve(moduleConfig.path, "somedir/yes.txt"),
-          resolve(moduleConfig.path, "yes.txt"),
+          resolve(dirname(build.configPath()!), "somedir/yes.txt"),
+          resolve(dirname(build.configPath()!), "yes.txt"),
         ])
       })
 
       it("should respect the exclude field, if specified", async () => {
         const projectRoot = getDataDir("test-projects", "include-exclude")
         const garden = await makeTestGarden(projectRoot, { gitScanMode })
-        const moduleConfig = await garden.resolveModule("module-b")
+        const log = garden.log
+        const graph = await garden.getConfigGraph({ log, emit: false })
+        const build = graph.getBuild("b")
+        const buildConfig = build.getConfig()
         const handler = new gitHandlerCls({
           garden,
           projectRoot: garden.projectRoot,
@@ -1503,16 +1509,19 @@ const getTreeVersionTests = (gitScanMode: GitScanMode) => {
         const version = await handler.getTreeVersion({
           log: garden.log,
           projectName: garden.projectName,
-          config: moduleConfig,
+          config: buildConfig,
         })
 
-        expect(version.files).to.eql([resolve(moduleConfig.path, "yes.txt")])
+        expect(version.files).to.eql([resolve(dirname(build.configPath()!), "yes.txt")])
       })
 
       it("should respect both include and exclude fields, if specified", async () => {
         const projectRoot = getDataDir("test-projects", "include-exclude")
         const garden = await makeTestGarden(projectRoot, { gitScanMode })
-        const moduleConfig = await garden.resolveModule("module-c")
+        const log = garden.log
+        const graph = await garden.getConfigGraph({ log, emit: false })
+        const build = graph.getBuild("b")
+        const buildConfig = build.getConfig()
 
         const handler = new gitHandlerCls({
           garden,
@@ -1525,10 +1534,10 @@ const getTreeVersionTests = (gitScanMode: GitScanMode) => {
         const version = await handler.getTreeVersion({
           log: garden.log,
           projectName: garden.projectName,
-          config: moduleConfig,
+          config: buildConfig,
         })
 
-        expect(version.files).to.eql([resolve(moduleConfig.path, "yes.txt")])
+        expect(version.files).to.eql([resolve(dirname(build.configPath()!), "yes.txt")])
       })
     })
   })

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

@@ -237,27 +237,29 @@ describe("VcsHandler", () => {
       expect(result).to.eql(cachedResult)
     })
 
-    it("should update content hash when include is set and there's a change in included files of module", async () => {
+    it("should update content hash when include is set and there's a change in the included files of an action", async () => {
       const projectRoot = getDataDir("test-projects", "include-exclude")
       const garden = await makeTestGarden(projectRoot)
-      const moduleConfigA1 = await garden.resolveModule("module-a")
-      const newFilePathModuleA = join(garden.projectRoot, "module-a", "somedir", "foo")
+      const log = garden.log
+      const graph = await garden.getConfigGraph({ emit: false, log })
+      const buildConfig = graph.getBuild("a").getConfig()
+      const newFilePathBuildA = join(garden.projectRoot, "build-a", "somedir", "foo")
       try {
         const version1 = await garden.vcs.getTreeVersion({
           log: garden.log,
           projectName: garden.projectName,
-          config: moduleConfigA1,
+          config: buildConfig,
         })
-        await writeFile(newFilePathModuleA, "abcd")
+        await writeFile(newFilePathBuildA, "abcd")
         const version2 = await garden.vcs.getTreeVersion({
           log: garden.log,
           projectName: garden.projectName,
-          config: moduleConfigA1,
+          config: buildConfig,
           force: true,
         })
         expect(version1).to.not.eql(version2)
       } finally {
-        await rm(newFilePathModuleA)
+        await rm(newFilePathBuildA)
       }
     })
 
@@ -305,30 +307,61 @@ describe("VcsHandler", () => {
       }
     })
 
-    it("should update content hash, when a file is renamed", async () => {
+    it("should update content hash when a file is renamed", async () => {
       const projectRoot = getDataDir("test-projects", "include-exclude")
       const garden = await makeTestGarden(projectRoot)
-      const newFilePathModuleA = join(garden.projectRoot, "module-a", "somedir", "foo")
-      const renamedFilePathModuleA = join(garden.projectRoot, "module-a", "somedir", "bar")
+      const log = garden.log
+      const newFilePathBuildA = join(garden.projectRoot, "build-a", "somedir", "foo")
+      const renamedFilePathBuildA = join(garden.projectRoot, "build-a", "somedir", "bar")
       try {
-        await writeFile(newFilePathModuleA, "abcd")
-        const moduleConfigA1 = await garden.resolveModule("module-a")
+        await writeFile(newFilePathBuildA, "abcd")
+        const graph = await garden.getConfigGraph({ emit: false, log })
+        const buildConfig = graph.getBuild("a").getConfig()
         const version1 = await garden.vcs.getTreeVersion({
           log: garden.log,
           projectName: garden.projectName,
-          config: moduleConfigA1,
+          config: buildConfig,
         })
         // rename file foo to bar
-        await rename(newFilePathModuleA, renamedFilePathModuleA)
+        await rename(newFilePathBuildA, renamedFilePathBuildA)
         const version2 = await garden.vcs.getTreeVersion({
           log: garden.log,
           projectName: garden.projectName,
-          config: moduleConfigA1,
+          config: buildConfig,
+          force: true,
+        })
+        expect(version1).to.not.eql(version2)
+      } finally {
+        await rm(renamedFilePathBuildA)
+      }
+    })
+
+    it("should not update content hash when the parent config's enclosing directory is renamed", async () => {
+      const projectRoot = getDataDir("test-projects", "include-exclude")
+      const garden = await makeTestGarden(projectRoot)
+      const log = garden.log
+      const newFilePathBuildA = join(garden.projectRoot, "build-a", "somedir", "foo")
+      const renamedFilePathBuildA = join(garden.projectRoot, "build-a", "somedir", "bar")
+      try {
+        await writeFile(newFilePathBuildA, "abcd")
+        const graph = await garden.getConfigGraph({ emit: false, log })
+        const buildConfig = graph.getBuild("a").getConfig()
+        const version1 = await garden.vcs.getTreeVersion({
+          log: garden.log,
+          projectName: garden.projectName,
+          config: buildConfig,
+        })
+        // rename file foo to bar
+        await rename(newFilePathBuildA, renamedFilePathBuildA)
+        const version2 = await garden.vcs.getTreeVersion({
+          log: garden.log,
+          projectName: garden.projectName,
+          config: buildConfig,
           force: true,
         })
         expect(version1).to.not.eql(version2)
       } finally {
-        await rm(renamedFilePathModuleA)
+        await rm(renamedFilePathBuildA)
       }
     })
   })
@@ -422,7 +455,7 @@ describe("getModuleVersionString", () => {
     const garden = await makeTestGarden(projectRoot, { noCache: true })
     const module = await garden.resolveModule("module-a")
 
-    const fixedVersionString = "v-e3eed855ed"
+    const fixedVersionString = "v-55de0aac5c"
     expect(module.version.versionString).to.eql(fixedVersionString)
 
     delete process.env.TEST_ENV_VAR