Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improvement(git): don't require Garden static dir to be a Git repo #5120

Merged
merged 3 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@
"typescript": "^5.1.3"
},
"scripts": {
"add-version-files": "node build/src/add-version-files.js",
"build": "tsc --build . --verbose && npm run add-version-files && npm run generate-docs",
"build": "tsc --build . --verbose && npm run generate-docs",
"check-package-lock": "git diff-index --quiet HEAD -- package-lock.json || (echo 'package-lock.json is dirty!' && exit 1)",
"clean": "shx rm -rf build dist",
"fix-format": "npm run lint -- --fix --quiet",
Expand Down
58 changes: 0 additions & 58 deletions cli/src/add-version-files.ts

This file was deleted.

1 change: 0 additions & 1 deletion cli/src/build-pkg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ async function buildBinaries(args: string[]) {
// Copy static dir, stripping out undesired files for the dist build
console.log(chalk.cyan("Copying static directory"))
await exec("rsync", ["-r", "-L", "--exclude=.garden", "--exclude=.git", STATIC_DIR, distTmpDir])
await exec("git", ["init"], { cwd: tmpStaticDir })

// Copy each package to the temp dir
console.log(chalk.cyan("Getting package info"))
Expand Down
1 change: 0 additions & 1 deletion core/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export const MUTAGEN_DIR_NAME = "mutagen"
export const LOGS_DIR_NAME = "logs"
export const GARDEN_GLOBAL_PATH = join(homedir(), DEFAULT_GARDEN_DIR_NAME)
export const ERROR_LOG_FILENAME = "error.log"
export const GARDEN_VERSIONFILE_NAME = ".garden-version"
export const DEFAULT_PORT_PROTOCOL = "TCP"

export enum GardenApiVersion {
Expand Down
110 changes: 1 addition & 109 deletions core/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { performance } from "perf_hooks"
import { isAbsolute, join, posix, relative, resolve } from "path"
import { isString } from "lodash-es"
import fsExtra from "fs-extra"

const { createReadStream, ensureDir, lstat, pathExists, readlink, realpath, stat } = fsExtra
import { PassThrough } from "stream"
import type { GetFilesParams, RemoteSourceParams, VcsFile, VcsInfo, VcsHandlerParams } from "./vcs.js"
Expand All @@ -22,7 +23,6 @@ import type { Log } from "../logger/log-entry.js"
import parseGitConfig from "parse-git-config"
import type { Profiler } from "../util/profiling.js"
import { getDefaultProfiler, Profile } from "../util/profiling.js"
import { STATIC_DIR } from "../constants.js"
import isGlob from "is-glob"
import chalk from "chalk"
import { pMemoizeDecorator } from "../lib/p-memoize.js"
Expand All @@ -34,14 +34,7 @@ import type { ExecaError } from "execa"
import { execa } from "execa"
import hasha from "hasha"

const gitConfigAsyncLock = new AsyncLock()

const submoduleErrorSuggestion = `Perhaps you need to run ${chalk.underline(`git submodule update --recursive`)}?`
const currentPlatformName = process.platform

const gitSafeDirs = new Set<string>()
let gitSafeDirsRead = false
let staticDirSafe = false

interface GitEntry extends VcsFile {
mode: string
Expand Down Expand Up @@ -119,101 +112,6 @@ export class GitHandler extends VcsHandler {
}
}

toGitConfigCompatiblePath(path: string, platformName: string): string {
// Windows paths require some pre-processing,
// see the full list of platform names here: https://nodejs.org/api/process.html#process_process_platform
if (platformName !== "win32") {
return path
}

// Replace back-slashes with forward-slashes to make paths compatible with .gitconfig in Windows
return path.replace(/\\/g, "/")
}

// TODO-0.13.1+ - get rid of this in/after https://github.com/garden-io/garden/pull/4047
/**
* Checks if a given {@code path} is a valid and safe Git repository.
* If it is a valid Git repository owned by another user,
* then the static dir will be added to the list of safe directories in .gitconfig.
*
* Git has stricter repository ownerships checks since 2.36.0,
* see https://github.blog/2022-04-18-highlights-from-git-2-36/ for more details.
*/
private async ensureSafeDirGitRepo(log: Log, path: string, failOnPrompt = false): Promise<void> {
if (gitSafeDirs.has(path)) {
return
}

// Avoid multiple concurrent checks on the same path
await this.lock.acquire(`safe-dir:${path}`, async () => {
if (gitSafeDirs.has(path)) {
return
}

const git = this.gitCli(log, path, failOnPrompt)

if (!gitSafeDirsRead) {
await gitConfigAsyncLock.acquire(".gitconfig", async () => {
if (!gitSafeDirsRead) {
const gitCli = this.gitCli(log, path, failOnPrompt)
try {
const safeDirectories = await gitCli("config", "--get-all", "safe.directory")
safeDirectories.forEach((safeDir) => gitSafeDirs.add(safeDir))
} catch (err) {
// ignore the error if there are no safe directories defined
log.debug(`Error reading safe directories from the .gitconfig: ${err}`)
}
gitSafeDirsRead = true
}
})
}

try {
await git("status")
gitSafeDirs.add(path)
} catch (err) {
if (!(err instanceof ChildProcessError)) {
throw err
}

// Git has stricter repo ownerships checks since 2.36.0
if (err.details.code === 128 && err.details.stderr.toLowerCase().includes("fatal: unsafe repository")) {
log.warn(
chalk.yellow(
`It looks like you're using Git 2.36.0 or newer and the directory "${path}" is owned by someone else. It will be added to safe.directory list in the .gitconfig.`
)
)

if (!gitSafeDirs.has(path)) {
await gitConfigAsyncLock.acquire(".gitconfig", async () => {
if (!gitSafeDirs.has(path)) {
const gitConfigCompatiblePath = this.toGitConfigCompatiblePath(path, currentPlatformName)
// Add the safe directory globally to be able to run git command outside a (trusted) git repo
// Wrap the path in quotes to pass it as a single argument in case if it contains any whitespaces
await git("config", "--global", "--add", "safe.directory", `'${gitConfigCompatiblePath}'`)
gitSafeDirs.add(path)
log.debug(`Configured git to trust repository in ${path}`)
}
})
}

return
} else if (
err.details.code === 128 &&
err.details.stderr.toLowerCase().includes("fatal: not a git repository")
) {
throw new RuntimeError({ message: notInRepoRootErrorMessage(path) })
} else {
log.error(
`Unexpected Git error occurred while running 'git status' from path "${path}". Exit code: ${err.details.code}. Error message: ${err.details.stderr}`
)
throw err
}
}
gitSafeDirs.add(path)
})
}

async getRepoRoot(log: Log, path: string, failOnPrompt = false) {
if (this.repoRoots.has(path)) {
return this.repoRoots.get(path)
Expand All @@ -225,12 +123,6 @@ export class GitHandler extends VcsHandler {
return this.repoRoots.get(path)
}

// TODO-0.13.1+ - get rid of this in/after https://github.com/garden-io/garden/pull/4047
if (!staticDirSafe) {
staticDirSafe = true
await this.ensureSafeDirGitRepo(log, STATIC_DIR, failOnPrompt)
}

const git = this.gitCli(log, path, failOnPrompt)

try {
Expand Down
68 changes: 9 additions & 59 deletions core/src/vcs/vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,18 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import type Joi from "@hapi/joi"
import normalize from "normalize-path"
import { sortBy, pick } from "lodash-es"
import { createHash } from "node:crypto"
import { validateSchema } from "../config/validation.js"
import { join, relative, isAbsolute, sep } from "path"
import { DOCS_BASE_URL, GARDEN_VERSIONFILE_NAME as GARDEN_TREEVERSION_FILENAME } from "../constants.js"
import { relative, sep } from "path"
import { DOCS_BASE_URL } from "../constants.js"
import fsExtra from "fs-extra"
const { pathExists, readFile, writeFile } = fsExtra
import { ConfigurationError } from "../exceptions.js"

const { writeFile } = fsExtra
import type { ExternalSourceType } from "../util/ext-source-util.js"
import { getRemoteSourceLocalPath, getRemoteSourcesPath } from "../util/ext-source-util.js"
import type { ModuleConfig } from "../config/module.js"
import { serializeConfig } from "../config/module.js"
import type { Log } from "../logger/log-entry.js"
import { treeVersionSchema } from "../config/common.js"
import { dedent, splitLast } from "../util/string.js"
import { fixedProjectExcludes } from "../util/fs.js"
import type { TreeCache } from "../cache.js"
Expand All @@ -38,6 +34,7 @@ import chalk from "chalk"
import { Profile } from "../util/profiling.js"

import AsyncLock from "async-lock"

const scanLock = new AsyncLock()

export const versionStringPrefix = "v-"
Expand Down Expand Up @@ -164,9 +161,13 @@ export abstract class VcsHandler {
abstract name: string

abstract getRepoRoot(log: Log, path: string): Promise<string>

abstract getFiles(params: GetFilesParams): Promise<VcsFile[]>

abstract ensureRemoteSource(params: RemoteSourceParams): Promise<string>

abstract updateRemoteSource(params: RemoteSourceParams): Promise<void>

abstract getPathInfo(log: Log, path: string): Promise<VcsInfo>

clearTreeCache() {
Expand Down Expand Up @@ -266,14 +267,6 @@ export abstract class VcsHandler {
this.cache.invalidateUp(log, pathToCacheContext(path))
}

async resolveTreeVersion(params: GetTreeVersionParams): Promise<TreeVersion> {
// the version file is used internally to specify versions outside of source control
const path = getSourcePath(params.config)
const versionFilePath = join(path, GARDEN_TREEVERSION_FILENAME)
const fileVersion = await readTreeVersionFile(versionFilePath)
return fileVersion || (await this.getTreeVersion(params))
}

/**
* Returns a map of the optimal paths for each of the given action/module source path.
* This is used to avoid scanning more of each git repository than necessary, and
Expand Down Expand Up @@ -346,49 +339,6 @@ export abstract class VcsHandler {
}
}

async function readVersionFile(path: string, schema: Joi.Schema): Promise<any> {
if (!(await pathExists(path))) {
return null
}

// this is used internally to specify version outside of source control
const versionFileContents = (await readFile(path)).toString().trim()

if (!versionFileContents) {
return null
}

try {
return validateSchema(JSON.parse(versionFileContents), schema)
} catch (error) {
throw new ConfigurationError({
message: `Unable to parse ${path} as valid version file: ${error}`,
})
}
}

export async function readTreeVersionFile(path: string): Promise<TreeVersion | null> {
return readVersionFile(path, treeVersionSchema())
}

/**
* 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")
}

/**
* We prefix with "v-" to prevent this.version from being read as a number when only a prefix of the
* commit hash is used, and that prefix consists of only numbers. This can cause errors in certain contexts
Expand Down
19 changes: 1 addition & 18 deletions core/test/unit/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { execa } from "execa"
import { expect } from "chai"
import tmp from "tmp-promise"
import fsExtra from "fs-extra"

const { createFile, ensureSymlink, lstat, mkdir, mkdirp, realpath, remove, symlink, writeFile } = fsExtra
import { basename, join, relative, resolve } from "path"

Expand Down Expand Up @@ -609,24 +610,6 @@ export const commonGitHandlerTests = (handlerCls: new (params: VcsHandlerParams)
})
})

describe("toGitConfigCompatiblePath", () => {
it("should return an unmodified path in Linux", async () => {
const path = "/home/user/repo"
expect(handler.toGitConfigCompatiblePath(path, "linux")).to.equal(path)
})

it("should return an unmodified path in macOS", async () => {
const path = "/Users/user/repo"
expect(handler.toGitConfigCompatiblePath(path, "darwin")).to.equal(path)
})

it("should return a modified and corrected path in Windows", async () => {
const path = "C:\\Users\\user\\repo"
const expectedPath = "C:/Users/user/repo"
expect(handler.toGitConfigCompatiblePath(path, "win32")).to.equal(expectedPath)
})
})

describe("getRepoRoot", () => {
it("should return the repo root if it is the same as the given path", async () => {
const path = tmpPath
Expand Down