refactor: build command is now an array, for consistency

BREAKING CHANGE: Any existing garden.yml files with the `build.command` key set need to be updated to provide an array of strings as a command, as opposed to a simple string.
garden-io · Jul 10, 2018 · 0bf020a · 0bf020a
1 parent ab3714b
commit 0bf020a
Show file tree

Hide file tree

Showing 35 changed files with 102 additions and 58 deletions.
diff --git a/docs/reference/config.md b/docs/reference/config.md
@@ -55,10 +55,15 @@ module:
   build: 
     # The command to run inside the module directory to perform the build.
     # 
-    # Example: "npm run build"
+    # Example:
+    #   - npm
+    #   - run
+    #   - build
     # 
     # Optional.
     command: 
+      - 
+
 
     # A list of modules that must be built before this module is built.
     # 
@@ -230,10 +235,15 @@ module:
   build: 
     # The command to run inside the module directory to perform the build.
     # 
-    # Example: "npm run build"
+    # Example:
+    #   - npm
+    #   - run
+    #   - build
     # 
     # Optional.
     command: 
+      - 
+
 
     # A list of modules that must be built before this module is built.
     # 
@@ -355,10 +365,15 @@ module:
   build: 
     # The command to run inside the module directory to perform the build.
     # 
-    # Example: "npm run build"
+    # Example:
+    #   - npm
+    #   - run
+    #   - build
     # 
     # Optional.
     command: 
+      - 
+
 
     # A list of modules that must be built before this module is built.
     # 

diff --git a/src/garden.ts b/src/garden.ts
@@ -20,6 +20,7 @@ import {
   merge,
   pick,
   keyBy,
+  cloneDeep,
 } from "lodash"
 import * as Joi from "joi"
 import { TreeCache } from "./cache"
@@ -703,7 +704,7 @@ export class Garden {
     // Looks like a path
     const path = resolve(this.projectRoot, nameOrLocation)
     const config = await loadConfig(this.projectRoot, path)
-    const moduleConfig = config.module
+    const moduleConfig = cloneDeep(config.module)
 
     if (!moduleConfig) {
       return null

diff --git a/src/plugins/generic.ts b/src/plugins/generic.ts
@@ -6,8 +6,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { exec } from "child-process-promise"
 import * as Joi from "joi"
+import { mapValues } from "lodash"
 import { join } from "path"
 import {
   joiArray,
@@ -43,7 +43,8 @@ import {
   baseTestSpecSchema,
 } from "../types/test"
 import { spawn } from "../util/util"
-import { TreeVersion, writeVersionFile, readVersionFile } from "../vcs/base"
+import { writeVersionFile, readVersionFile } from "../vcs/base"
+import execa = require("execa")
 
 export const name = "generic"
 export const buildVersionFilename = ".garden-build-version"
@@ -97,12 +98,15 @@ export async function parseGenericModule(
 export async function buildGenericModule({ module }: BuildModuleParams<GenericModule>): Promise<BuildResult> {
   const config: ModuleConfig = module.config
 
-  if (config.build.command) {
+  if (config.build.command.length) {
     const buildPath = await module.getBuildPath()
-    const result = await exec(config.build.command, {
-      cwd: buildPath,
-      env: { ...process.env, ...module.spec.env },
-    })
+    const result = await execa.shell(
+      config.build.command.join(" "),
+      {
+        cwd: buildPath,
+        env: { ...process.env, ...mapValues(module.spec.env, v => v.toString()) },
+      },
+    )
 
     // keep track of which version has been built
     const buildVersionFilePath = join(buildPath, buildVersionFilename)
@@ -155,7 +159,7 @@ export const genericPlugin: GardenPlugin = {
       testModule: testGenericModule,
 
       async getModuleBuildStatus({ module }: GetModuleBuildStatusParams): Promise<BuildStatus> {
-        if (!module.config.build.command) {
+        if (!module.config.build.command.length) {
           return { ready: true }
         }
 

diff --git a/src/plugins/local/local-google-cloud-functions.ts b/src/plugins/local/local-google-cloud-functions.ts
@@ -80,6 +80,7 @@ export const gardenPlugin = (): GardenPlugin => ({
         const module: ModuleConfig<ContainerModuleSpec> = {
           allowPush: true,
           build: {
+            command: [],
             dependencies: parsed.module.build.dependencies.concat([{
               name: emulatorModuleName,
               plugin: pluginName,

diff --git a/src/types/module.ts b/src/types/module.ts
@@ -76,8 +76,7 @@ export const buildDependencySchema = Joi.object().keys({
 })
 
 export interface BuildConfig {
-  // TODO: this should be a string array, to match other command specs
-  command?: string,
+  command: string[],
   dependencies: BuildDependencyConfig[],
 }
 
@@ -109,9 +108,9 @@ export const baseModuleSpecSchema = Joi.object()
       .default(true)
       .description("Set to false to disable pushing this module to remote registries."),
     build: Joi.object().keys({
-      command: Joi.string()
+      command: joiArray(Joi.string())
         .description("The command to run inside the module directory to perform the build.")
-        .example("npm run build"),
+        .example(["npm", "run", "build"]),
       dependencies: joiArray(buildDependencySchema)
         .description("A list of modules that must be built before this module is built.")
         .example([{ name: "some-other-module-name" }]),

diff --git a/test/data/test-project-a/module-a/garden.yml b/test/data/test-project-a/module-a/garden.yml
@@ -4,7 +4,7 @@ module:
   services:
     - name: service-a
   build:
-    command: echo A
+    command: [echo, A]
   tests:
     - name: unit
       command: [echo, OK]
diff --git a/test/data/test-project-a/module-b/garden.yml b/test/data/test-project-a/module-b/garden.yml
@@ -6,7 +6,7 @@ module:
       dependencies:
         - service-a
   build:
-    command: echo B
+    command: [echo, B]
     dependencies:
       - module-a
   tests:

diff --git a/test/data/test-project-auto-reload/module-a/garden.yml b/test/data/test-project-auto-reload/module-a/garden.yml
@@ -11,4 +11,4 @@ module:
         - name: http
           containerPort: 8080
   build:
-    command: echo A
+    command: [echo, A]
diff --git a/test/data/test-project-auto-reload/module-b/garden.yml b/test/data/test-project-auto-reload/module-b/garden.yml
@@ -13,6 +13,6 @@ module:
         - name: http
           containerPort: 8080
   build:
-    command: echo B
+    command: [echo, B]
     dependencies:
       - module-a
diff --git a/test/data/test-project-auto-reload/module-c/garden.yml b/test/data/test-project-auto-reload/module-c/garden.yml
@@ -11,4 +11,4 @@ module:
         - name: http
           containerPort: 8080
   build:
-    command: echo C
+    command: [echo, C]
diff --git a/test/data/test-project-auto-reload/module-d/garden.yml b/test/data/test-project-auto-reload/module-d/garden.yml
@@ -13,6 +13,6 @@ module:
         - name: http
           containerPort: 8080
   build:
-    command: echo D
+    command: [echo, D]
     dependencies:
       - module-b
diff --git a/test/data/test-project-auto-reload/module-e/garden.yml b/test/data/test-project-auto-reload/module-e/garden.yml
@@ -15,4 +15,4 @@ module:
   build:
     dependencies:
       - module-b
-    command: echo E
+    command: [echo, E]
diff --git a/test/data/test-project-auto-reload/module-f/garden.yml b/test/data/test-project-auto-reload/module-f/garden.yml
@@ -15,4 +15,4 @@ module:
   build:
     dependencies:
       - module-c
-    command: echo F
+    command: [echo, F]
diff --git a/test/data/test-project-b/module-a/garden.yml b/test/data/test-project-b/module-a/garden.yml
@@ -10,4 +10,4 @@ module:
         - name: http
           containerPort: 8080
   build:
-    command: echo A
+    command: [echo, A]
diff --git a/test/data/test-project-b/module-b/garden.yml b/test/data/test-project-b/module-b/garden.yml
@@ -12,6 +12,6 @@ module:
       dependencies:
         - service-a
   build:
-    command: echo B
+    command: [echo, B]
     dependencies:
       - module-a
diff --git a/test/data/test-project-build-products/module-a/garden.yml b/test/data/test-project-build-products/module-a/garden.yml
@@ -2,4 +2,4 @@ module:
   name: module-a
   type: test
   build:
-    command: "echo A && touch a.txt"
+    command: [echo, A, "&&", touch, a.txt]
diff --git a/test/data/test-project-build-products/module-b/garden.yml b/test/data/test-project-build-products/module-b/garden.yml
@@ -2,4 +2,10 @@ module:
   name: module-b
   type: test
   build:
-    command: "echo B && mkdir -p build/build_subdir && touch build/b1.txt build/build_subdir/b2.txt build/unused.txt"
+    command: [
+      echo, B,
+      "&&",
+      mkdir, -p, build/build_subdir,
+      "&&",
+      touch, build/b1.txt, build/build_subdir/b2.txt, build/unused.txt
+    ]
diff --git a/test/data/test-project-build-products/module-c/garden.yml b/test/data/test-project-build-products/module-c/garden.yml
@@ -2,4 +2,4 @@ module:
   name: module-c
   type: test
   build:
-    command: "echo C"
+    command: [echo, C]
diff --git a/test/data/test-project-build-products/module-d/garden.yml b/test/data/test-project-build-products/module-d/garden.yml
@@ -2,7 +2,13 @@ module:
   name: module-d
   type: test
   build:
-    command: "echo D && mkdir -p build && touch build/d.txt"
+    command: [
+      echo, D,
+      "&&",
+      mkdir, -p, build,
+      "&&",
+      touch, build/d.txt
+    ]
     dependencies:
       - name: module-a
         copy:

diff --git a/test/data/test-project-build-products/module-e/garden.yml b/test/data/test-project-build-products/module-e/garden.yml
@@ -2,7 +2,7 @@ module:
   name: module-e
   type: test
   build:
-    command: "echo E"
+    command: [echo, E]
     dependencies:
       - name: module-d
         copy:

diff --git a/test/data/test-project-circular-deps/module-a/garden.yml b/test/data/test-project-circular-deps/module-a/garden.yml
@@ -9,6 +9,6 @@ module:
       dependencies:
         - service-c
   build:
-    command: echo A
+    command: [echo, A]
     dependencies:
       - module-c
diff --git a/test/data/test-project-circular-deps/module-b/garden.yml b/test/data/test-project-circular-deps/module-b/garden.yml
@@ -10,6 +10,6 @@ module:
         - service-a
         - service-c
   build:
-    command: echo B
+    command: [echo, B]
     dependencies:
       - module-a
diff --git a/test/data/test-project-templated/garden.yml b/test/data/test-project-templated/garden.yml
@@ -3,7 +3,7 @@ project:
   environmentDefaults:
     variables:
       some: ${local.env.TEST_VARIABLE}
-      service-a-build-command: echo OK
+      service-a-build-command: OK
   environments:
     - name: local
       providers:

diff --git a/test/data/test-project-templated/module-a/garden.yml b/test/data/test-project-templated/module-a/garden.yml
@@ -5,7 +5,7 @@ module:
     - name: service-a
       command: [echo, "${local.env.TEST_VARIABLE}"]
   build:
-    command: ${variables.service-a-build-command}
+    command: [echo, "${variables.service-a-build-command}"]
   tests:
     - name: unit
       command: [echo, OK]
diff --git a/test/data/test-project-templated/module-b/garden.yml b/test/data/test-project-templated/module-b/garden.yml
@@ -7,7 +7,7 @@ module:
       dependencies:
         - service-a
   build:
-    command: ${variables.service-a-build-command}
+    command: [echo, "${variables.service-a-build-command}"]
   tests:
     - name: unit
       command: [echo, "${config.project.my.variable}"]
diff --git a/test/helpers.ts b/test/helpers.ts
@@ -185,7 +185,7 @@ export const defaultModuleConfig: ModuleConfig = {
   path: "bla",
   allowPush: false,
   variables: {},
-  build: { dependencies: [] },
+  build: { command: [], dependencies: [] },
   spec: {
     services: [
       {

diff --git a/test/src/commands/build.ts b/test/src/commands/build.ts
@@ -13,8 +13,8 @@ describe("commands.build", () => {
     const { result } = await command.action(ctx, { module: undefined }, { watch: false, force: true })
 
     expect(taskResultOutputs(result!)).to.eql({
-      "build.module-a": { fresh: true, buildLog: "A\n" },
-      "build.module-b": { fresh: true, buildLog: "B\n" },
+      "build.module-a": { fresh: true, buildLog: "A" },
+      "build.module-b": { fresh: true, buildLog: "B" },
       "build.module-c": {},
     })
   })
@@ -26,8 +26,8 @@ describe("commands.build", () => {
     const { result } = await command.action(ctx, { module: "module-b" }, { watch: false, force: true })
 
     expect(taskResultOutputs(result!)).to.eql({
-      "build.module-a": { fresh: true, buildLog: "A\n" },
-      "build.module-b": { fresh: true, buildLog: "B\n" },
+      "build.module-a": { fresh: true, buildLog: "A" },
+      "build.module-b": { fresh: true, buildLog: "B" },
     })
   })
 })
diff --git a/test/src/commands/data/validate/bad-project/ok-module/garden.yml b/test/src/commands/data/validate/bad-project/ok-module/garden.yml
@@ -2,4 +2,4 @@ module:
   name: module-a
   type: generic
   build:
-    command: echo A
+    command: [echo, A]
diff --git a/test/src/commands/deploy.ts b/test/src/commands/deploy.ts
@@ -82,8 +82,8 @@ describe("DeployCommand", () => {
     )
 
     expect(taskResultOutputs(result!)).to.eql({
-      "build.module-a": { fresh: true, buildLog: "A\n" },
-      "build.module-b": { fresh: true, buildLog: "B\n" },
+      "build.module-a": { fresh: true, buildLog: "A" },
+      "build.module-b": { fresh: true, buildLog: "B" },
       "build.module-c": {},
       "deploy.service-a": { version: "1", state: "ready" },
       "deploy.service-b": { version: "1", state: "ready" },
@@ -109,8 +109,8 @@ describe("DeployCommand", () => {
     )
 
     expect(taskResultOutputs(result!)).to.eql({
-      "build.module-a": { fresh: true, buildLog: "A\n" },
-      "build.module-b": { fresh: true, buildLog: "B\n" },
+      "build.module-a": { fresh: true, buildLog: "A" },
+      "build.module-b": { fresh: true, buildLog: "B" },
       "deploy.service-a": { version: "1", state: "ready" },
       "deploy.service-b": { version: "1", state: "ready" },
     })

diff --git a/test/src/commands/push.ts b/test/src/commands/push.ts
@@ -190,7 +190,7 @@ describe("PushCommand", () => {
 
     expect(taskResultOutputs(result!)).to.eql({
       "build.module-a": {
-        buildLog: "A\n",
+        buildLog: "A",
         fresh: true,
       },
       "push.module-a": { pushed: false, message: chalk.yellow("No push handler available for module type test") },