From ea001d404a54fddad346e394c1a5dc9001132e01 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Wed, 17 Jul 2019 14:34:48 +0200 Subject: [PATCH] fix: various issues with path handling on Windows --- garden-service/package-lock.json | 6 ++ garden-service/package.json | 4 +- garden-service/src/bin/add-version-files.ts | 12 ++-- garden-service/src/build-dir.ts | 6 +- .../src/plugins/kubernetes/container/build.ts | 2 +- garden-service/src/vcs/vcs.ts | 31 ++++++--- garden-service/test/unit/src/vcs/vcs.ts | 68 ++++++++++++++++++- 7 files changed, 109 insertions(+), 20 deletions(-) diff --git a/garden-service/package-lock.json b/garden-service/package-lock.json index fd87681308..8780de1456 100644 --- a/garden-service/package-lock.json +++ b/garden-service/package-lock.json @@ -852,6 +852,12 @@ "@types/node": "*" } }, + "@types/normalize-path": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@types/normalize-path/-/normalize-path-3.0.0.tgz", + "integrity": "sha512-Nd8y/5t/7CRakPYiyPzr/IAfYusy1FkcZYFEAcoMZkwpJv2n4Wm+olW+e7xBdHEXhOnWdG9ddbar0gqZWS4x5Q==", + "dev": true + }, "@types/path-is-inside": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/@types/path-is-inside/-/path-is-inside-1.0.0.tgz", diff --git a/garden-service/package.json b/garden-service/package.json index ad5432d924..07c52b6a76 100644 --- a/garden-service/package.json +++ b/garden-service/package.json @@ -81,6 +81,7 @@ "moment": "^2.24.0", "node-emoji": "^1.10.0", "node-forge": "^0.8.2", + "normalize-path": "^3.0.0", "normalize-url": "^4.3.0", "p-queue": "^5.0.0", "p-retry": "^4.1.0", @@ -142,6 +143,7 @@ "@types/node": "^12.0.1", "@types/node-emoji": "^1.8.1", "@types/node-forge": "^0.8.2", + "@types/normalize-path": "^3.0.0", "@types/path-is-inside": "^1.0.0", "@types/pluralize": "0.0.29", "@types/prettyjson": "0.0.29", @@ -212,4 +214,4 @@ }, "snyk": true, "gitHead": "b0647221a4d2ff06952bae58000b104215aed922" -} \ No newline at end of file +} diff --git a/garden-service/src/bin/add-version-files.ts b/garden-service/src/bin/add-version-files.ts index b028b358f4..04cec4c383 100644 --- a/garden-service/src/bin/add-version-files.ts +++ b/garden-service/src/bin/add-version-files.ts @@ -12,8 +12,8 @@ import { Logger } from "../logger/logger" import { LogLevel } from "../logger/log-node" import { resolve, relative } from "path" import * as Bluebird from "bluebird" -import { writeFile } from "fs-extra" -import { STATIC_DIR } from "../constants" +import { STATIC_DIR, GARDEN_VERSIONFILE_NAME } from "../constants" +import { writeTreeVersionFile } from "../vcs/vcs" // make sure logger is initialized try { @@ -30,18 +30,14 @@ async function addVersionFiles() { return Bluebird.map(moduleConfigs, async (config) => { const path = config.path - const versionFilePath = resolve(path, ".garden-version") + const versionFilePath = resolve(path, GARDEN_VERSIONFILE_NAME) const vcsHandler = new GitHandler(garden.gardenDirPath) const treeVersion = await vcsHandler.getTreeVersion(path, config.include || null) - treeVersion.files = treeVersion.files - .map(f => relative(path, f)) - .filter(f => f !== ".garden-version") - console.log(`${config.name} -> ${relative(STATIC_DIR, versionFilePath)}`) - return writeFile(versionFilePath, JSON.stringify(treeVersion, null, 4) + "\n") + return writeTreeVersionFile(path, treeVersion) }) } diff --git a/garden-service/src/build-dir.ts b/garden-service/src/build-dir.ts index 738c909f56..1a7e77a7fb 100644 --- a/garden-service/src/build-dir.ts +++ b/garden-service/src/build-dir.ts @@ -7,6 +7,7 @@ */ import { map as bluebirdMap } from "bluebird" +import normalize = require("normalize-path") import { isAbsolute, join, @@ -38,7 +39,8 @@ export class BuildDir { async syncFromSrc(module: Module, log: LogEntry) { const files = module.version.files - .map(f => isAbsolute(f) ? relative(module.path, f) : f) + // Normalize to relative POSIX-style paths + .map(f => normalize(isAbsolute(f) ? relative(module.path, f) : f)) await this.sync({ module, @@ -150,7 +152,9 @@ export class BuildDir { if (files !== undefined) { syncOpts.push("--files-from=-") + files = files.sort() input = files.join("\n") + log.silly(`File list: ${JSON.stringify(files)}`) } await execa("rsync", [...syncOpts, sourcePath, destinationPath], { input }) diff --git a/garden-service/src/plugins/kubernetes/container/build.ts b/garden-service/src/plugins/kubernetes/container/build.ts index 9ac63e7f18..6ba69bc1c1 100644 --- a/garden-service/src/plugins/kubernetes/container/build.ts +++ b/garden-service/src/plugins/kubernetes/container/build.ts @@ -138,7 +138,7 @@ const remoteBuild: BuildHandler = async (params) => { const buildRoot = resolve(module.buildPath, "..") // The '/./' trick is used to automatically create the correct target directory with rsync: // https://stackoverflow.com/questions/1636889/rsync-how-can-i-configure-it-to-create-target-directory-on-server - let src = normalizeLocalRsyncPath(`${buildRoot}/./${module.name}/`) + let src = normalizeLocalRsyncPath(`${buildRoot}`) + `/./${module.name}/` const destination = `rsync://localhost:${syncFwd.localPort}/volume/${ctx.workingCopyId}/` diff --git a/garden-service/src/vcs/vcs.ts b/garden-service/src/vcs/vcs.ts index 1ad594c563..f59ba2fedd 100644 --- a/garden-service/src/vcs/vcs.ts +++ b/garden-service/src/vcs/vcs.ts @@ -8,11 +8,12 @@ import * as Joi from "@hapi/joi" import * as Bluebird from "bluebird" +import normalize = require("normalize-path") import { mapValues, keyBy, sortBy, omit } from "lodash" import { createHash } from "crypto" import { validate, joiArray, joi } from "../config/common" -import { join } from "path" -import { GARDEN_VERSIONFILE_NAME } from "../constants" +import { join, relative, isAbsolute } from "path" +import { GARDEN_VERSIONFILE_NAME as GARDEN_TREEVERSION_FILENAME } from "../constants" import { pathExists, readFile, writeFile } from "fs-extra" import { ConfigurationError } from "../exceptions" import { ExternalSourceType, getRemoteSourcesDirname, getRemoteSourceRelPath } from "../util/ext-source-util" @@ -93,7 +94,7 @@ export abstract class VcsHandler { async resolveTreeVersion(path: string, include: string[] | null): Promise { // the version file is used internally to specify versions outside of source control - const versionFilePath = join(path, GARDEN_VERSIONFILE_NAME) + const versionFilePath = join(path, GARDEN_TREEVERSION_FILENAME) const fileVersion = await readTreeVersionFile(versionFilePath) return fileVersion || this.getTreeVersion(path, include) } @@ -176,16 +177,30 @@ export async function readTreeVersionFile(path: string): Promise { return readVersionFile(path, moduleVersionSchema) } +/** + * Writes a normalized TreeVersion file to the specified directory + * + * @param dir The directory to write the file to + * @param version The TreeVersion for the directory + */ +export async function writeTreeVersionFile(dir: string, version: TreeVersion) { + const processed = { + ...version, + files: version.files + // Always write relative paths, normalized to POSIX style + .map(f => normalize(isAbsolute(f) ? relative(dir, f) : f)) + .filter(f => f !== GARDEN_TREEVERSION_FILENAME), + } + const path = join(dir, GARDEN_TREEVERSION_FILENAME) + await writeFile(path, JSON.stringify(processed, null, 4) + "\n") +} + export async function writeModuleVersionFile(path: string, version: ModuleVersion) { - await writeFile(path, JSON.stringify(version)) + await writeFile(path, JSON.stringify(version, null, 4) + "\n") } /** diff --git a/garden-service/test/unit/src/vcs/vcs.ts b/garden-service/test/unit/src/vcs/vcs.ts index 034dcfa7c7..5cdf8577ba 100644 --- a/garden-service/test/unit/src/vcs/vcs.ts +++ b/garden-service/test/unit/src/vcs/vcs.ts @@ -3,6 +3,8 @@ import { TreeVersions, TreeVersion, getVersionString, + writeTreeVersionFile, + readTreeVersionFile, } from "../../../../src/vcs/vcs" import { projectRootA, makeTestGardenA, makeTestGarden, getDataDir } from "../../../helpers" import { expect } from "chai" @@ -11,8 +13,11 @@ import { Garden } from "../../../../src/garden" import { ModuleConfigContext } from "../../../../src/config/config-context" import { ModuleConfig } from "../../../../src/config/module" import { GitHandler } from "../../../../src/vcs/git" -import { resolve } from "path" +import { resolve, join } from "path" import * as td from "testdouble" +import * as tmp from "tmp-promise" +import { realpath } from "fs-extra" +import { GARDEN_VERSIONFILE_NAME } from "../../../../src/constants" class TestVcsHandler extends VcsHandler { name = "test" @@ -258,3 +263,64 @@ describe("VcsHandler", () => { }) }) }) + +describe("writeTreeVersionFile", () => { + let tmpDir: tmp.DirectoryResult + let tmpPath: string + + beforeEach(async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + tmpPath = await realpath(tmpDir.path) + }) + + afterEach(async () => { + await tmpDir.cleanup() + }) + + describe("writeVersionFile", () => { + it("should write relative paths for files", async () => { + await writeTreeVersionFile(tmpPath, { + contentHash: "foo", + files: [ + join(tmpPath, "some", "file"), + ], + }) + expect(await readTreeVersionFile(join(tmpPath, GARDEN_VERSIONFILE_NAME))).to.eql({ + contentHash: "foo", + files: [ + "some/file", + ], + }) + }) + + it("should handle relative paths in input", async () => { + await writeTreeVersionFile(tmpPath, { + contentHash: "foo", + files: [ + "some/file", + ], + }) + expect(await readTreeVersionFile(join(tmpPath, GARDEN_VERSIONFILE_NAME))).to.eql({ + contentHash: "foo", + files: [ + "some/file", + ], + }) + }) + + it("should normalize Windows-style paths to POSIX-style", async () => { + await writeTreeVersionFile(tmpPath, { + contentHash: "foo", + files: [ + `some\\file`, + ], + }) + expect(await readTreeVersionFile(join(tmpPath, GARDEN_VERSIONFILE_NAME))).to.eql({ + contentHash: "foo", + files: [ + "some/file", + ], + }) + }) + }) +})