fix(core): ensure deletes are synced when staging builds

Due to our mishandling/misunderstanding of rsync behavior (see
https://stackoverflow.com/questions/1813907), deleted files would
linger in the build staging directory.

Fixes #1278
Fixes #1314

garden-service/package.json

@@ -94,6 +94,7 @@
     "path-is-inside": "^1.0.2",
     "pluralize": "^8.0.0",
     "proper-url-join": "^2.0.1",
+    "recursive-readdir": "^2.2.2",
     "request": "^2.88.0",
     "request-promise": "^4.2.4",
     "source-map-support": "^0.5.13",
@@ -158,6 +159,7 @@
     "@types/path-is-inside": "^1.0.0",
     "@types/pluralize": "0.0.29",
     "@types/prettyjson": "0.0.29",
+    "@types/recursive-readdir": "^2.2.0",
     "@types/request": "^2.48.2",
     "@types/request-promise": "^4.1.44",
     "@types/split2": "^2.1.6",
@@ -228,4 +230,4 @@
     ]
   },
   "gitHead": "b0647221a4d2ff06952bae58000b104215aed922"
-}
+}

garden-service/src/build-dir.ts

@@ -17,6 +17,7 @@ import { LogEntry } from "./logger/log-entry"
 import { ModuleConfig } from "./config/module"
 import { ConfigGraph } from "./config-graph"
 import { exec } from "./util/util"
+import { LogLevel } from "./logger/log-node"
 
 // FIXME: We don't want to keep special casing this module type so we need to think
 // of a better way around this.
@@ -158,11 +159,7 @@ export class BuildDir {
     destinationPath = stripWildcard(destinationPath)
 
     // --exclude is required for modules where the module and project are in the same directory
-    const syncOpts = ["-rptgo", `--exclude=${this.buildDirPath}`]
-
-    if (withDelete) {
-      syncOpts.push("--delete")
-    }
+    const syncOpts = ["-rptgo", `--exclude=${this.buildDirPath}`, "--ignore-missing-args"]
 
     let logMsg =
       `Syncing ${module.version.files.length} files from ` +
@@ -174,12 +171,28 @@ export class BuildDir {
 
     log.debug(logMsg)
 
+    // rsync benefits from file lists being sorted
+    files && files.sort()
     let input: string | undefined
 
-    if (files !== undefined) {
+    if (withDelete) {
+      syncOpts.push("--prune-empty-dirs")
+
+      if (files === undefined) {
+        syncOpts.push("--delete")
+      } else {
+        // Workaround for this issue:
+        // https://stackoverflow.com/questions/1813907/rsync-delete-files-from-list-dest-does-not-delete-unwanted-files
+        syncOpts.push("--include-from=-", "--exclude=*", "--delete-excluded")
+        input = "/**/\n" + files.join("\n")
+      }
+    } else if (files !== undefined) {
       syncOpts.push("--files-from=-")
-      files = files.sort()
       input = files.join("\n")
+    }
+
+    // Avoid rendering the full file list except when at the silly log level
+    if (log.root.level === LogLevel.silly) {
       log.silly(`File list: ${JSON.stringify(files)}`)
     }
 

garden-service/test/unit/src/build-dir.ts

@@ -1,6 +1,6 @@
 const nodetree = require("nodetree")
 import { join } from "path"
-import { pathExists, readdir } from "fs-extra"
+import { pathExists, readdir, createFile } from "fs-extra"
 import { expect } from "chai"
 import { BuildTask } from "../../../src/tasks/build"
 import { makeTestGarden, dataDir } from "../../helpers"
@@ -50,40 +50,58 @@ describe("BuildDir", () => {
     expect(await pathExists(buildPath)).to.eql(true)
   })
 
-  it("should sync sources to the build dir", async () => {
-    const graph = await garden.getConfigGraph()
-    const moduleA = await graph.getModule("module-a")
-    await garden.buildDir.syncFromSrc(moduleA, garden.log)
-    const buildDirA = await garden.buildDir.buildPath(moduleA)
+  describe("syncFromSrc", () => {
+    it("should sync sources to the build dir", async () => {
+      const graph = await garden.getConfigGraph()
+      const moduleA = await graph.getModule("module-a")
+      await garden.buildDir.syncFromSrc(moduleA, garden.log)
+      const buildDirA = await garden.buildDir.buildPath(moduleA)
 
-    const copiedPaths = [join(buildDirA, "some-dir", "some-file")]
+      const copiedPaths = [join(buildDirA, "some-dir", "some-file")]
 
-    for (const p of copiedPaths) {
-      expect(await pathExists(p)).to.eql(true)
-    }
-  })
+      for (const p of copiedPaths) {
+        expect(await pathExists(p)).to.eql(true)
+      }
+    })
 
-  it("should not sync sources for local exec modules", async () => {
-    const graph = await garden.getConfigGraph()
-    const moduleE = await graph.getModule("module-e")
-    await garden.buildDir.syncFromSrc(moduleE, garden.log)
-    // This is the dir Garden would have synced the sources into
-    const buildDirF = join(garden.buildDir.buildDirPath, moduleE.name)
+    it("should not sync sources for local exec modules", async () => {
+      const graph = await garden.getConfigGraph()
+      const moduleE = await graph.getModule("module-e")
+      await garden.buildDir.syncFromSrc(moduleE, garden.log)
+      // This is the dir Garden would have synced the sources into
+      const buildDirF = join(garden.buildDir.buildDirPath, moduleE.name)
 
-    expect(await pathExists(buildDirF)).to.eql(false)
-  })
+      expect(await pathExists(buildDirF)).to.eql(false)
+    })
 
-  it("should respect the file list in the module's version", async () => {
-    const graph = await garden.getConfigGraph()
-    const moduleA = await graph.getModule("module-a")
+    it("should respect the file list in the module's version", async () => {
+      const graph = await garden.getConfigGraph()
+      const moduleA = await graph.getModule("module-a")
 
-    moduleA.version.files = [await getConfigFilePath(moduleA.path)]
+      moduleA.version.files = [await getConfigFilePath(moduleA.path)]
 
-    await garden.buildDir.syncFromSrc(moduleA, garden.log)
-    const buildDirA = await garden.buildDir.buildPath(moduleA)
+      await garden.buildDir.syncFromSrc(moduleA, garden.log)
+      const buildDirA = await garden.buildDir.buildPath(moduleA)
+
+      expect(await pathExists(await getConfigFilePath(buildDirA))).to.eql(true)
+      expect(await pathExists(join(buildDirA, "some-dir", "some-file"))).to.eql(false)
+    })
+
+    it("should delete files that are not being synced from the module source directory", async () => {
+      const graph = await garden.getConfigGraph()
+      const moduleA = await graph.getModule("module-a")
 
-    expect(await pathExists(await getConfigFilePath(buildDirA))).to.eql(true)
-    expect(await pathExists(join(buildDirA, "some-dir", "some-file"))).to.eql(false)
+      const buildDirA = await garden.buildDir.buildPath(moduleA)
+      const deleteMe = join(buildDirA, "delete-me")
+
+      await createFile(deleteMe)
+
+      moduleA.version.files = [await getConfigFilePath(moduleA.path)]
+
+      await garden.buildDir.syncFromSrc(moduleA, garden.log)
+
+      expect(await pathExists(deleteMe)).to.be.false
+    })
   })
 
   it("should sync dependency products to their specified destinations", async () => {