Remove recursivelyFindRoot and make cwd optional for commitChangesSinceBase (#118)

* Remove recursivelyFindRoot and make cwd optional

* Fix test

Author: bluwy

.changeset/kind-spoons-prove.md

@@ -0,0 +1,5 @@
+---
+"@changesets/ghcommit": major
+---
+
+Remove `recursivelyFindRoot` and make `cwd` optional for `commitChangesSinceBase`. Files will also not be filtered by `cwd` by default. To only commit files from a directory, use the `filterFiles` option instead.

src/git.ts

@@ -21,28 +21,21 @@ import { resolveGitRef } from "./utils.ts";
  * Works in Node.js only.
  */
 export async function commitChangesSinceBase({
-  cwd: workingDirectory,
-  recursivelyFindRoot = true,
+  cwd,
   filterFiles,
   ...otherArgs
 }: CommitChangesSinceBaseOptions): Promise<CommitChangesResult> {
-  const ref = resolveGitRef(otherArgs.base ?? { commit: "HEAD" });
-  const cwd = path.resolve(workingDirectory);
-  const repoRoot = recursivelyFindRoot ? await findGitRoot(cwd) : cwd;
+  cwd = path.resolve(cwd ?? process.cwd());
 
-  const refSha = await getShaForRef(repoRoot, ref);
+  const ref = resolveGitRef(otherArgs.base ?? { commit: "HEAD" });
+  const refSha = await getShaForRef(cwd, ref);
   if (!refSha) {
     throw new Error(`Could not determine sha for ref ${ref}`);
   }
 
   return await commitChanges({
     ...otherArgs,
-    fileChanges: await getFileChanges(
-      workingDirectory,
-      repoRoot,
-      refSha,
-      filterFiles,
-    ),
+    fileChanges: await getFileChanges(cwd, refSha, filterFiles),
     base: {
       commit: refSha,
     },
@@ -52,30 +45,17 @@ export async function commitChangesSinceBase({
 // Exported for testing only
 export async function getFileChanges(
   cwd: string,
-  repoRoot: string,
   ref: string,
   filterFiles?: CommitChangesSinceBaseOptions["filterFiles"],
 ): Promise<CommitChangesOptions["fileChanges"]> {
-  /**
-   * The directory to add files from. This is relative to the repository
-   * root, and is used to filter files.
-   */
-  const relativeStartDirectory =
-    cwd === repoRoot ? null : path.relative(repoRoot, cwd) + "/";
+  const repoRoot = await findGitRoot(cwd);
 
   const additions: CommitChangesOptions["fileChanges"]["additions"] = [];
   const deletions: CommitChangesOptions["fileChanges"]["deletions"] = [];
 
   const addPath = async (filePath: string) => {
-    if (
-      relativeStartDirectory &&
-      !filePath.startsWith(relativeStartDirectory)
-    ) {
-      return;
-    }
-    if (filterFiles && !filterFiles(filePath)) {
-      return;
-    }
+    if (filterFiles && !filterFiles(filePath)) return;
+
     const resolvedFilePath = path.join(repoRoot, filePath);
     const stat = await fs.lstat(resolvedFilePath);
     if (stat.isSymbolicLink()) {
@@ -97,15 +77,7 @@ export async function getFileChanges(
   };
 
   const deletePath = (filePath: string) => {
-    if (
-      relativeStartDirectory &&
-      !filePath.startsWith(relativeStartDirectory)
-    ) {
-      return;
-    }
-    if (filterFiles && !filterFiles(filePath)) {
-      return;
-    }
+    if (filterFiles && !filterFiles(filePath)) return;
 
     deletions.push({ path: filePath });
   };
@@ -171,7 +143,7 @@ async function findGitRoot(cwd: string): Promise<string> {
       throwOnError: true,
       nodeOptions: { cwd },
     });
-    return path.resolve(cwd, stdout.trim());
+    return path.dirname(path.resolve(cwd, stdout.trim()));
   } catch {
     return cwd;
   }

src/types.ts

@@ -93,21 +93,16 @@ export interface CommitChangesSinceBaseOptions extends Omit<
    */
   base?: GitRef;
   /**
-   * The directory to execute git commands in. And any changes outside of this
-   * directory (but within the repository) is ignored.
-   */
-  cwd: string;
-  /**
-   * Don't require {@link cwd} to be the root of the repository,
-   * and use it as a starting point to recursively search for the `.git`
-   * directory in parent directories.
+   * The directory to execute git commands in. This can be set if the repository
+   * is in a different directory.
    *
-   * @default true
+   * @default process.cwd()
    */
-  recursivelyFindRoot?: boolean;
+  cwd?: string;
   /**
-   * An optional function that can be used to filter which files are included
-   * in the commit. True should be returned for files that should be included.
+   * Filter the files to be included in the commit. The file paths are relative
+   * to the repository root, e.g. `"src/index.ts"`. Return `true` to include
+   * the file.
    *
    * By default, all files are included.
    */

tests/git.test.ts

@@ -44,7 +44,7 @@ describe("getFileChanges", () => {
     await fixture.writeFile("ignored/file.txt", "This file should be ignored");
     await fixture.writeFile(".env", "This file should be ignored");
 
-    const result = await getFileChanges(fixture.path, fixture.path, "HEAD");
+    const result = await getFileChanges(fixture.path, "HEAD");
     expect(result).toEqual({
       additions: [
         {
@@ -75,11 +75,7 @@ describe("getFileChanges", () => {
     });
     await fixture.writeFile("b.txt", "This is a new file!");
 
-    const result = await getFileChanges(
-      fixture.path,
-      fixture.path,
-      "refs/heads/new-branch",
-    );
+    const result = await getFileChanges(fixture.path, "refs/heads/new-branch");
     expect(result).toEqual({
       additions: [
         {
@@ -102,11 +98,7 @@ describe("getFileChanges", () => {
     });
     await fixture.writeFile("b.txt", "This is a new file!");
 
-    const result = await getFileChanges(
-      fixture.path,
-      fixture.path,
-      "refs/tags/v1.0.0",
-    );
+    const result = await getFileChanges(fixture.path, "refs/tags/v1.0.0");
     expect(result).toEqual({
       additions: [
         {
@@ -132,7 +124,7 @@ describe("getFileChanges", () => {
 
     await fixture.writeFile("b.txt", "This is a new file!");
 
-    const result = await getFileChanges(fixture.path, fixture.path, commitSha);
+    const result = await getFileChanges(fixture.path, commitSha);
     expect(result).toEqual({
       additions: [
         {
@@ -157,7 +149,6 @@ describe("getFileChanges", () => {
     await fixture.writeFile("nested/bar.txt", "This is a new file!");
 
     const result = await getFileChanges(
-      fixture.path,
       fixture.path,
       "HEAD",
       // Only include top-level files
@@ -174,34 +165,6 @@ describe("getFileChanges", () => {
     });
   });
 
-  it("should filter files when running in a repository sub-directory", async () => {
-    await using fixture = await createFixture({
-      "foo.txt": "Hello, world!",
-      "nested/foo.txt": "Hello, world!",
-    });
-    await setupGit(fixture.path);
-
-    await fixture.rm("foo.txt");
-    await fixture.rm("nested/foo.txt");
-    await fixture.writeFile("bar.txt", "This is a new file!");
-    await fixture.writeFile("nested/bar.txt", "This is a new file!");
-
-    const result = await getFileChanges(
-      path.join(fixture.path, "nested"),
-      fixture.path,
-      "HEAD",
-    );
-    expect(result).toEqual({
-      additions: [
-        {
-          path: "nested/bar.txt",
-          contents: await fixture.readFile("nested/bar.txt", "base64"),
-        },
-      ],
-      deletions: [{ path: "nested/foo.txt" }],
-    });
-  });
-
   it("should allow existing symlinks", async () => {
     await using fixture = await createFixture({
       "foo.txt": "Hello, world!",
@@ -220,7 +183,7 @@ describe("getFileChanges", () => {
     });
 
     // Since we committed, HEAD points to the last commit and there's no change since then
-    const result = await getFileChanges(fixture.path, fixture.path, "HEAD");
+    const result = await getFileChanges(fixture.path, "HEAD");
     expect(result).toEqual({ additions: [], deletions: [] });
 
     await fixture.rm("some-dir/nested");
@@ -230,9 +193,7 @@ describe("getFileChanges", () => {
     );
 
     // We made symlink changes since the last commit, so this should error now
-    await expect(
-      getFileChanges(fixture.path, fixture.path, "HEAD"),
-    ).rejects.toThrow(
+    await expect(getFileChanges(fixture.path, "HEAD")).rejects.toThrow(
       "Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore",
     );
   });
@@ -255,7 +216,7 @@ describe("getFileChanges", () => {
       fixture.getPath("some-dir/nested"),
     );
 
-    const result = await getFileChanges(fixture.path, fixture.path, "HEAD");
+    const result = await getFileChanges(fixture.path, "HEAD");
     expect(result).toEqual({ additions: [], deletions: [] });
   });
 
@@ -269,9 +230,7 @@ describe("getFileChanges", () => {
       fixture.getPath("some-dir/nested"),
     );
 
-    await expect(
-      getFileChanges(fixture.path, fixture.path, "HEAD"),
-    ).rejects.toThrow(
+    await expect(getFileChanges(fixture.path, "HEAD")).rejects.toThrow(
       "Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore",
     );
   });
@@ -288,9 +247,7 @@ describe("getFileChanges", () => {
       fixture.getPath("some-dir/nested"),
     );
 
-    await expect(
-      getFileChanges(fixture.path, fixture.path, "HEAD"),
-    ).rejects.toThrow(
+    await expect(getFileChanges(fixture.path, "HEAD")).rejects.toThrow(
       "Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore",
     );
   });
@@ -302,9 +259,7 @@ describe("getFileChanges", () => {
     await fixture.writeFile("executable-file.sh", "#!/bin/bash\necho hello");
     await fs.chmod(fixture.getPath("executable-file.sh"), 0o755);
 
-    await expect(
-      getFileChanges(fixture.path, fixture.path, "HEAD"),
-    ).rejects.toThrow(
+    await expect(getFileChanges(fixture.path, "HEAD")).rejects.toThrow(
       "Unexpected executable file at executable-file.sh, GitHub API only supports non-executable files and directories. You may need to add this file to .gitignore",
     );
   });