Repository: atlassian/changesets (ts)

Issue:
Detect insufficient commit history for changelog in shallow repo, and deepen clone
## Affected Packages

`@changesets/changelog-git`@0.1.6
`@changesets/cli`@2.11.2

## Problem

I've been trying to diagnose why, when I run `changeset version` locally I get changelog files with correct commit history, but when my Github action or other build tools run it, all the commits show the same (latest) commit id.

This is, obviously, because on my build tools I'm running with a 1-commit shallow clone. (See also https://github.com/atlassian/changesets/issues/379.)

The recommendation at the moment seems to be "run with --depth 0", i.e. "don't use shallow clones".  That doesn't really scale, unfortunately. We have some really large (20k, 30k+ commit) repos where it's really important that we use shallow clones for builds.

Running with a deeper - but fixed - shallow clone (e.g. `--depth 50`) is also not a great option: it's entirely likely that the commits mentioned in our changelog will be older than 50 commits ago.

Would it be difficult to automatically deepen the clone if it's found that the current depth is insufficient?  We have some code that automatically does this for some of our visual-regression baseline comparisons (e.g. if our baseline details are outside of the current clone depth).

## Proposed solution

I don't yet know much about the architecture of `changesets`.  I'd be happy helping out writing some code that does the automatic deepening, as we have some experience with that, but does that require support from the changelog APIs to indicate that the correct commit was not found?


Pull request:
Automatically deepen clones to find correct commits
This fixes #494 - where we would get the wrong commit
that added a file if we were operating in a shallow
repository that didn't contain the actual commit -
by modifying `getCommitThatAddsFile` so that it
automatically deepens a shallow clone until it finds the
_real_ commit that added a file.

Patch:
diff --git a/.changeset/six-roses-hammer.md b/.changeset/six-roses-hammer.md
new file mode 100644
index 0000000..20da1ff
--- /dev/null
+++ b/.changeset/six-roses-hammer.md
@@ -0,0 +1,5 @@
+---
+"@changesets/git": minor
+---
+
+Automatically deepen shallow clones in order to determine the correct commit at which changesets were added.
diff --git a/.changeset/some-other-changeset.md b/.changeset/some-other-changeset.md
new file mode 100644
index 0000000..64ffcea
--- /dev/null
+++ b/.changeset/some-other-changeset.md
@@ -0,0 +1,6 @@
+---
+"@changesets/git": minor
+---
+
+Deprecate the `getCommitThatAddsFile` function. It's replaced with a bulk `getCommitsThatAddFiles` operation which will safely deepen a
+shallow repo whilst processing multiple filenames simultaneously.
diff --git a/.changeset/yet-another-core-changeset.md b/.changeset/yet-another-core-changeset.md
new file mode 100644
index 0000000..bfdb3c7
--- /dev/null
+++ b/.changeset/yet-another-core-changeset.md
@@ -0,0 +1,5 @@
+---
+"@changesets/cli": minor
+---
+
+Automatically deepen shallow clones in order to determine the correct commit at which changesets were added. This helps Git-based changelog generators to always link to the correct commit. From now on it's not required to configure `fetch-depth: 0` for your `actions/checkout` when using [Changesets GitHub action](https://github.com/changesets/action).
diff --git a/packages/apply-release-plan/src/index.ts b/packages/apply-release-plan/src/index.ts
index 1efc437..5438cbc 100644
--- a/packages/apply-release-plan/src/index.ts
+++ b/packages/apply-release-plan/src/index.ts
@@ -20,21 +20,41 @@ import versionPackage from "./version-package";
 import createVersionCommit from "./createVersionCommit";
 import getChangelogEntry from "./get-changelog-entry";
 
-async function getCommitThatAddsChangeset(changesetId: string, cwd: string) {
-  let commit = await git.getCommitThatAddsFile(
-    `.changeset/${changesetId}.md`,
-    cwd
-  );
-  if (commit) {
-    return commit;
+function stringDefined(s: string | undefined): s is string {
+  return !!s;
+}
+async function getCommitsThatAddChangesets(
+  changesetIds: string[],
+  cwd: string
+) {
+  const paths = changesetIds.map(id => `.changeset/${id}.md`);
+  const commits = await git.getCommitsThatAddFiles(paths, cwd);
+
+  if (commits.every(stringDefined)) {
+    // We have commits for all files
+    return commits;
   }
-  let commitForOldChangeset = await git.getCommitThatAddsFile(
-    `.changeset/${changesetId}/changes.json`,
+
+  // Some files didn't exist. Try legacy filenames instead
+  const missingIds = changesetIds
+    .map((id, i) => (commits[i] ? undefined : id))
+    .filter(stringDefined);
+
+  const legacyPaths = missingIds.map(id => `.changeset/${id}/changes.json`);
+  const commitsForLegacyPaths = await git.getCommitsThatAddFiles(
+    legacyPaths,
     cwd
   );
-  if (commitForOldChangeset) {
-    return commitForOldChangeset;
-  }
+
+  // Fill in the blanks in the array of commits
+  changesetIds.forEach((id, i) => {
+    if (!commits[i]) {
+      const missingIndex = missingIds.indexOf(id);
+      commits[i] = commitsForLegacyPaths[missingIndex];
+    }
+  });
+
+  return commits;
 }
 
 export default async function applyReleasePlan(
@@ -200,12 +220,14 @@ async function getNewChangelogEntry(
     }
   }
 
-  let moddedChangesets = await Promise.all(
-    changesets.map(async cs => ({
-      ...cs,
-      commit: await getCommitThatAddsChangeset(cs.id, cwd)
-    }))
+  let commits = await getCommitsThatAddChangesets(
+    changesets.map(cs => cs.id),
+    cwd
   );
+  let moddedChangesets = changesets.map((cs, i) => ({
+    ...cs,
+    commit: commits[i]
+  }));
 
   return Promise.all(
     releasesWithPackage.map(async release => {
diff --git a/packages/git/package.json b/packages/git/package.json
index a598fc7..508d24c 100644
--- a/packages/git/package.json
+++ b/packages/git/package.json
@@ -15,6 +15,7 @@
     "spawndamnit": "^2.0.0"
   },
   "devDependencies": {
+    "file-url": "^3.0.0",
     "fixturez": "^1.1.0"
   }
 }
diff --git a/packages/git/src/index.ts b/packages/git/src/index.ts
index 0091ce2..92c5bbd 100644
--- a/packages/git/src/index.ts
+++ b/packages/git/src/index.ts
@@ -1,8 +1,10 @@
 import spawn from "spawndamnit";
+import fs from "fs";
 import path from "path";
 import { getPackages, Package } from "@manypkg/get-packages";
 import { GitError } from "@changesets/errors";
 import isSubdir from "is-subdir";
+import { deprecate } from "util";
 
 const isInDir = (dir: string) => (subdir: string) => isSubdir(dir, subdir);
 
@@ -43,13 +45,125 @@ export async function getDivergedCommit(cwd: string, ref: string) {
   return cmd.stdout.toString().trim();
 }
 
-async function getCommitThatAddsFile(gitPath: string, cwd: string) {
-  const gitCmd = await spawn(
-    "git",
-    ["log", "--diff-filter=A", "--max-count=1", "--pretty=format:%h", gitPath],
-    { cwd }
-  );
-  return gitCmd.stdout.toString();
+const getCommitThatAddsFile = deprecate(
+  async (gitPath: string, cwd: string) => {
+    return (await getCommitsThatAddFiles([gitPath], cwd))[0];
+  },
+  "Use the bulk getCommitsThatAddFiles function instead"
+);
+
+/**
+ * Get the short SHAs for the commits that added files, including automatically
+ * extending a shallow clone if necessary to determine any commits.
+ * @param gitPaths - Paths to fetch
+ * @param cwd - Location of the repository
+ */
+async function getCommitsThatAddFiles(
+  gitPaths: string[],
+  cwd: string
+): Promise<(string | undefined)[]> {
+  // Maps gitPath to short commit SHA
+  const map = new Map<string, string>();
+
+  // Paths we haven't completed processing on yet
+  let remaining = gitPaths;
+
+  do {
+    // Fetch commit information for all paths we don't have yet
+    const commitInfos = await Promise.all(remaining.map(findCommitAndParent));
+
+    // To collect commits without parents (usually because they're absent from
+    // a shallow clone).
+    let commitsWithMissingParents = [];
+
+    for (const info of commitInfos) {
+      if (info.commitSha) {
+        if (info.parentSha) {
+          // We have found the parent of the commit that added the file.
+          // Therefore we know that the commit is legitimate and isn't simply the boundary of a shallow clone.
+          map.set(info.path, info.commitSha);
+        } else {
+          commitsWithMissingParents.push(info);
+        }
+      } else {
+        // No commit for this file, which indicates it doesn't exist.
+      }
+    }
+
+    if (commitsWithMissingParents.length === 0) {
+      break;
+    }
+
+    // The commits we've found may be the real commits or they may be the boundary of
+    // a shallow clone.
+
+    // Can we deepen the clone?
+    if (await isRepoShallow()) {
+      // Yes.
+      await deepenCloneBy(50);
+      remaining = commitsWithMissingParents.map(p => p.path);
+    } else {
+      // It's not a shallow clone, so all the commit SHAs we have are legitimate.
+      for (const unresolved of commitsWithMissingParents) {
+        map.set(unresolved.path, unresolved.commitSha);
+      }
+      break;
+    }
+  } while (true);
+
+  return gitPaths.map(p => map.get(p));
+
+  /** Find the commit that added a file, and the parent of that commit */
+  async function findCommitAndParent(gitPath: string) {
+    const logResult = await spawn(
+      "git",
+      [
+        "log",
+        "--diff-filter=A",
+        "--max-count=1",
+        "--pretty=format:%h:%p",
+        gitPath
+      ],
+      { cwd }
+    );
+    const [commitSha, parentSha] = logResult.stdout.toString().split(":");
+    return { path: gitPath, commitSha, parentSha };
+  }
+
+  async function isRepoShallow() {
+    const gitCmd = await spawn(
+      "git",
+      ["rev-parse", "--is-shallow-repository"],
+      { cwd }
+    );
+
+    const isShallowRepoOutput = gitCmd.stdout.toString().trim();
+
+    if (isShallowRepoOutput === "--is-shallow-repository") {
+      // We have an old version of Git (<2.15) which doesn't support `rev-parse --is-shallow-repository`
+      // In that case, we'll test for the existence of .git/shallow.
+
+      // Firstly, find the .git folder for the repo; note that this will be relative to the repo dir
+      const gitDir = (
+        await spawn("git", ["rev-parse", "--git-dir"], { cwd })
+      ).stdout
+        .toString()
+        .trim();
+
+      const fullGitDir = path.resolve(cwd, gitDir);
+
+      // Check for the existence of <gitDir>/shallow
+      return fs.existsSync(path.join(fullGitDir, "shallow"));
+    } else {
+      // We have a newer Git which supports `rev-parse --is-shallow-repository`. We'll use
+      // the output of that instead of messing with .git/shallow in case that changes in the future.
+      return isShallowRepoOutput === "true";
+    }
+  }
+
+  async function deepenCloneBy(by: number) {
+    await spawn("git", ["fetch", `--deepen=${by}`], { cwd });
+  }
 }
 
 async function getChangedFilesSince({
@@ -140,6 +254,7 @@ async function getChangedPackagesSinceRef({
 
 export {
   getCommitThatAddsFile,
+  getCommitsThatAddFiles,
   getChangedFilesSince,
   add,
   commit,


Test patch:
diff --git a/packages/cli/src/commands/version/version.test.ts b/packages/cli/src/commands/version/version.test.ts
index ff5419c..2a40939 100644
--- a/packages/cli/src/commands/version/version.test.ts
+++ b/packages/cli/src/commands/version/version.test.ts
@@ -47,6 +47,8 @@ git.add.mockImplementation(() => Promise.resolve(true));
 // @ts-ignore
 git.commit.mockImplementation(() => Promise.resolve(true));
 // @ts-ignore
+git.getCommitsThatAddFiles.mockImplementation(() => Promise.resolve([]));
+// @ts-ignore
 git.tag.mockImplementation(() => Promise.resolve(true));
 
 const simpleChangeset: NewChangeset = {
diff --git a/packages/git/src/index.test.ts b/packages/git/src/index.test.ts
index dfb7652..84d3479 100644
--- a/packages/git/src/index.test.ts
+++ b/packages/git/src/index.test.ts
@@ -1,8 +1,10 @@
 import fixtures from "fixturez";
 import spawn from "spawndamnit";
+import fileUrl from "file-url";
 
 import {
   getCommitThatAddsFile,
+  getCommitsThatAddFiles,
   getChangedFilesSince,
   add,
   commit,
@@ -19,6 +21,16 @@ async function getCurrentCommit(cwd: string) {
   return cmd.stdout.toString().trim();
 }
 
+async function getCurrentCommitShort(cwd: string) {
+  const cmd = await spawn("git", ["rev-parse", "--short", "HEAD"], { cwd });
+  return cmd.stdout.toString().trim();
+}
+
+async function getCommitCount(cwd: string) {
+  const cmd = await spawn("git", ["rev-list", "--count", "HEAD"], { cwd });
+  return parseInt(cmd.stdout.toString(), 10);
+}
+
 describe("git", () => {
   let cwd: string;
   beforeEach(async () => {
@@ -185,20 +197,156 @@ describe("git", () => {
     });
   });
 
-  describe("getCommitThatAddsFile", () => {
+  describe("getCommitsThatAddFiles", () => {
     it("should commit a file and get the hash of that commit", async () => {
       await add("packages/pkg-b/package.json", cwd);
       await commit("added packageB package.json", cwd);
-      const head = await spawn("git", ["rev-parse", "--short", "HEAD"], {
+      const headSha = await getCurrentCommitShort(cwd);
+
+      const commitHash = await getCommitsThatAddFiles(
+        ["packages/pkg-b/package.json"],
         cwd
-      });
+      );
+
+      expect(commitHash).toEqual([headSha]);
+    });
+
+    // We have replaced the single-file version with a multi-file version
+    // which will correctly run all the file retrieves in parallel, safely
+    // deepening a shallow clone as necessary.  We've deprecated the
+    // single-file version and can remove it in a major release.
+    it("exposes a deprecated single-file call", async () => {
+      await add("packages/pkg-b/package.json", cwd);
+      await commit("added packageB package.json", cwd);
+      const headSha = await getCurrentCommitShort(cwd);
 
       const commitHash = await getCommitThatAddsFile(
         "packages/pkg-b/package.json",
         cwd
       );
 
-      expect(commitHash).toEqual(head.stdout.toString().trim());
+      expect(commitHash).toEqual(headSha);
+    });
+
+    describe("with shallow clone", () => {
+      // We will add these three well-known files
+      // over multiple commits, then test looking up
+      // the commits at which they were added.
+      const file1 = ".changeset/quick-lions-devour.md";
+      const file2 = "packages/pkg-a/index.js";
+      const file3 = "packages/pkg-b/index.js";
+
+      // Roughly how many commits will the deepening algorithm
+      // deepen each time?  We use this to set up test data to
+      // check that the deepens the clone but doesn't need to *fully* unshallow
+      // the clone.
+      const shallowCloneDeepeningAmount = 50;
+
+      /**
+       * Creates a number of empty commits; this is useful to ensure
+       * that a particular commit doesn't make it into a shallow clone.
+       */
+      async function createDummyCommits(count: number) {
+        for (let i = 0; i < count; i++) {
+          await commit("dummy commit", cwd);
+        }
+      }
+
+      async function addFileAndCommit(file: string) {
+        await add(file, cwd);
+        await commit(`add file ${file}`, cwd);
+        const commitSha = await getCurrentCommitShort(cwd);
+        return commitSha;
+      }
+
+      async function createShallowClone(depth: number): Promise<string> {
+        // Make a 1-commit-deep shallow clone of this repo
+        const cloneDir = f.temp();
+        await spawn(
+          "git",
+          // Note: a file:// URL is needed in order to make a shallow clone of
+          // a local repo
+          ["clone", "--depth", depth.toString(), fileUrl(cwd), "."],
+          {
+            cwd: cloneDir
+          }
+        );
+        return cloneDir;
+      }
+
+      it("reads the SHA of a file-add without deepening if commit already included in the shallow clone", async () => {
+        // We create a repo that we shallow-clone;
+        // the commit we're going to scan for is the latest commit,
+        // so will be in the shallow clone immediately without deepening
+        await createDummyCommits(10);
+        const originalCommit = await addFileAndCommit(file1);
+
+        const clone = await createShallowClone(5);
+
+        // This file was added in the head commit, so will definitely be in our
+        // 1-commit clone.
+        const commits = await getCommitsThatAddFiles([file1], clone);
+        expect(commits).toEqual([originalCommit]);
+
+        // We should not need to have deepened the clone for this
+        expect(await getCommitCount(clone)).toEqual(5);
+      });
+
+      it("reads the SHA of a file-add even if not already included in the shallow clone", async () => {
+        // We're going to create a repo where the commit we're looking for isn't
+        // in the shallow clone, so we'll need to deepen it to locate it.
+        await createDummyCommits((shallowCloneDeepeningAmount * 2) / 3);
+        const originalCommit = await addFileAndCommit(file2);
+        await createDummyCommits((shallowCloneDeepeningAmount * 2) / 3);
+
+        const clone = await createShallowClone(5);
+
+        // Finding this commit will require deepening the clone until it appears.
+        const commit = await getCommitThatAddsFile(file2, clone);
+        expect(commit).toEqual(originalCommit);
+
+        // It should not have completely unshallowed the clone; just enough.
+        const originalRepoDepth = await getCommitCount(cwd);
+        expect(await getCommitCount(clone)).toBeGreaterThan(5);
+        expect(await getCommitCount(clone)).toBeLessThan(originalRepoDepth);
+      });
+
+      it("reads the SHA of a file-add even if the first commit of a repo", async () => {
+        // Finding this commit will require deepening the clone right to the start
+        // of the repo history, and coping with a commit that has no parent.
+        const originalCommit = await addFileAndCommit(file3);
+        await createDummyCommits(shallowCloneDeepeningAmount * 2);
+        const clone = await createShallowClone(5);
+
+        // Finding this commit will require fully deepening the repo
+        const commit = await getCommitThatAddsFile(file3, clone);
+        expect(commit).toEqual(originalCommit);
+
+        // We should have fully deepened
+        const originalRepoDepth = await getCommitCount(cwd);
+        expect(await getCommitCount(clone)).toEqual(originalRepoDepth);
+      });
+
+      it("can return SHAs for multiple files including return blanks for missing files", async () => {
+        // We want to ensure that we can retrieve SHAs for multiple files at the same time,
+        // and also that requesting missing files doesn't affect the location of commits
+        // for the files that succeed.
+        await createDummyCommits(shallowCloneDeepeningAmount);
+        const originalCommit1 = await addFileAndCommit(file1);
+        await createDummyCommits(shallowCloneDeepeningAmount);
+        const originalCommit2 = await addFileAndCommit(file2);
+
+        const nonExistentFile = "this-file-does-not-exist";
+
+        const clone = await createShallowClone(5);
+
+        const commits = await getCommitsThatAddFiles(
+          [file1, nonExistentFile, file2],
+          clone
+        );
+
+        expect(commits).toEqual([originalCommit1, undefined, originalCommit2]);
+      });
     });
   });