memfs icon indicating copy to clipboard operation
memfs copied to clipboard

"utimes" doesn't work for directories: "EISDIR: illegal operation on a directory, open ..." error

Open vladimiry opened this issue 6 years ago • 2 comments

utimes calls utimesBase which uses openSync call internally but openSync call on a directory obviously throws EISDIR error.

https://github.com/streamich/memfs/blob/674a1e78469e1e9e766db7e3160538e958fdd97b/src/volume.ts#L1771-L1778

vladimiry avatar Jun 08 '19 11:06 vladimiry

I've also run into this issue. Any solutions?

kellym avatar Sep 11 '19 18:09 kellym

This seems related to https://github.com/streamich/memfs/issues/58. Maybe r is too restrictive there? Modifying those lines seems to fix the issue for me.

kellym avatar Sep 11 '19 18:09 kellym

For what it is worth, here is how I fixed this in my fork (following your suggestions above, and adding a test):

~/upstream/memfs-js$ git show HEAD
commit 671af81ba00e546a10581217f43d32f2406dec6b (HEAD -> main)
Author: William Stein <[email protected]>
Date:   Tue Oct 25 17:56:15 2022 -0700

    fix #2 -- utimesSync doesn't work on directories
    
    - also is upstream https://github.com/streamich/memfs/issues/391

diff --git a/src/__tests__/volume.test.ts b/src/__tests__/volume.test.ts
index 53a1f89..39c39f2 100644
--- a/src/__tests__/volume.test.ts
+++ b/src/__tests__/volume.test.ts
@@ -961,10 +961,17 @@ describe('volume', () => {
     });
     describe('.utimesSync(path, atime, mtime)', () => {
       const vol = new Volume();
+      vol.mkdirSync('/foo');
       it('Set times on file', () => {
-        vol.writeFileSync('/lol', '12345');
-        vol.utimesSync('/lol', 1234, 12345);
-        const stats = vol.statSync('/lol');
+        vol.writeFileSync('/foo/lol', '12345');
+        vol.utimesSync('/foo/lol', 1234, 12345);
+        const stats = vol.statSync('/foo/lol');
+        expect(Math.round(stats.atime.getTime() / 1000)).toBe(1234);
+        expect(Math.round(stats.mtime.getTime() / 1000)).toBe(12345);
+      });
+      it("Sets times on a directory (see https://github.com/streamich/memfs/issues/391)", () => {
+        vol.utimesSync('/foo', 1234, 12345);
+        const stats = vol.statSync('/foo');
         expect(Math.round(stats.atime.getTime() / 1000)).toBe(1234);
         expect(Math.round(stats.mtime.getTime() / 1000)).toBe(12345);
       });
diff --git a/src/volume.ts b/src/volume.ts
index b34860e..e4b05e8 100644
--- a/src/volume.ts
+++ b/src/volume.ts
@@ -1882,7 +1882,7 @@ export class Volume {
   }
 
   private utimesBase(filename: string, atime: number, mtime: number) {
-    const fd = this.openSync(filename, 'r+');
+    const fd = this.openSync(filename, 'r');
     try {
       this.futimesBase(fd, atime, mtime);
     } finally {

williamstein avatar Oct 26 '22 00:10 williamstein

@williamstein I'm happy to review a pull request if you want to open one 🙂

G-Rath avatar Oct 26 '22 00:10 G-Rath

I'm happy to review a pull request if you want to open one

Cool! I will.

williamstein avatar Oct 26 '22 01:10 williamstein

OK, I created https://github.com/streamich/memfs/pull/866 for this.

williamstein avatar Oct 26 '22 01:10 williamstein

Thanks, I'll have a look once I'm off work - it seems like you need to apply prettier to your PRs too.

G-Rath avatar Oct 26 '22 01:10 G-Rath

Thanks, I'll have a look once I'm off work - it seems like you need to apply prettier to your PRs too.

Done.

This memfs codebase is some unusually good quality code. Thanks for maintaining it!

williamstein avatar Oct 26 '22 04:10 williamstein

:tada: This issue has been resolved in version 3.4.9 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

streamich avatar Oct 29 '22 20:10 streamich