From 7edad00f11a682fbffa6636f3c7a1300b6d79e41 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 20 Mar 2022 20:15:10 +0530 Subject: [PATCH 1/4] test,fs: add fs.rm() tests for .git directories Git for Windows creates read-only files inside the .git directory. fs.rm() was built in a way, to work around any EPERM error that can happen while deleting the .git directory by turning the directory into a writable one. This change adds a regression test for that. Refs: https://github.com/isaacs/rimraf/issues/21 Refs: https://github.com/nodejs/node/pull/38810#issuecomment-1073152001 Signed-off-by: Darshan Sen --- test/parallel/test-fs-rm.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index e0e3fd88544fff..d8e24d7e98e121 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -130,6 +130,17 @@ function removeAsync(dir) { })); } +// Removing a .git directory should not throw an EPERM. +// Refs: https://github.com/isaacs/rimraf/issues/21. +{ + const gitDirectory = nextDirPath(); + fs.mkdirSync(gitDirectory); + execSync(`git -C ${gitDirectory} init`); + fs.rm(gitDirectory, { recursive: true }, common.mustSucceed(() => { + assert.strictEqual(fs.existsSync(gitDirectory), false); + })); +} + // Test the synchronous version. { const dir = nextDirPath(); @@ -179,6 +190,16 @@ function removeAsync(dir) { assert.throws(() => fs.rmSync(dir), { syscall: 'stat' }); } +// Removing a .git directory should not throw an EPERM. +// Refs: https://github.com/isaacs/rimraf/issues/21. +{ + const gitDirectory = nextDirPath(); + fs.mkdirSync(gitDirectory); + execSync(`git -C ${gitDirectory} init`); + fs.rmSync(gitDirectory, { recursive: true }); + assert.strictEqual(fs.existsSync(gitDirectory), false); +} + // Test the Promises based version. (async () => { const dir = nextDirPath(); @@ -230,6 +251,16 @@ function removeAsync(dir) { } })().then(common.mustCall()); +// Removing a .git directory should not throw an EPERM. +// Refs: https://github.com/isaacs/rimraf/issues/21. +(async () => { + const gitDirectory = nextDirPath(); + fs.mkdirSync(gitDirectory); + execSync(`git -C ${gitDirectory} init`); + await fs.promises.rm(gitDirectory, { recursive: true }); + assert.strictEqual(fs.existsSync(gitDirectory), false); +})().then(common.mustCall()); + // Test input validation. { const dir = nextDirPath(); From b76ef18fae205f4dcdad5f206ce3c7ebbd3275f3 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Mon, 21 Mar 2022 19:32:32 +0530 Subject: [PATCH 2/4] test: check if git is present before running the tests Signed-off-by: Darshan Sen --- test/parallel/test-fs-rm.js | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index d8e24d7e98e121..930b4f97d36f05 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -16,6 +16,10 @@ let count = 0; const nextDirPath = (name = 'rm') => path.join(tmpdir.path, `${name}-${count++}`); +const isGitPresent = (() => { + try { execSync('git --version'); return true; } catch { return false; } +})(); + function makeNonEmptyDirectory(depth, files, folders, dirname, createSymLinks) { fs.mkdirSync(dirname, { recursive: true }); fs.writeFileSync(path.join(dirname, 'text.txt'), 'hello', 'utf8'); @@ -132,7 +136,7 @@ function removeAsync(dir) { // Removing a .git directory should not throw an EPERM. // Refs: https://github.com/isaacs/rimraf/issues/21. -{ +if (isGitPresent) { const gitDirectory = nextDirPath(); fs.mkdirSync(gitDirectory); execSync(`git -C ${gitDirectory} init`); @@ -192,7 +196,7 @@ function removeAsync(dir) { // Removing a .git directory should not throw an EPERM. // Refs: https://github.com/isaacs/rimraf/issues/21. -{ +if (isGitPresent) { const gitDirectory = nextDirPath(); fs.mkdirSync(gitDirectory); execSync(`git -C ${gitDirectory} init`); @@ -253,13 +257,15 @@ function removeAsync(dir) { // Removing a .git directory should not throw an EPERM. // Refs: https://github.com/isaacs/rimraf/issues/21. -(async () => { - const gitDirectory = nextDirPath(); - fs.mkdirSync(gitDirectory); - execSync(`git -C ${gitDirectory} init`); - await fs.promises.rm(gitDirectory, { recursive: true }); - assert.strictEqual(fs.existsSync(gitDirectory), false); -})().then(common.mustCall()); +if (isGitPresent) { + (async () => { + const gitDirectory = nextDirPath(); + fs.mkdirSync(gitDirectory); + execSync(`git -C ${gitDirectory} init`); + await fs.promises.rm(gitDirectory, { recursive: true }); + assert.strictEqual(fs.existsSync(gitDirectory), false); + })().then(common.mustCall()); +} // Test input validation. { From a2da073c5c25c022cd17aaef7d6f2dfca8639d70 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 26 Mar 2022 19:03:42 +0530 Subject: [PATCH 3/4] test: run `cd dir && git init && cd -` instead of `git init -C dir` Signed-off-by: Darshan Sen --- test/parallel/test-fs-rm.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 930b4f97d36f05..a796a4309f5cd0 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -20,6 +20,14 @@ const isGitPresent = (() => { try { execSync('git --version'); return true; } catch { return false; } })(); +function gitInit(gitDirectory) { + const cwd = process.cwd(); + fs.mkdirSync(gitDirectory); + process.chdir(gitDirectory); + execSync('git init'); + process.chdir(cwd); +} + function makeNonEmptyDirectory(depth, files, folders, dirname, createSymLinks) { fs.mkdirSync(dirname, { recursive: true }); fs.writeFileSync(path.join(dirname, 'text.txt'), 'hello', 'utf8'); @@ -138,8 +146,7 @@ function removeAsync(dir) { // Refs: https://github.com/isaacs/rimraf/issues/21. if (isGitPresent) { const gitDirectory = nextDirPath(); - fs.mkdirSync(gitDirectory); - execSync(`git -C ${gitDirectory} init`); + gitInit(gitDirectory); fs.rm(gitDirectory, { recursive: true }, common.mustSucceed(() => { assert.strictEqual(fs.existsSync(gitDirectory), false); })); @@ -198,8 +205,7 @@ if (isGitPresent) { // Refs: https://github.com/isaacs/rimraf/issues/21. if (isGitPresent) { const gitDirectory = nextDirPath(); - fs.mkdirSync(gitDirectory); - execSync(`git -C ${gitDirectory} init`); + gitInit(gitDirectory); fs.rmSync(gitDirectory, { recursive: true }); assert.strictEqual(fs.existsSync(gitDirectory), false); } @@ -260,8 +266,7 @@ if (isGitPresent) { if (isGitPresent) { (async () => { const gitDirectory = nextDirPath(); - fs.mkdirSync(gitDirectory); - execSync(`git -C ${gitDirectory} init`); + gitInit(gitDirectory); await fs.promises.rm(gitDirectory, { recursive: true }); assert.strictEqual(fs.existsSync(gitDirectory), false); })().then(common.mustCall()); From bc950fa017555d26da8c8a4257a64ee17ffbe072 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 26 Mar 2022 19:37:05 +0530 Subject: [PATCH 4/4] fixup! test: run `cd dir && git init && cd -` instead of `git init -C dir` Signed-off-by: Darshan Sen --- test/parallel/test-fs-rm.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index a796a4309f5cd0..82a5273f28e775 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -21,11 +21,8 @@ const isGitPresent = (() => { })(); function gitInit(gitDirectory) { - const cwd = process.cwd(); fs.mkdirSync(gitDirectory); - process.chdir(gitDirectory); - execSync('git init'); - process.chdir(cwd); + execSync('git init', { cwd: gitDirectory }); } function makeNonEmptyDirectory(depth, files, folders, dirname, createSymLinks) {