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

…5120) * improvement(git): don't require Garden `static` dir to be a Git repo --------- Co-authored-by: Jon Edvald <[email protected]> * chore: remove unnecessary `git init` calls for `static` dirs * chore: remove unnecessary git safe-dir checks for Garden `static` dir --------- Co-authored-by: Jon Edvald <[email protected]>
garden-io · Nov 9, 2023 · dc8ba20 · dc8ba20
1 parent 3637b34
commit dc8ba20
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 302 deletions.
diff --git a/cli/package.json b/cli/package.json
@@ -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",

diff --git a/cli/src/add-version-files.ts b/cli/src/add-version-files.ts
diff --git a/cli/src/build-pkg.ts b/cli/src/build-pkg.ts
@@ -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"))

diff --git a/core/src/constants.ts b/core/src/constants.ts
@@ -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 {

diff --git a/core/src/vcs/git.ts b/core/src/vcs/git.ts
@@ -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"
@@ -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"
@@ -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
@@ -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)
@@ -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 {

diff --git a/core/src/vcs/vcs.ts b/core/src/vcs/vcs.ts
@@ -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"
@@ -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-"
@@ -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() {
@@ -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
@@ -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

diff --git a/core/test/unit/src/vcs/git.ts b/core/test/unit/src/vcs/git.ts
@@ -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"
 
@@ -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