From 3679529bef2c23c6a61af06a56d9164327809e1c Mon Sep 17 00:00:00 2001 From: William Chargin Date: Thu, 26 Apr 2018 19:31:27 -0700 Subject: [PATCH] Move `localGit`/`GitDriver` into Git utils (#150) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: A few reasons for this: 1. This _is_ a utility, so it makes sense semantically. 2. This unifies the utilities API; clients like `loadRepository.test` don’t have to keep around both a `git` and a `gitUtils`. 3. Most importantly, further scripts and tests shouldn’t depend on `loadRepository` just for `localGit`. Depending on `gitUtils` makes much more sense. (Note that `makeUtils` is no longer dependency-injectable, but that’s okay; I considered this and favored YAGNI on this one.) Test Plan: Existing unit tests pass. wchargin-branch: move-localgit --- src/plugins/git/gitUtils.js | 26 +++++++++++--- src/plugins/git/loadRepository.js | 17 ++-------- src/plugins/git/loadRepository.test.js | 47 ++++++++++++-------------- 3 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/plugins/git/gitUtils.js b/src/plugins/git/gitUtils.js index eb6f4fa..e73b17b 100644 --- a/src/plugins/git/gitUtils.js +++ b/src/plugins/git/gitUtils.js @@ -1,19 +1,37 @@ // @flow +import {execFileSync} from "child_process"; import fs from "fs"; import mkdirp from "mkdirp"; import path from "path"; -import type {GitDriver} from "./loadRepository"; - -interface Utils { +export interface Utils { + exec: GitDriver; head(): string; writeAndStage(filename: string, contents: string): void; deterministicCommit(message: string): void; } -export function makeUtils(git: GitDriver, repositoryPath: string): Utils { +export type GitDriver = (args: string[], options?: ExecOptions) => string; +// `ExecOptions` is the type of the second argument to `execFileSync`. +// See here for details: https://nodejs.org/api/child_process.html +type ExecOptions = Object; + +export function localGit(repositoryPath: string): GitDriver { + return function git(args: string[], options?: ExecOptions): string { + return execFileSync( + "git", + ["-C", repositoryPath, ...args], + options + ).toString(); + }; +} + +export function makeUtils(repositoryPath: string): Utils { + const git = localGit(repositoryPath); return { + exec: git, + head() { return git(["rev-parse", "HEAD"]).trim(); }, diff --git a/src/plugins/git/loadRepository.js b/src/plugins/git/loadRepository.js index d4477c1..33a7317 100644 --- a/src/plugins/git/loadRepository.js +++ b/src/plugins/git/loadRepository.js @@ -9,22 +9,9 @@ */ // @flow -import {execFileSync} from "child_process"; - +import type {GitDriver} from "./gitUtils"; import type {Repository, Hash, Commit, Tree, TreeEntry} from "./types"; - -export type GitDriver = (args: string[], options?: ExecOptions) => string; -type ExecOptions = Object; // close enough -export function localGit(repositoryPath: string): GitDriver { - return function git(args: string[], options?: ExecOptions): string { - // Throws an Error on shell failure. - return execFileSync( - "git", - ["-C", repositoryPath, ...args], - options - ).toString(); - }; -} +import {localGit} from "./gitUtils"; /** * Load a Git repository from disk into memory. The `rootRef` should be diff --git a/src/plugins/git/loadRepository.test.js b/src/plugins/git/loadRepository.test.js index d1638c9..dbf7455 100644 --- a/src/plugins/git/loadRepository.test.js +++ b/src/plugins/git/loadRepository.test.js @@ -2,9 +2,8 @@ import tmp from "tmp"; -import type {GitDriver} from "./loadRepository"; import {makeUtils} from "./gitUtils"; -import {localGit, loadRepository} from "./loadRepository"; +import {loadRepository} from "./loadRepository"; const cleanups: (() => void)[] = []; afterAll(() => { @@ -21,34 +20,33 @@ function mkdtemp() { function createRepository(): {path: string, commits: string[]} { const repositoryPath = mkdtemp(); - const git = localGit(repositoryPath); - const gitUtils = makeUtils(git, repositoryPath); + const git = makeUtils(repositoryPath); - git(["init"]); + git.exec(["init"]); - gitUtils.writeAndStage("README.txt", "Amazing physics going on...\n"); - gitUtils.deterministicCommit("Initial commit"); - const commit1 = gitUtils.head(); + git.writeAndStage("README.txt", "Amazing physics going on...\n"); + git.deterministicCommit("Initial commit"); + const commit1 = git.head(); - gitUtils.writeAndStage("src/index.py", "import antigravity\n"); - gitUtils.writeAndStage( + git.writeAndStage("src/index.py", "import antigravity\n"); + git.writeAndStage( "src/quantum_gravity.py", 'raise NotImplementedError("TODO(physicists)")\n' ); - gitUtils.writeAndStage("TODOS.txt", "1. Resolve quantum gravity\n"); - gitUtils.deterministicCommit("Discover gravity"); - const commit2 = gitUtils.head(); + git.writeAndStage("TODOS.txt", "1. Resolve quantum gravity\n"); + git.deterministicCommit("Discover gravity"); + const commit2 = git.head(); - gitUtils.writeAndStage( + git.writeAndStage( "src/quantum_gravity.py", "import random\nif random.random() < 0.5:\n import antigravity\n" ); - gitUtils.deterministicCommit("Solve quantum gravity"); - const commit3 = gitUtils.head(); + git.deterministicCommit("Solve quantum gravity"); + const commit3 = git.head(); - git(["rm", "TODOS.txt"]); - gitUtils.deterministicCommit("Clean up TODOS"); - const commit4 = gitUtils.head(); + git.exec(["rm", "TODOS.txt"]); + git.deterministicCommit("Clean up TODOS"); + const commit4 = git.head(); return { path: repositoryPath, @@ -88,16 +86,15 @@ describe("loadRepository", () => { it("works with submodules", () => { const repositoryPath = mkdtemp(); - const git = localGit(repositoryPath); - const gitUtils = makeUtils(git, repositoryPath); + const git = makeUtils(repositoryPath); const subproject = createRepository(); - git(["init"]); - git(["submodule", "--quiet", "add", subproject.path, "physics"]); - gitUtils.deterministicCommit("Initial commit"); + git.exec(["init"]); + git.exec(["submodule", "--quiet", "add", subproject.path, "physics"]); + git.deterministicCommit("Initial commit"); - const head = gitUtils.head(); + const head = git.head(); const repository = loadRepository(repositoryPath, "HEAD"); const commit = repository.commits.get(head);