diff --git a/src/git.ts b/src/git.ts index a6e2a80..1306a9b 100644 --- a/src/git.ts +++ b/src/git.ts @@ -74,6 +74,12 @@ export const commitChangesFromRepo = async ({ ) { return null; } + const prevOid = await commit?.oid(); + const currentOid = await workdir?.oid(); + // Don't include files that haven't changed, and exist in both trees + if (prevOid === currentOid && !commit === !workdir) { + return null; + } if ( (await commit?.mode()) === FILE_MODES.symlink || (await workdir?.mode()) === FILE_MODES.symlink @@ -87,12 +93,6 @@ export const commitChangesFromRepo = async ({ `Unexpected executable file at ${filepath}, GitHub API only supports non-executable files and directories. You may need to add this file to .gitignore`, ); } - const prevOid = await commit?.oid(); - const currentOid = await workdir?.oid(); - // Don't include files that haven't changed, and exist in both trees - if (prevOid === currentOid && !commit === !workdir) { - return null; - } // Iterate through anything that may be a directory in either the // current commit or the working directory if ( diff --git a/src/test/integration/git.test.ts b/src/test/integration/git.test.ts index 2e46c60..3875050 100644 --- a/src/test/integration/git.test.ts +++ b/src/test/integration/git.test.ts @@ -85,7 +85,9 @@ const makeFileChanges = async ( | "with-executable-file" | "with-ignored-symlink" | "with-included-valid-symlink" - | "with-included-invalid-symlink", + | "with-included-invalid-symlink" + | "with-unchanged-symlink" + | "with-changed-symlink", ) => { // Update an existing file await fs.promises.writeFile( @@ -164,6 +166,40 @@ const makeFileChanges = async ( path.join(repoDirectory, "some-dir", "nested"), ); } + if ( + changegroup === "with-unchanged-symlink" || + changegroup === "with-changed-symlink" + ) { + // Create a symlink and commit it locally first + await fs.promises.mkdir(path.join(repoDirectory, "some-dir"), { + recursive: true, + }); + await fs.promises.symlink( + path.join(repoDirectory, "README.md"), + path.join(repoDirectory, "some-dir", "nested"), + ); + // Commit the symlink locally so it's in the base commit tree + await git.add({ + fs, + dir: repoDirectory, + filepath: "some-dir/nested", + }); + await git.commit({ + fs, + dir: repoDirectory, + message: "Add symlink", + author: { name: "Test", email: "test@test.com" }, + }); + + if (changegroup === "with-changed-symlink") { + // Change the symlink to point to a different target + await fs.promises.rm(path.join(repoDirectory, "some-dir", "nested")); + await fs.promises.symlink( + path.join(repoDirectory, "LICENSE"), + path.join(repoDirectory, "some-dir", "nested"), + ); + } + } }; const makeFileChangeAssertions = async (branch: string) => { @@ -324,6 +360,62 @@ describe("git", () => { }); } + it(`should allow unchanged symlinks without throwing symlink error`, async () => { + const branch = `${TEST_BRANCH_PREFIX}-multiple-changes-with-unchanged-symlink`; + branches.push(branch); + + await fs.promises.mkdir(testDir, { recursive: true }); + const repoDirectory = path.join(testDir, `repo-1-with-unchanged-symlink`); + + // Clone the git repo locally using the git cli and child-process + await new Promise((resolve, reject) => { + const p = execFile( + "git", + ["clone", process.cwd(), `repo-1-with-unchanged-symlink`], + { cwd: testDir }, + (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }, + ); + p.stdout?.pipe(process.stdout); + p.stderr?.pipe(process.stderr); + }); + + await makeFileChanges(repoDirectory, "with-unchanged-symlink"); + + // The symlink was committed locally and is unchanged in workdir. + // When using local HEAD as base, the symlink should be detected as + // unchanged and skipped (no symlink error thrown during tree walk). + // The actual GitHub push may fail because the local commit doesn't + // exist on GitHub, but that's OK - we're testing the tree walk logic. + try { + await commitChangesFromRepo({ + octokit, + ...REPO, + branch, + message: { + headline: "Test commit", + body: "This is a test commit", + }, + cwd: repoDirectory, + log, + }); + + // If push somehow succeeded, verify the file changes + await waitForGitHubToBeReady(); + await makeFileChangeAssertions(branch); + } catch (error) { + // The error should NOT be about symlinks - that would mean tree walk + // failed to skip the unchanged symlink + expect((error as Error).message).not.toContain("Unexpected symlink"); + expect((error as Error).message).not.toContain("Unexpected executable"); + } + }); + describe(`should throw appropriate error when symlink is present`, () => { it(`and file does not exist`, async () => { const branch = `${TEST_BRANCH_PREFIX}-invalid-symlink-error`; @@ -414,6 +506,51 @@ describe("git", () => { "Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore", ); }); + + it(`and symlink was changed`, async () => { + const branch = `${TEST_BRANCH_PREFIX}-changed-symlink-error`; + branches.push(branch); + + await fs.promises.mkdir(testDir, { recursive: true }); + const repoDirectory = path.join(testDir, `repo-changed-symlink`); + + // Clone the git repo locally using the git cli and child-process + await new Promise((resolve, reject) => { + const p = execFile( + "git", + ["clone", process.cwd(), `repo-changed-symlink`], + { cwd: testDir }, + (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }, + ); + p.stdout?.pipe(process.stdout); + p.stderr?.pipe(process.stderr); + }); + + await makeFileChanges(repoDirectory, "with-changed-symlink"); + + // Push the changes + await expect(() => + commitChangesFromRepo({ + octokit, + ...REPO, + branch, + message: { + headline: "Test commit", + body: "This is a test commit", + }, + cwd: repoDirectory, + log, + }), + ).rejects.toThrow( + "Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore", + ); + }); }); it(`should throw appropriate error when executable file is present`, async () => {