fix(core): don't flag remote modules as overlapping with modules in root

Came across this while debugging another issue. Simple fix.
garden-io · Nov 26, 2020 · 8059741 · 8059741
1 parent dcd8be7
commit 8059741
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 33 deletions.
diff --git a/core/src/garden.ts b/core/src/garden.ts
@@ -788,7 +788,11 @@ export class Garden {
     let graph: ConfigGraph | undefined = undefined
 
     // Require include/exclude on modules if their paths overlap
-    const overlaps = detectModuleOverlap(resolvedModules)
+    const overlaps = detectModuleOverlap({
+      projectRoot: this.projectRoot,
+      gardenDirPath: this.gardenDirPath,
+      moduleConfigs: resolvedModules,
+    })
     if (overlaps.length > 0) {
       const { message, detail } = this.makeOverlapError(overlaps)
       throw new ConfigurationError(message, detail)

diff --git a/core/src/util/fs.ts b/core/src/util/fs.ts
@@ -74,13 +74,28 @@ export interface ModuleOverlap {
   overlaps: ModuleConfig[]
 }
 
-export function detectModuleOverlap(moduleConfigs: ModuleConfig[]): ModuleOverlap[] {
+export function detectModuleOverlap({
+  projectRoot,
+  gardenDirPath,
+  moduleConfigs,
+}: {
+  projectRoot: string
+  gardenDirPath: string
+  moduleConfigs: ModuleConfig[]
+}): ModuleOverlap[] {
   let overlaps: ModuleOverlap[] = []
   for (const config of moduleConfigs) {
-    const setsBuildCtx = !!config.include || !!config.exclude
+    if (!!config.include || !!config.exclude) {
+      continue
+    }
     const matches = moduleConfigs
-      .filter((compare) => config.name !== compare.name)
-      .filter((compare) => !setsBuildCtx && pathIsInside(compare.path, config.path))
+      .filter(
+        (compare) =>
+          config.name !== compare.name &&
+          pathIsInside(compare.path, config.path) &&
+          // Don't consider overlap between modules in root and those in the .garden directory
+          !(config.path === projectRoot && pathIsInside(compare.path, gardenDirPath))
+      )
       .sort((a, b) => (a.name > b.name ? 1 : -1))
 
     if (matches.length > 0) {

diff --git a/core/test/unit/src/util/fs.ts b/core/test/unit/src/util/fs.ts
@@ -24,24 +24,29 @@ import { ModuleConfig } from "../../../../src/config/module"
 
 describe("util", () => {
   describe("detectModuleOverlap", () => {
+    const projectRoot = join("/", "user", "code")
+    const gardenDirPath = join(projectRoot, ".garden")
+
     it("should detect if modules have the same root", () => {
       const moduleA = {
         name: "module-a",
-        path: join("/", "user", "code", "foo"),
+        path: join(projectRoot, "foo"),
       } as ModuleConfig
       const moduleB = {
         name: "module-b",
-        path: join("/", "user", "code", "foo"),
+        path: join(projectRoot, "foo"),
       } as ModuleConfig
       const moduleC = {
         name: "module-c",
-        path: join("/", "user", "code", "foo"),
+        path: join(projectRoot, "foo"),
       } as ModuleConfig
       const moduleD = {
         name: "module-d",
-        path: join("/", "user", "code", "bas"),
+        path: join(projectRoot, "bas"),
       } as ModuleConfig
-      expect(detectModuleOverlap([moduleA, moduleB, moduleC, moduleD])).to.eql([
+      expect(
+        detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleA, moduleB, moduleC, moduleD] })
+      ).to.eql([
         {
           module: moduleA,
           overlaps: [moduleB, moduleC],
@@ -59,21 +64,23 @@ describe("util", () => {
     it("should detect if a module has another module in its path", () => {
       const moduleA = {
         name: "module-a",
-        path: join("/", "user", "code", "foo"),
+        path: join(projectRoot, "foo"),
       } as ModuleConfig
       const moduleB = {
         name: "module-b",
-        path: join("/", "user", "code", "foo", "bar"),
+        path: join(projectRoot, "foo", "bar"),
       } as ModuleConfig
       const moduleC = {
         name: "module-c",
-        path: join("/", "user", "code", "foo", "bar", "bas"),
+        path: join(projectRoot, "foo", "bar", "bas"),
       } as ModuleConfig
       const moduleD = {
         name: "module-d",
-        path: join("/", "user", "code", "bas", "bar", "bas"),
+        path: join(projectRoot, "bas", "bar", "bas"),
       } as ModuleConfig
-      expect(detectModuleOverlap([moduleA, moduleB, moduleC, moduleD])).to.eql([
+      expect(
+        detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleA, moduleB, moduleC, moduleD] })
+      ).to.eql([
         {
           module: moduleA,
           overlaps: [moduleB, moduleC],
@@ -89,14 +96,14 @@ describe("util", () => {
       it("should ignore modules that set includes", () => {
         const moduleA = {
           name: "module-a",
-          path: join("/", "user", "code", "foo"),
+          path: join(projectRoot, "foo"),
           include: [""],
         } as ModuleConfig
         const moduleB = {
           name: "module-b",
-          path: join("/", "user", "code", "foo"),
+          path: join(projectRoot, "foo"),
         } as ModuleConfig
-        expect(detectModuleOverlap([moduleA, moduleB])).to.eql([
+        expect(detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleA, moduleB] })).to.eql([
           {
             module: moduleB,
             overlaps: [moduleA],
@@ -106,14 +113,14 @@ describe("util", () => {
       it("should ignore modules that set excludes", () => {
         const moduleA = {
           name: "module-a",
-          path: join("/", "user", "code", "foo"),
+          path: join(projectRoot, "foo"),
           exclude: [""],
         } as ModuleConfig
         const moduleB = {
           name: "module-b",
-          path: join("/", "user", "code", "foo"),
+          path: join(projectRoot, "foo"),
         } as ModuleConfig
-        expect(detectModuleOverlap([moduleA, moduleB])).to.eql([
+        expect(detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleA, moduleB] })).to.eql([
           {
             module: moduleB,
             overlaps: [moduleA],
@@ -126,59 +133,76 @@ describe("util", () => {
       it("should ignore modules that set includes", () => {
         const moduleA = {
           name: "module-a",
-          path: join("/", "user", "code", "foo"),
+          path: join(projectRoot, "foo"),
           include: [""],
         } as ModuleConfig
         const moduleB = {
           name: "module-b",
-          path: join("/", "user", "code", "foo", "bar"),
+          path: join(projectRoot, "foo", "bar"),
         } as ModuleConfig
-        expect(detectModuleOverlap([moduleA, moduleB])).to.be.empty
+        expect(detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleA, moduleB] })).to.be.empty
       })
+
       it("should ignore modules that set excludes", () => {
         const moduleA = {
           name: "module-a",
-          path: join("/", "user", "code", "foo"),
+          path: join(projectRoot, "foo"),
           exclude: [""],
         } as ModuleConfig
         const moduleB = {
           name: "module-b",
-          path: join("/", "user", "code", "foo", "bar"),
+          path: join(projectRoot, "foo", "bar"),
         } as ModuleConfig
-        expect(detectModuleOverlap([moduleA, moduleB])).to.be.empty
+        expect(detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleA, moduleB] })).to.be.empty
       })
+
       it("should detect overlaps if only nested module has includes/excludes", () => {
         const moduleA1 = {
           name: "module-a",
-          path: join("/", "user", "code", "foo"),
+          path: join(projectRoot, "foo"),
         } as ModuleConfig
         const moduleB1 = {
           name: "module-b",
-          path: join("/", "user", "code", "foo", "bar"),
+          path: join(projectRoot, "foo", "bar"),
           include: [""],
         } as ModuleConfig
         const moduleA2 = {
           name: "module-a",
-          path: join("/", "user", "code", "foo"),
+          path: join(projectRoot, "foo"),
         } as ModuleConfig
         const moduleB2 = {
           name: "module-b",
-          path: join("/", "user", "code", "foo", "bar"),
+          path: join(projectRoot, "foo", "bar"),
           exclude: [""],
         } as ModuleConfig
-        expect(detectModuleOverlap([moduleA1, moduleB1])).to.eql([
+        expect(detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleA1, moduleB1] })).to.eql([
           {
             module: moduleA1,
             overlaps: [moduleB1],
           },
         ])
-        expect(detectModuleOverlap([moduleA2, moduleB2])).to.eql([
+        expect(detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleA2, moduleB2] })).to.eql([
           {
             module: moduleA2,
             overlaps: [moduleB2],
           },
         ])
       })
+
+      it("should not consider remote source modules to overlap with module in project root", () => {
+        const remoteModule = {
+          name: "remote-module",
+          path: join(gardenDirPath, "sources", "foo", "bar"),
+        } as ModuleConfig
+
+        const moduleFoo = {
+          name: "module-foo",
+          path: join(projectRoot, "foo"),
+          include: [""],
+        } as ModuleConfig
+
+        expect(detectModuleOverlap({ projectRoot, gardenDirPath, moduleConfigs: [moduleFoo, remoteModule] })).to.eql([])
+      })
     })
   })