Repository: rclone/rclone
Pull request #4324 (2020-06-09 14:09:10)

vfs: Change modtime of file before upload to current

<!--
Thank you very much for contributing code or documentation to rclone! Please
fill out the following questions to make it easier for us to review your
changes.

You do not need to check all the boxes below all at once, feel free to take
your time and add more commits. If you're done and ready for review, please
check the last box.
-->

#### What is the purpose of this change?

Previously files before they get uploaded will inherit the directory modtime.  This changes that to use the current time instead.

#### Was the change discussed in an issue or in the forum before?

https://forum.rclone.org/t/times-on-files-initially-set-incorrect-on-mount/16923/3

#### Checklist

- [ ] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request).
- [ ] I have added tests for all changes in this PR if appropriate.
- [ ] I have added documentation for the changes if appropriate.
- [ ] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [x] I'm done, this Pull Request is ready for review :-)



Patch:
diff --git a/fs/accounting/accounting.go b/fs/accounting/accounting.go
index 0c6c0a50c..244df0784 100644
--- a/fs/accounting/accounting.go
+++ b/fs/accounting/accounting.go
@@ -128,6 +128,13 @@ func (acc *Account) GetAsyncReader() *asyncreader.AsyncReader {
 
 // StopBuffering stops the async buffer doing any more buffering
 func (acc *Account) StopBuffering() {
+	if asyncIn, ok := acc.in.(*asyncreader.AsyncReader); ok {
+		asyncIn.StopBuffering()
+	}
+}
+
+// Abandon stops the async buffer doing any more buffering
+func (acc *Account) Abandon() {
 	if asyncIn, ok := acc.in.(*asyncreader.AsyncReader); ok {
 		asyncIn.Abandon()
 	}
@@ -139,7 +146,7 @@ func (acc *Account) UpdateReader(in io.ReadCloser) {
 	acc.mu.Lock()
 	withBuf := acc.withBuf
 	if withBuf {
-		acc.StopBuffering()
+		acc.Abandon()
 		acc.withBuf = false
 	}
 	acc.in = in
diff --git a/fs/asyncreader/asyncreader.go b/fs/asyncreader/asyncreader.go
index 1df1603f9..6b97c8f6c 100644
--- a/fs/asyncreader/asyncreader.go
+++ b/fs/asyncreader/asyncreader.go
@@ -268,9 +268,15 @@ func (a *AsyncReader) SkipBytes(skip int) (ok bool) {
 	}
 }
 
-// Abandon will ensure that the underlying async reader is shut down.
-// It will NOT close the input supplied on New.
-func (a *AsyncReader) Abandon() {
+// StopBuffering will ensure that the underlying async reader is shut
+// down so no more is read from the input.
+//
+// This does not free the memory so Abandon() or Close() need to be
+// called on the input.
+//
+// This does not wait for Read/WriteTo to complete so can be called
+// concurrently to those.
+func (a *AsyncReader) StopBuffering() {
 	select {
 	case <-a.exit:
 		// Do nothing if reader routine already exited
@@ -280,6 +286,14 @@ func (a *AsyncReader) Abandon() {
 	// Close and wait for go routine
 	close(a.exit)
 	<-a.exited
+}
+
+// Abandon will ensure that the underlying async reader is shut down
+// and memory is returned. It does everything but close the input.
+//
+// It will NOT close the input supplied on New.
+func (a *AsyncReader) Abandon() {
+	a.StopBuffering()
 	// take the lock to wait for Read/WriteTo to complete
 	a.mu.Lock()
 	defer a.mu.Unlock()
diff --git a/vfs/file.go b/vfs/file.go
index 2d1a3c59e..aa11ca013 100644
--- a/vfs/file.go
+++ b/vfs/file.go
@@ -356,7 +356,7 @@ func (f *File) ModTime() (modTime time.Time) {
 		return pendingModTime
 	}
 	if o == nil {
-		return d.ModTime()
+		return time.Now()
 	}
 	return o.ModTime(context.TODO())
 }
diff --git a/vfs/read_write.go b/vfs/read_write.go
index 8f6e2f39c..0eaef8155 100644
--- a/vfs/read_write.go
+++ b/vfs/read_write.go
@@ -20,27 +20,18 @@ import (
 // It will be open to a temporary file which, when closed, will be
 // transferred to the remote.
 type RWFileHandle struct {
-	*os.File
 	mu          sync.Mutex
-	closed      bool // set if handle has been closed
+	fd          *os.File
+	offset      int64 // file pointer offset
 	file        *File
 	d           *Dir
-	opened      bool
 	flags       int  // open flags
+	closed      bool // set if handle has been closed
+	opened      bool
 	writeCalled bool // if any Write() methods have been called
 	changed     bool // file contents was changed in any other way
 }
 
-// Check interfaces
-var (
-	_ io.Reader   = (*RWFileHandle)(nil)
-	_ io.ReaderAt = (*RWFileHandle)(nil)
-	_ io.Writer   = (*RWFileHandle)(nil)
-	_ io.WriterAt = (*RWFileHandle)(nil)
-	_ io.Seeker   = (*RWFileHandle)(nil)
-	_ io.Closer   = (*RWFileHandle)(nil)
-)
-
 func newRWFileHandle(d *Dir, f *File, flags int) (fh *RWFileHandle, err error) {
 	// if O_CREATE and O_EXCL are set and if path already exists, then return EEXIST
 	if flags&(os.O_CREATE|os.O_EXCL) == os.O_CREATE|os.O_EXCL && f.exists() {
@@ -172,7 +163,7 @@ func (fh *RWFileHandle) openPending(truncate bool) (err error) {
 			return errors.Wrap(err, "cache open file failed")
 		}
 	}
-	fh.File = fd
+	fh.fd = fd
 	fh.opened = true
 	fh.file.addRWOpen()
 	fh.d.addObject(fh.file) // make sure the directory has this object in it now
@@ -184,8 +175,6 @@ func (fh *RWFileHandle) String() string {
 	if fh == nil {
 		return "<nil *RWFileHandle>"
 	}
-	fh.mu.Lock()
-	defer fh.mu.Unlock()
 	if fh.file == nil {
 		return "<nil *RWFileHandle.file>"
 	}
@@ -244,7 +233,7 @@ func (fh *RWFileHandle) flushWrites(closeFile bool) error {
 	}
 
 	if writer && fh.opened {
-		fi, err := fh.File.Stat()
+		fi, err := fh.fd.Stat()
 		if err != nil {
 			fs.Errorf(fh.logPrefix(), "Failed to stat cache file: %v", err)
 		} else {
@@ -254,7 +243,7 @@ func (fh *RWFileHandle) flushWrites(closeFile bool) error {
 
 	// Close the underlying file
 	if fh.opened && closeFile {
-		err := fh.File.Close()
+		err := fh.fd.Close()
 		if err != nil {
 			err = errors.Wrap(err, "failed to close cache file")
 			return err
@@ -366,20 +355,30 @@ func (fh *RWFileHandle) Release() error {
 	return err
 }
 
-// Size returns the size of the underlying file
-func (fh *RWFileHandle) Size() int64 {
-	fh.mu.Lock()
-	defer fh.mu.Unlock()
+// _size returns the size of the underlying file
+//
+// call with the lock held
+//
+// FIXME what if a file was partially read in - this may return the wrong thing?
+// FIXME need to make sure we extend the file to the maximum when creating it
+func (fh *RWFileHandle) _size() int64 {
 	if !fh.opened {
 		return fh.file.Size()
 	}
-	fi, err := fh.File.Stat()
+	fi, err := fh.fd.Stat()
 	if err != nil {
 		return 0
 	}
 	return fi.Size()
 }
 
+// Size returns the size of the underlying file
+func (fh *RWFileHandle) Size() int64 {
+	fh.mu.Lock()
+	defer fh.mu.Unlock()
+	return fh._size()
+}
+
 // Stat returns info about the file
 func (fh *RWFileHandle) Stat() (os.FileInfo, error) {
 	fh.mu.Lock()
@@ -387,35 +386,36 @@ func (fh *RWFileHandle) Stat() (os.FileInfo, error) {
 	return fh.file, nil
 }
 
-// readFn is a general purpose read function - pass in a closure to do
-// the actual read
-func (fh *RWFileHandle) readFn(read func() (int, error)) (n int, err error) {
-	fh.mu.Lock()
-	defer fh.mu.Unlock()
+// _readAt bytes from the file at off
+//
+// call with lock held
+func (fh *RWFileHandle) _readAt(b []byte, off int64) (n int, err error) {
 	if fh.closed {
-		return 0, ECLOSED
+		return n, ECLOSED
 	}
 	if fh.flags&accessModeMask == os.O_WRONLY {
-		return 0, EBADF
+		return n, EBADF
 	}
 	if err = fh.openPending(false); err != nil {
 		return n, err
 	}
-	return read()
-}
-
-// Read bytes from the file
-func (fh *RWFileHandle) Read(b []byte) (n int, err error) {
-	return fh.readFn(func() (int, error) {
-		return fh.File.Read(b)
-	})
+	return fh.fd.ReadAt(b, off)
 }
 
 // ReadAt bytes from the file at off
 func (fh *RWFileHandle) ReadAt(b []byte, off int64) (n int, err error) {
-	return fh.readFn(func() (int, error) {
-		return fh.File.ReadAt(b, off)
-	})
+	fh.mu.Lock()
+	defer fh.mu.Unlock()
+	return fh._readAt(b, off)
+}
+
+// Read bytes from the file
+func (fh *RWFileHandle) Read(b []byte) (n int, err error) {
+	fh.mu.Lock()
+	defer fh.mu.Unlock()
+	n, err = fh._readAt(b, fh.offset)
+	fh.offset += int64(n)
+	return n, err
 }
 
 // Seek to new file position
@@ -431,66 +431,67 @@ func (fh *RWFileHandle) Seek(offset int64, whence int) (ret int64, err error) {
 	if err = fh.openPending(false); err != nil {
 		return ret, err
 	}
-	return fh.File.Seek(offset, whence)
+	switch whence {
+	case io.SeekStart:
+		fh.offset = 0
+	case io.SeekEnd:
+		fh.offset = fh._size()
+	}
+	fh.offset += offset
+	// we don't check the offset - the next Read will
+	return fh.offset, nil
 }
 
-// writeFn general purpose write call
-//
-// Pass a closure to do the actual write
-func (fh *RWFileHandle) writeFn(write func() error) (err error) {
-	fh.mu.Lock()
-	defer fh.mu.Unlock()
+// WriteAt bytes to the file at off
+func (fh *RWFileHandle) _writeAt(b []byte, off int64) (n int, err error) {
 	if fh.closed {
-		return ECLOSED
+		return n, ECLOSED
 	}
 	if fh.flags&accessModeMask == os.O_RDONLY {
-		return EBADF
+		return n, EBADF
 	}
 	if err = fh.openPending(false); err != nil {
-		return err
+		return n, err
 	}
 	fh.writeCalled = true
-	err = write()
+
+	if fh.flags&os.O_APPEND != 0 {
+		// if append is set, call Write as WriteAt returns an error if append is set
+		n, err = fh.fd.Write(b)
+	} else {
+		n, err = fh.fd.WriteAt(b, off)
+	}
 	if err != nil {
-		return err
+		return n, err
 	}
-	fi, err := fh.File.Stat()
+
+	fi, err := fh.fd.Stat()
 	if err != nil {
-		return errors.Wrap(err, "failed to stat cache file")
+		return n, errors.Wrap(err, "failed to stat cache file")
 	}
 	fh.file.setSize(fi.Size())
-	return nil
-}
-
-// Write bytes to the file
-func (fh *RWFileHandle) Write(b []byte) (n int, err error) {
-	err = fh.writeFn(func() error {
-		n, err = fh.File.Write(b)
-		return err
-	})
 	return n, err
 }
 
 // WriteAt bytes to the file at off
 func (fh *RWFileHandle) WriteAt(b []byte, off int64) (n int, err error) {
-	if fh.flags&os.O_APPEND != 0 {
-		// if append is set, call Write as WriteAt returns an error if append is set
-		return fh.Write(b)
-	}
-	err = fh.writeFn(func() error {
-		n, err = fh.File.WriteAt(b, off)
-		return err
-	})
+	fh.mu.Lock()
+	defer fh.mu.Unlock()
+	return fh._writeAt(b, off)
+}
+
+// Write bytes to the file
+func (fh *RWFileHandle) Write(b []byte) (n int, err error) {
+	fh.mu.Lock()
+	defer fh.mu.Unlock()
+	n, err = fh._writeAt(b, fh.offset)
+	fh.offset += int64(n)
 	return n, err
 }
 
 // WriteString a string to the file
 func (fh *RWFileHandle) WriteString(s string) (n int, err error) {
-	err = fh.writeFn(func() error {
-		n, err = fh.File.WriteString(s)
-		return err
-	})
-	return n, err
+	return fh.Write([]byte(s))
 }
 
 // Truncate file to given size
@@ -505,7 +506,7 @@ func (fh *RWFileHandle) Truncate(size int64) (err error) {
 	}
 	fh.changed = true
 	fh.file.setSize(size)
-	return fh.File.Truncate(size)
+	return fh.fd.Truncate(size)
 }
 
 // Sync commits the current contents of the file to stable storage. Typically,
@@ -523,9 +524,45 @@ func (fh *RWFileHandle) Sync() error {
 	if fh.flags&accessModeMask == os.O_RDONLY {
 		return nil
 	}
-	return fh.File.Sync()
+	return fh.fd.Sync()
 }
 
 func (fh *RWFileHandle) logPrefix() string {
 	return fmt.Sprintf("%s(%p)", fh.file.Path(), fh)
 }
+
+// Chdir changes the current working directory to the file, which must
+// be a directory.
+func (fh *RWFileHandle) Chdir() error {
+	return ENOSYS
+}
+
+// Chmod changes the mode of the file to mode.
+func (fh *RWFileHandle) Chmod(mode os.FileMode) error {
+	return ENOSYS
+}
+
+// Chown changes the numeric uid and gid of the named file.
+func (fh *RWFileHandle) Chown(uid, gid int) error {
+	return ENOSYS
+}
+
+// Fd returns the integer Unix file descriptor referencing the open file.
+func (fh *RWFileHandle) Fd() uintptr {
+	return fh.fd.Fd()
+}
+
+// Name returns the name of the file from the underlying Object.
+func (fh *RWFileHandle) Name() string {
+	return fh.file.String()
+}
+
+// Readdir reads the contents of the directory associated with file.
+func (fh *RWFileHandle) Readdir(n int) ([]os.FileInfo, error) {
+	return nil, ENOSYS
+}
+
+// Readdirnames reads the contents of the directory associated with file.
+func (fh *RWFileHandle) Readdirnames(n int) (names []string, err error) {
+	return nil, ENOSYS
+}


Test patch:
diff --git a/vfs/read_write_test.go b/vfs/read_write_test.go
index 4e4bdb1c0..c5753aa4e 100644
--- a/vfs/read_write_test.go
+++ b/vfs/read_write_test.go
@@ -18,6 +18,17 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// Check interfaces
+var (
+	_ io.Reader   = (*RWFileHandle)(nil)
+	_ io.ReaderAt = (*RWFileHandle)(nil)
+	_ io.Writer   = (*RWFileHandle)(nil)
+	_ io.WriterAt = (*RWFileHandle)(nil)
+	_ io.Seeker   = (*RWFileHandle)(nil)
+	_ io.Closer   = (*RWFileHandle)(nil)
+	_ Handle      = (*RWFileHandle)(nil)
+)
+
 func cleanup(t *testing.T, r *fstest.Run, vfs *VFS) {
 	assert.NoError(t, vfs.CleanUp())
 	vfs.Shutdown()