Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
vladimiry opened this issue Jun 8, 2019 · 9 comments · Fixed by #866
Labels

Comments

@vladimiry
Copy link

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

memfs/src/volume.ts

Lines 1771 to 1778 in 674a1e7

private utimesBase(filename: string, atime: number, mtime: number) {
const fd = this.openSync(filename, 'r+');
try {
this.futimesBase(fd, atime, mtime);
} finally {
this.closeSync(fd);
}
}

@kellym
Copy link

kellym commented Sep 11, 2019

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

@kellym
Copy link

kellym commented Sep 11, 2019

This seems related to #58. Maybe r is too restrictive there? Modifying those lines seems to fix the issue for me.

@williamstein
Copy link
Contributor

williamstein commented Oct 26, 2022

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 <wstein@sagemath.com>
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 added a commit to sagemathinc/memfs-js that referenced this issue Oct 26, 2022
@G-Rath
Copy link
Collaborator

G-Rath commented Oct 26, 2022

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

@williamstein
Copy link
Contributor

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

Cool! I will.

williamstein added a commit to williamstein/memfs that referenced this issue Oct 26, 2022
- "EISDIR: illegal operation on a directory, open ..." error
- streamich#391
@williamstein
Copy link
Contributor

OK, I created #866 for this.

@G-Rath
Copy link
Collaborator

G-Rath commented Oct 26, 2022

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

@williamstein
Copy link
Contributor

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!

G-Rath added a commit that referenced this issue Oct 29, 2022
* fix #391 -- "utimes" doesn't work for directories

- "EISDIR: illegal operation on a directory, open ..." error
- #391

* run prettier

* test: adjust spec title

Co-authored-by: Gareth Jones <Jones258@Gmail.com>
streamich pushed a commit that referenced this issue Oct 29, 2022
## [3.4.9](v3.4.8...v3.4.9) (2022-10-29)

### Bug Fixes

* support calling `utimes` on a directory ([#866](#866)) ([301f2d1](301f2d1)), closes [#391](#391)
@streamich
Copy link
Owner

🎉 This issue has been resolved in version 3.4.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants