GNOME Bugzilla – Bug 733469
gphoto's pull method doesn't maintain mtime
Last modified: 2014-09-15 07:25:54 UTC
Since implementing the pull method for gphoto, it doesn't maintain the mtime of the files like it should.
Created attachment 281749 [details] [review] gphoto: Set mtime when pulling Commit 86162bbe4b09 ("gphoto2: Implement pull support") failed to set the modification time of the destination file when implementing pull support. Fix this by setting the modification time on the destination file (only the mtime in seconds needs to be set since this is the only time info the gphoto supports).
Review of attachment 281749 [details] [review]: The patch looks good, however hard to say what is correct behavioral in this case. Pull can be used for copy or move. Permissions and time should be copied in case of move, however it shouldn't be copied in case of copy: $ touch A $ cp A A.copied $ mv A A.moved $ ls --full-time A* -rw-rw-r-- 1 oholy19 oholy19 0 2014-07-30 19:39:04.948378342 +0200 A -rw-rw-r-- 1 oholy19 oholy19 0 2014-07-30 19:39:28.413445201 +0200 A.copied -rw-rw-r-- 1 oholy19 oholy19 0 2014-07-30 19:39:04.948378342 +0200 A.moved Maybe permissions and time should be set from gdeamonfile instead. What are you thinking about?
(In reply to comment #2) > Review of attachment 281749 [details] [review]: > > The patch looks good, however hard to say what is correct behavioral in this > case. Pull can be used for copy or move. Permissions and time should be copied > in case of move, however it shouldn't be copied in case of copy: > Thanks for the review. Each file attribute can have G_FILE_ATTRIBUTE_INFO_COPY_WITH_FILE or G_FILE_ATTRIBUTE_INFO_COPY_WHEN_MOVED set. Look at the following: $ touch A $ gvfs-info -w A Settable attributes: standard::symlink-target (bytestring) time::access (uint64, Keep with file when moved) time::access-usec (uint32, Keep with file when moved) time::modified (uint64, Copy with file, Keep with file when moved) time::modified-usec (uint32, Copy with file, Keep with file when moved) unix::gid (uint32, Keep with file when moved) unix::mode (uint32, Copy with file, Keep with file when moved) unix::uid (uint32, Keep with file when moved) Writable attribute namespaces: metadata (string, Copy with file, Keep with file when moved) xattr (string, Copy with file, Keep with file when moved) xattr-sys (string, Keep with file when moved) $ gvfs-copy A A.copied $ gvfs-move A A.moved $ ls -l --full-time A* -rw-r--r-- 1 ross ross 0 2014-07-30 22:44:16.006417000 +0100 A.copied -rw-r--r-- 1 ross ross 0 2014-07-30 22:44:16.006417000 +0100 A.moved So time::modified* should be copied with the file. unix::mode should also, but since the the gphoto2 backend doesn't have the concept of unix::mode, nothing needs to be done for permissions. As such, I think the patch's behavior is OK. (I hadn't really noticed this difference in behavior between cp and gvfs-copy before...)
Comment on attachment 281749 [details] [review] gphoto: Set mtime when pulling Makes sense, so please commit :-) (I hadn't noticed the existence of G_FILE_ATTRIBUTE_INFO_COPY_WITH_FILE and G_FILE_ATTRIBUTE_INFO_COPY_WHEN_MOVED flags... :-D)
Pushed to master and gnome-3-12. Thanks for the review.
*** Bug 730595 has been marked as a duplicate of this bug. ***