Skip to content

Commit

Permalink
fix(core): fix circular dependency detection
Browse files Browse the repository at this point in the history
Before this fix, circular build dependencies between modules would
result in an internal error (instead of an informative error message).
  • Loading branch information
thsig authored and edvald committed Oct 14, 2020
1 parent 37d21cf commit 802f118
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 7 deletions.
16 changes: 9 additions & 7 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ import { RuntimeContext } from "./runtime-context"
import { loadPlugins, getDependencyOrder, getModuleTypes } from "./plugins"
import { deline, naturalList } from "./util/string"
import { ensureConnected } from "./db/connection"
import { DependencyValidationGraph } from "./util/validate-dependencies"
import { cyclesToString, DependencyValidationGraph } from "./util/validate-dependencies"
import { Profile } from "./util/profiling"
import { ResolveModuleTask, getResolvedModules, moduleResolutionConcurrencyLimit } from "./tasks/resolve-module"
import username from "username"
Expand Down Expand Up @@ -768,13 +768,15 @@ export class Garden {
} catch (err) {
// Wrap the circular dependency error to print a more specific message
if (err.type === "circular-dependencies") {
const cycles = err.cycles.map((c: string[][]) => {
// Get the module name of the the cycle (anything else is internal detail as far as users are concerned)
return c.map((cycle) => cycle.map((key) => key.split(".")[1]))
})
// Get the module names from the cycle keys (anything else is internal detail as far as users are concerned)
const cycleDescription = cyclesToString([err.detail.cycle.map((key: string) => key.split(".")[1])])
throw new ConfigurationError(
`Detected one or more circular dependencies between module configurations:\n\n${cycles.join("\n")}`,
{ cycles }
dedent`
Detected circular dependencies between module configurations:
${cycleDescription}
`,
{ cycle: err.detail.cycle }
)
} else {
throw err
Expand Down
57 changes: 57 additions & 0 deletions core/test/unit/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2784,6 +2784,63 @@ describe("Garden", () => {
})
})

describe("getConfigGraph", () => {
it("should throw an error if modules have circular build dependencies", async () => {
const garden = await TestGarden.factory(pathFoo, {
config: {
apiVersion: DEFAULT_API_VERSION,
kind: "Project",
name: "test",
path: pathFoo,
defaultEnvironment: "default",
dotIgnoreFiles: [],
environments: [{ name: "default", defaultNamespace, variables: {} }],
providers: [],
variables: {},
},
})

garden.setModuleConfigs([
{
apiVersion: DEFAULT_API_VERSION,
name: "module-a",
type: "exec",
allowPublish: false,
build: { dependencies: [{ name: "module-b", copy: [] }] }, // <----
disabled: false,
path: pathFoo,
serviceConfigs: [],
taskConfigs: [],
testConfigs: [],
spec: {},
},
{
apiVersion: DEFAULT_API_VERSION,
name: "module-b",
type: "exec",
allowPublish: false,
build: { dependencies: [{ name: "module-a", copy: [] }] }, // <----
disabled: false,
path: pathFoo,
serviceConfigs: [],
taskConfigs: [],
testConfigs: [],
spec: {},
},
])

await expectError(
() => garden.getConfigGraph(garden.log),
(err) =>
expect(err.message).to.equal(dedent`
Detected circular dependencies between module configurations:
module-a <- module-b <- module-a
`)
)
})
})

context("module type has a base", () => {
it("should throw if the configure handler output doesn't match the module type's base schema", async () => {
const base = createGardenPlugin({
Expand Down

0 comments on commit 802f118

Please sign in to comment.