From 881c3c77becefa28a69db306d1d96740b9f36db6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Thu, 23 Aug 2018 15:44:12 +0200 Subject: [PATCH] fix(multi-repo): ensure external source gets updated if repo url changes --- .../src/commands/update-remote/helpers.ts | 26 ++++-- .../src/commands/update-remote/modules.ts | 2 +- .../src/commands/update-remote/sources.ts | 2 +- garden-cli/src/util/ext-source-util.ts | 21 ++++- garden-cli/src/vcs/base.ts | 15 +++- garden-cli/src/vcs/git.ts | 83 ++++++++++++------- garden-cli/test/helpers.ts | 6 +- garden-cli/test/src/garden.ts | 26 ++++-- garden-cli/test/src/util/ext-source-util.ts | 30 +++++-- 9 files changed, 154 insertions(+), 57 deletions(-) diff --git a/garden-cli/src/commands/update-remote/helpers.ts b/garden-cli/src/commands/update-remote/helpers.ts index f0f27dc03c..2832353efb 100644 --- a/garden-cli/src/commands/update-remote/helpers.ts +++ b/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)) } } diff --git a/garden-cli/src/commands/update-remote/modules.ts b/garden-cli/src/commands/update-remote/modules.ts index 3fd71c6be6..ed9207f1e2 100644 --- a/garden-cli/src/commands/update-remote/modules.ts +++ b/garden-cli/src/commands/update-remote/modules.ts @@ -76,7 +76,7 @@ export class UpdateRemoteModulesCommand extends Command Promise => { - 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 => { + 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 { - 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") } } diff --git a/garden-cli/test/helpers.ts b/garden-cli/test/helpers.ts index b9fafba3d1..1d809c28b7 100644 --- a/garden-cli/test/helpers.ts +++ b/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")) } diff --git a/garden-cli/test/src/garden.ts b/garden-cli/test/src/garden.ts index ea364c9003..13d0b59126 100644 --- a/garden-cli/test/src/garden.ts +++ b/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 () => { diff --git a/garden-cli/test/src/util/ext-source-util.ts b/garden-cli/test/src/util/ext-source-util.ts index 26fcabce50..9cf9e5bcd0 100644 --- a/garden-cli/test/src/util/ext-source-util.ts +++ b/garden-cli/test/src/util/ext-source-util.ts @@ -1,10 +1,12 @@ import { expect } from "chai" import { - getRemoteSourcesDirName, + getRemoteSourcesDirname, getLinkedSources, addLinkedSources, removeLinkedSources, + getRemoteSourcePath, + hashRepoUrl, } from "../../../src/util/ext-source-util" import { makeTestContextA, cleanProject, expectError } from "../../helpers" import { PluginContext } from "../../../src/plugin-context" @@ -22,17 +24,35 @@ describe("ext-source-util", () => { await cleanProject(ctx.projectRoot) }) - it("should should return the project sources dir name", () => { - const dirName = getRemoteSourcesDirName("project") + it("should return the relative path to the remote projects directory", () => { + const dirName = getRemoteSourcesDirname("project") expect(dirName).to.equal(".garden/sources/project") }) - it("should should return the modules sources dir name", () => { - const dirName = getRemoteSourcesDirName("module") + it("should return the relative path to the remote modules directory", () => { + const dirName = getRemoteSourcesDirname("module") expect(dirName).to.equal(".garden/sources/module") }) }) + describe("getRemoteSourcePath", () => { + it("should return the relative path to a remote project source", () => { + const url = "banana" + const urlHash = hashRepoUrl(url) + + const path = getRemoteSourcePath({ url, name: "my-source", sourceType: "project" }) + expect(path).to.equal(`.garden/sources/project/my-source--${urlHash}`) + }) + + it("should return the relative path to a remote module source", () => { + const url = "banana" + const urlHash = hashRepoUrl(url) + + const path = getRemoteSourcePath({ url, name: "my-module", sourceType: "module" }) + expect(path).to.equal(`.garden/sources/module/my-module--${urlHash}`) + }) + }) + describe("getLinkedSources", () => { beforeEach(async () => { ctx = await makeTestContextA()