fix(multi-repo): ensure external source gets updated if repo url changes

garden-cli/src/commands/update-remote/helpers.ts

@@ -7,26 +7,36 @@
  */
 
 import { difference } from "lodash"
-import { join } from "path"
+import { join, basename } from "path"
 import { remove, pathExists } from "fs-extra"
 
 import { getChildDirNames } from "../../util/util"
-import { ExternalSourceType, getRemoteSourcesDirName } from "../../util/ext-source-util"
+import {
+  ExternalSourceType,
+  getRemoteSourcesDirname,
+  getRemoteSourcePath,
+} from "../../util/ext-source-util"
+import { SourceConfig } from "../../config/project"
 
-export async function pruneRemoteSources({ projectRoot, names, type }: {
+export async function pruneRemoteSources({ projectRoot, sources, type }: {
   projectRoot: string,
-  names: string[],
+  sources: SourceConfig[],
   type: ExternalSourceType,
 }) {
-  const remoteSourcesPath = join(projectRoot, getRemoteSourcesDirName(type))
+  const remoteSourcesPath = join(projectRoot, getRemoteSourcesDirname(type))
 
   if (!(await pathExists(remoteSourcesPath))) {
     return
   }
 
-  const currentRemoteSourceNames = await getChildDirNames(remoteSourcesPath)
-  const staleRemoteSourceNames = difference(currentRemoteSourceNames, names)
-  for (const dirName of staleRemoteSourceNames) {
+  const sourceNames = sources
+    .map(({ name, repositoryUrl: url }) => getRemoteSourcePath({ name, url, sourceType: type }))
+    .map(srcPath => basename(srcPath))
+
+  const currentRemoteSources = await getChildDirNames(remoteSourcesPath)
+  const staleRemoteSources = difference(currentRemoteSources, sourceNames)
+
+  for (const dirName of staleRemoteSources) {
     await remove(join(remoteSourcesPath, dirName))
   }
 }

garden-cli/src/commands/update-remote/modules.ts

@@ -76,7 +76,7 @@ export class UpdateRemoteModulesCommand extends Command<typeof updateRemoteModul
       await ctx.vcs.updateRemoteSource({ name, url: repositoryUrl, sourceType: "module", logEntry: ctx.log })
     }
 
-    await pruneRemoteSources({ names, projectRoot: ctx.projectRoot, type: "module" })
+    await pruneRemoteSources({ projectRoot: ctx.projectRoot, type: "module", sources: moduleSources })
 
     return { result: moduleSources }
   }

garden-cli/src/commands/update-remote/sources.ts

@@ -72,7 +72,7 @@ export class UpdateRemoteSourcesCommand extends Command<typeof updateRemoteSourc
       await ctx.vcs.updateRemoteSource({ name, url: repositoryUrl, sourceType: "project", logEntry: ctx.log })
     }
 
-    await pruneRemoteSources({ names, projectRoot: ctx.projectRoot, type: "project" })
+    await pruneRemoteSources({ projectRoot: ctx.projectRoot, type: "project", sources: projectSources })
 
     return { result: projectSources }
   }

garden-cli/src/util/ext-source-util.ts

@@ -6,6 +6,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+import { createHash } from "crypto"
 import { uniqBy } from "lodash"
 import chalk from "chalk"
 import pathIsInside = require("path-is-inside")
@@ -21,16 +22,34 @@ import {
 import { ParameterError } from "../exceptions"
 import { Module } from "../types/module"
 import { PluginContext } from "../plugin-context"
+import { join } from "path"
 
 export type ExternalSourceType = "project" | "module"
 
-export function getRemoteSourcesDirName(type: ExternalSourceType): string {
+export function getRemoteSourcesDirname(type: ExternalSourceType): string {
   return type === "project" ? PROJECT_SOURCES_DIR_NAME : MODULE_SOURCES_DIR_NAME
 }
 
+/**
+ * A remote source dir name has the format 'source-name--HASH_OF_REPO_URL'
+ * so that we can detect if the repo url has changed
+ */
+export function getRemoteSourcePath({ name, url, sourceType }:
+  { name: string, url: string, sourceType: ExternalSourceType }) {
+  const dirname = name + "--" + hashRepoUrl(url)
+  return join(getRemoteSourcesDirname(sourceType), dirname)
+}
+
+export function hashRepoUrl(url: string) {
+  const urlHash = createHash("sha256")
+  urlHash.update(url)
+  return urlHash.digest("hex").slice(0, 10)
+}
+
 export function hasRemoteSource(module: Module): boolean {
   return !!module.repositoryUrl
 }
+
 export function getConfigKey(type: ExternalSourceType): string {
   return type === "project" ? localConfigKeys.linkedProjectSources : localConfigKeys.linkedModuleSources
 }

garden-cli/src/vcs/base.ts

@@ -15,7 +15,11 @@ import { join } from "path"
 import { GARDEN_VERSIONFILE_NAME } from "../constants"
 import { pathExists, readFile, writeFile } from "fs-extra"
 import { ConfigurationError } from "../exceptions"
-import { ExternalSourceType, getRemoteSourcesDirName } from "../util/ext-source-util"
+import {
+  ExternalSourceType,
+  getRemoteSourcesDirname,
+  getRemoteSourcePath,
+} from "../util/ext-source-util"
 import { LogNode } from "../logger/logger"
 import { ModuleConfig } from "../config/module"
 
@@ -47,7 +51,7 @@ const dirtyTimestampSchema = Joi.number()
   .required()
   .description(
     "Set to the last modified time (as UNIX timestamp) if the module contains uncommitted changes, otherwise null.",
-  )
+)
 
 export const treeVersionSchema = Joi.object()
   .keys({
@@ -159,10 +163,13 @@ export abstract class VcsHandler {
     }
   }
 
-  getRemoteSourcesDirName(type: ExternalSourceType) {
-    return getRemoteSourcesDirName(type)
+  getRemoteSourcesDirname(type: ExternalSourceType) {
+    return getRemoteSourcesDirname(type)
   }
 
+  getRemoteSourcePath(name, url, sourceType) {
+    return getRemoteSourcePath({ name, url, sourceType })
+  }
 }
 
 function hashVersions(versions: NamedTreeVersion[]) {

garden-cli/src/vcs/git.ts

@@ -6,30 +6,37 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { exec } from "child-process-promise"
+import execa = require("execa")
 import { join } from "path"
 import { ensureDir, pathExists, stat } from "fs-extra"
 import { argv } from "process"
 import Bluebird = require("bluebird")
-import { parse } from "url"
 
 import { NEW_MODULE_VERSION, VcsHandler, RemoteSourceParams } from "./base"
 import { EntryStyle } from "../logger/types"
 
 export const helpers = {
-  gitCli: (cwd: string): (args: string | string[]) => Promise<string> => {
-    return async args => {
-      const cmd = Array.isArray(args) ? `git ${args.join(" && git ")}` : `git ${args}`
-      const res = await exec(cmd, { cwd })
-      return res.stdout.trim()
+  gitCli: (cwd: string): (cmd: string, args: string[]) => Promise<string> => {
+    return async (cmd, args) => {
+      return execa.stdout("git", [cmd, ...args], { cwd })
     }
   },
 }
 
-function getUrlHash(url: string) {
-  return (parse(url).hash || "").split("#")[1]
+function getGitUrlParts(url: string) {
+  const parts = url.split("#")
+  return { repositoryUrl: parts[0], hash: parts[1] }
 }
 
+function parseRefList(res: string): string {
+  const refList = res.split("\n").map(str => {
+    const parts = str.split("\n")
+    return { commitId: parts[0], ref: parts[1] }
+  })
+  return refList[0].commitId
+}
+
+// TODO Consider moving git commands to separate (and testable) functions
 export class GitHandler extends VcsHandler {
   name = "git"
 
@@ -38,7 +45,12 @@ export class GitHandler extends VcsHandler {
 
     let commitHash
     try {
-      commitHash = await git("rev-list -1 --abbrev-commit --abbrev=10 HEAD") || NEW_MODULE_VERSION
+      commitHash = await git("rev-list", [
+        "--max-count=1",
+        "--abbrev-commit",
+        "--abbrev=10",
+        "HEAD",
+      ]) || NEW_MODULE_VERSION
     } catch (err) {
       if (err.code === 128) {
         // not in a repo root, return default version
@@ -48,13 +60,13 @@ export class GitHandler extends VcsHandler {
 
     let latestDirty = 0
 
-    const res = await git([`diff-index --name-only HEAD ${path}`, `ls-files --other --exclude-standard ${path}`])
+    const res = await git("diff-index", ["--name-only", "HEAD", path]) + "\n"
+      + await git("ls-files", ["--other", "--exclude-standard", path])
 
     const dirtyFiles: string[] = res.split("\n").filter((f) => f.length > 0)
     // for dirty trees, we append the last modified time of last modified or added file
     if (dirtyFiles.length) {
-
-      const repoRoot = await git("rev-parse --show-toplevel")
+      const repoRoot = await git("rev-parse", ["--show-toplevel"])
       const stats = await Bluebird.map(dirtyFiles, file => join(repoRoot, file))
         .filter((file: string) => pathExists(file))
         .map((file: string) => stat(file))
@@ -75,43 +87,56 @@ export class GitHandler extends VcsHandler {
 
   // TODO Better auth handling
   async ensureRemoteSource({ url, name, logEntry, sourceType }: RemoteSourceParams): Promise<string> {
-    const remoteSourcesPath = join(this.projectRoot, this.getRemoteSourcesDirName(sourceType))
+    const remoteSourcesPath = join(this.projectRoot, this.getRemoteSourcesDirname(sourceType))
     await ensureDir(remoteSourcesPath)
     const git = helpers.gitCli(remoteSourcesPath)
-    const fullPath = join(remoteSourcesPath, name)
 
-    if (!(await pathExists(fullPath))) {
+    const absPath = join(this.projectRoot, this.getRemoteSourcePath(name, url, sourceType))
+    const isCloned = await pathExists(absPath)
+
+    if (!isCloned) {
       const entry = logEntry.info({ section: name, msg: `Fetching from ${url}`, entryStyle: EntryStyle.activity })
-      const hash = getUrlHash(url)
-      const branch = hash ? `--branch=${hash}` : ""
+      const { repositoryUrl, hash } = getGitUrlParts(url)
 
-      await git(`clone --depth=1 ${branch} ${url} ${name}`)
+      const cmdOpts = ["--depth=1"]
+      if (hash) {
+        cmdOpts.push("--branch=hash")
+      }
+
+      await git("clone", [...cmdOpts, repositoryUrl, absPath])
 
       entry.setSuccess()
     }
 
-    return fullPath
+    return absPath
   }
 
   async updateRemoteSource({ url, name, sourceType, logEntry }: RemoteSourceParams) {
-    const sourcePath = join(this.projectRoot, this.getRemoteSourcesDirName(sourceType), name)
-    const git = helpers.gitCli(sourcePath)
+    const absPath = join(this.projectRoot, this.getRemoteSourcePath(name, url, sourceType))
+    const git = helpers.gitCli(absPath)
+    const { repositoryUrl, hash } = getGitUrlParts(url)
+
     await this.ensureRemoteSource({ url, name, sourceType, logEntry })
 
     const entry = logEntry.info({ section: name, msg: "Getting remote state", entryStyle: EntryStyle.activity })
-    await git("remote update")
+    await git("remote", ["update"])
+
+    const listRemoteArgs = hash ? [repositoryUrl, hash] : [repositoryUrl]
+    const showRefArgs = hash ? [hash] : []
+    const remoteCommitId = parseRefList(await git("ls-remote", listRemoteArgs))
+    const localCommitId = parseRefList(await git("show-ref", ["--hash", ...showRefArgs]))
 
-    const remoteHash = await git("rev-parse @")
-    const localHash = await git("rev-parse @{u}")
-    if (localHash !== remoteHash) {
+    if (localCommitId !== remoteCommitId) {
       entry.setState({ section: name, msg: `Fetching from ${url}`, entryStyle: EntryStyle.activity })
-      const hash = getUrlHash(url)
 
-      await git([`fetch origin ${hash} --depth=1`, `reset origin/${hash} --hard`])
+      const fetchArgs = hash ? ["origin", hash] : ["origin"]
+      const resetArgs = hash ? [`origin/${hash}`] : ["origin"]
+      await git("fetch", ["--depth=1", ...fetchArgs])
+      await git("reset", ["--hard", ...resetArgs])
 
       entry.setSuccess("Source updated")
     } else {
-      entry.setSuccess("Source up to date")
+      entry.setSuccess("Source already up to date")
     }
   }
 

garden-cli/test/helpers.ts

@@ -282,8 +282,8 @@ export function stubGitCli() {
  */
 export function stubExtSources(garden: Garden) {
   stubGitCli()
-  const getRemoteSourcesDirName = td.replace(garden.vcs, "getRemoteSourcesDirName")
+  const getRemoteSourcesDirname = td.replace(garden.vcs, "getRemoteSourcesDirname")
 
-  td.when(getRemoteSourcesDirName("module")).thenReturn(join("mock-dot-garden", "sources", "module"))
-  td.when(getRemoteSourcesDirName("project")).thenReturn(join("mock-dot-garden", "sources", "project"))
+  td.when(getRemoteSourcesDirname("module")).thenReturn(join("mock-dot-garden", "sources", "module"))
+  td.when(getRemoteSourcesDirname("project")).thenReturn(join("mock-dot-garden", "sources", "project"))
 }

garden-cli/test/src/garden.ts

@@ -22,6 +22,7 @@ import { getNames } from "../../src/util/util"
 import { MOCK_CONFIG } from "../../src/cli/cli"
 import { LinkedSource } from "../../src/config-store"
 import { ModuleVersion } from "../../src/vcs/base"
+import { hashRepoUrl } from "../../src/util/ext-source-util"
 
 describe("Garden", () => {
   beforeEach(async () => {
@@ -255,7 +256,16 @@ describe("Garden", () => {
 
     it("should scan and add modules for projects with external project sources", async () => {
       const garden = await makeTestGarden(resolve(dataDir, "test-project-ext-project-sources"))
+
+      const getRemoteSourcePath = td.replace(garden.vcs, "getRemoteSourcePath")
+      td.when(getRemoteSourcePath("source-a"), { ignoreExtraArgs: true })
+        .thenReturn(join("mock-dot-garden", "sources", "project", "source-a"))
+      td.when(getRemoteSourcePath("source-b"), { ignoreExtraArgs: true })
+        .thenReturn(join("mock-dot-garden", "sources", "project", "source-b"))
+      td.when(getRemoteSourcePath("source-c"), { ignoreExtraArgs: true })
+        .thenReturn(join("mock-dot-garden", "sources", "project", "source-c"))
       stubExtSources(garden)
+
       await garden.scanModules()
 
       const modules = await garden.getModules(undefined, true)
@@ -415,7 +425,9 @@ describe("Garden", () => {
       stubGitCli()
 
       const module = await garden.resolveModule("./module-a")
-      expect(module!.path).to.equal(join(projectRoot, ".garden", "sources", "module", "module-a"))
+      const repoUrlHash = hashRepoUrl(module!.repositoryUrl!)
+
+      expect(module!.path).to.equal(join(projectRoot, ".garden", "sources", "module", `module-a--${repoUrlHash}`))
     })
   })
 
@@ -580,15 +592,19 @@ describe("Garden", () => {
     it("should return the path to the project source if source type is project", async () => {
       projectRoot = getDataDir("test-project-ext-project-sources")
       const ctx = await makeGardenContext(projectRoot)
-      const path = await ctx.loadExtSourcePath({ name: "source-a", repositoryUrl: "", sourceType: "project" })
-      expect(path).to.equal(join(projectRoot, ".garden", "sources", "project", "source-a"))
+      const repositoryUrl = "foo"
+      const path = await ctx.loadExtSourcePath({ repositoryUrl, name: "source-a", sourceType: "project" })
+      const repoUrlHash = hashRepoUrl(repositoryUrl)
+      expect(path).to.equal(join(projectRoot, ".garden", "sources", "project", `source-a--${repoUrlHash}`))
     })
 
     it("should return the path to the module source if source type is module", async () => {
       projectRoot = getDataDir("test-project-ext-module-sources")
       const ctx = await makeGardenContext(projectRoot)
-      const path = await ctx.loadExtSourcePath({ name: "module-a", repositoryUrl: "", sourceType: "module" })
-      expect(path).to.equal(join(projectRoot, ".garden", "sources", "module", "module-a"))
+      const repositoryUrl = "foo"
+      const path = await ctx.loadExtSourcePath({ repositoryUrl, name: "module-a", sourceType: "module" })
+      const repoUrlHash = hashRepoUrl(repositoryUrl)
+      expect(path).to.equal(join(projectRoot, ".garden", "sources", "module", `module-a--${repoUrlHash}`))
     })
 
     it("should return the local path of the project source if linked", async () => {