From 9873af762dc5477fb10a748feeba1443171677bf Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 15:04:47 +1000 Subject: [PATCH 1/5] Clarify that mode changes are silently ignored. Currently the semantics are underspecified, and we actually return ENOSYS in this case. But we need to silently ignore changes instead, since otherwise the OS X Finder gives the user an error when they attempt to copy in a file or directory (issue #125). I have observed similar non-fatal errors printed on some Linux distributions. --- docs/semantics.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/semantics.md b/docs/semantics.md index d6b651ce1a..32998e0b04 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -372,7 +372,8 @@ By default, all inodes in a gcsfuse file system show up as being owned by the UID and GID of the gcsfuse process itself, i.e. the user who mounted the file system. All files have permission bits `0644`, and all directories have permission bits `0755` (but see below for issues with use by other users). -Changing permission bits is not supported. +Changing inode mode (using `chmod(2)` or similar) is unsupported, and changes +are silently ignored. These defaults can be overriden with the `--uid`, `--gid`, `--file-mode`, and `--dir-mode` flags. From 23d885b7c88e84ec2c345d01f0e02e825f2707b4 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 15:07:32 +1000 Subject: [PATCH 2/5] Explicitly reserve the right to silently ignore directory mtime changes. The motivation the same as in commit 9873af7. For #125. --- docs/semantics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/semantics.md b/docs/semantics.md index 32998e0b04..6282d58e88 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -300,7 +300,7 @@ look up child inodes. Unlike file inodes: * gcsfuse does not keep track of modification time for directories. There are no guarantees for the contents of `stat::st_mtime` - or equivalent. + or equivalent or the behavior of `utimes(2)` and similar. * There are no guarantees about `stat::st_nlink`. From a5e9287755977d927b032f21fc02de5b6faa9806 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 15:12:05 +1000 Subject: [PATCH 3/5] Add tests for the behaviors that the Finder seems to require. --- internal/fs/local_modifications_test.go | 34 ++++++++++++++++++------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index f389cbdbd5..b99292f1f5 100644 --- a/internal/fs/local_modifications_test.go +++ b/internal/fs/local_modifications_test.go @@ -1676,19 +1676,34 @@ func (t *FileTest) UnlinkFile_ThenRecreateWithSameName() { ExpectEq(1, fi.Sys().(*syscall.Stat_t).Nlink) } -func (t *FileTest) Chmod() { +func (t *FileTest) Chmod_File() { var err error // Write a file. - fileName := path.Join(t.mfs.Dir(), "foo") - err = ioutil.WriteFile(fileName, []byte(""), 0700) + p := path.Join(t.mfs.Dir(), "foo") + err = ioutil.WriteFile(p, []byte(""), 0700) AssertEq(nil, err) - // Attempt to chmod it. We don't support doing so. - err = os.Chmod(fileName, 0777) + // Attempt to chmod it. Chmod should succeed even though we don't do anything + // useful. The OS X Finder otherwise complains to the user when copying in a + // file. + err = os.Chmod(p, 0777) + ExpectEq(nil, err) +} - AssertNe(nil, err) - ExpectThat(err, Error(HasSubstr("not implemented"))) +func (t *FileTest) Chmod_Directory() { + var err error + + // Create a directory. + p := path.Join(t.mfs.Dir(), "foo") + err = os.Mkdir(p, 0700) + AssertEq(nil, err) + + // Attempt to chmod it. Chmod should succeed even though we don't do anything + // useful. The OS X Finder otherwise complains to the user when copying in a + // file. + err = os.Chmod(p, 0777) + ExpectEq(nil, err) } func (t *FileTest) Chtimes_InactiveFile() { @@ -1792,9 +1807,10 @@ func (t *FileTest) Chtimes_Directory() { err = os.Mkdir(p, 0700) AssertEq(nil, err) - // Chtimes should fail; we don't support it for directories. + // Chtimes should succeed even though we don't do anything useful. The OS X + // Finder otherwise complains to the user when copying in a directory. err = os.Chtimes(p, time.Now(), time.Now()) - ExpectThat(err, Error(HasSubstr("not implemented"))) + ExpectEq(nil, err) } func (t *FileTest) Sync_Dirty() { From 174a5a7f5dcdf17e925a58e35b01de32c7c29c95 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 15:14:01 +1000 Subject: [PATCH 4/5] Moved two miscategorized tests. --- internal/fs/local_modifications_test.go | 60 ++++++++++++------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index b99292f1f5..7d0a30cdb4 100644 --- a/internal/fs/local_modifications_test.go +++ b/internal/fs/local_modifications_test.go @@ -1161,6 +1161,35 @@ func (t *DirectoryTest) CreateHardLink() { ExpectThat(err, Error(HasSubstr("not implemented"))) } +func (t *DirectoryTest) Chmod() { + var err error + + // Create a directory. + p := path.Join(t.mfs.Dir(), "foo") + err = os.Mkdir(p, 0700) + AssertEq(nil, err) + + // Attempt to chmod it. Chmod should succeed even though we don't do anything + // useful. The OS X Finder otherwise complains to the user when copying in a + // file. + err = os.Chmod(p, 0777) + ExpectEq(nil, err) +} + +func (t *DirectoryTest) Chtimes() { + var err error + + // Create a directory. + p := path.Join(t.mfs.Dir(), "foo") + err = os.Mkdir(p, 0700) + AssertEq(nil, err) + + // Chtimes should succeed even though we don't do anything useful. The OS X + // Finder otherwise complains to the user when copying in a directory. + err = os.Chtimes(p, time.Now(), time.Now()) + ExpectEq(nil, err) +} + //////////////////////////////////////////////////////////////////////// // File interaction //////////////////////////////////////////////////////////////////////// @@ -1676,7 +1705,7 @@ func (t *FileTest) UnlinkFile_ThenRecreateWithSameName() { ExpectEq(1, fi.Sys().(*syscall.Stat_t).Nlink) } -func (t *FileTest) Chmod_File() { +func (t *FileTest) Chmod() { var err error // Write a file. @@ -1691,21 +1720,6 @@ func (t *FileTest) Chmod_File() { ExpectEq(nil, err) } -func (t *FileTest) Chmod_Directory() { - var err error - - // Create a directory. - p := path.Join(t.mfs.Dir(), "foo") - err = os.Mkdir(p, 0700) - AssertEq(nil, err) - - // Attempt to chmod it. Chmod should succeed even though we don't do anything - // useful. The OS X Finder otherwise complains to the user when copying in a - // file. - err = os.Chmod(p, 0777) - ExpectEq(nil, err) -} - func (t *FileTest) Chtimes_InactiveFile() { var err error @@ -1799,20 +1813,6 @@ func (t *FileTest) Chtimes_OpenFile_Dirty() { ExpectThat(fi.ModTime(), timeutil.TimeEq(newMtime)) } -func (t *FileTest) Chtimes_Directory() { - var err error - - // Create a directory. - p := path.Join(t.mfs.Dir(), "foo") - err = os.Mkdir(p, 0700) - AssertEq(nil, err) - - // Chtimes should succeed even though we don't do anything useful. The OS X - // Finder otherwise complains to the user when copying in a directory. - err = os.Chtimes(p, time.Now(), time.Now()) - ExpectEq(nil, err) -} - func (t *FileTest) Sync_Dirty() { var err error var n int From 679a9409643d287ec2745218bebdcb41dab174f8 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 10 Sep 2015 15:15:56 +1000 Subject: [PATCH 5/5] Updated fileSystem.SetInodeAttributes. --- internal/fs/fs.go | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 1888378dde..7d62ed6a6e 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -952,16 +952,10 @@ func (fs *fileSystem) SetInodeAttributes( in.Lock() defer in.Unlock() + file, isFile := in.(*inode.FileInode) - // We don't support changing non-files. - file, ok := in.(*inode.FileInode) - if !ok { - err = fuse.ENOSYS - return - } - - // Set the mtime, if requested. - if op.Mtime != nil { + // Set file mtimes. + if isFile && op.Mtime != nil { err = file.SetMtime(ctx, *op.Mtime) if err != nil { err = fmt.Errorf("SetMtime: %v", err) @@ -969,8 +963,8 @@ func (fs *fileSystem) SetInodeAttributes( } } - // Set the size, if specified. - if op.Size != nil { + // Truncate files. + if isFile && op.Size != nil { err = file.Truncate(ctx, int64(*op.Size)) if err != nil { err = fmt.Errorf("Truncate: %v", err) @@ -978,12 +972,7 @@ func (fs *fileSystem) SetInodeAttributes( } } - // We don't support setting mode. (We silently ignore atime updates, as per - // docs/semantics.md.) - if op.Mode != nil { - err = fuse.ENOSYS - return - } + // We silently ignore updates to mode and atime. // Fill in the response. op.Attributes, op.AttributesExpiration, err = fs.getAttributes(ctx, in)