[PATCH] copying files: timestamps with nanosecond precision not preserved #3575
Comments
Proposed fix |
|
FWIW, I've read the patch and it looks good to me, but I haven't tried it myself. |
|
|
:-( |
Loosing meta object information by object copy/move is always bad.
What ":-(" means in this case? Re-send as single patches? |
:-( means that I wanted to review it for 4.7.17, but no chance. Either we do it next week, or in some months, so I again moved it to 4.7.18.
Can you review it for 4.7.18? It looks good, but there are no tests and I'm not sure if all places are covered, there will be no problems on non-Linux, etc. so I feel unsafe just committing it. |
Branch: 3575_nanoseconds |
As I can see, now local file copying preserves nanoseconds, but fish copying doesn't. |
Andrew,
thanks for moving this forward.
Regarding fish. Yes, it doesn't support this now, especially if perl is in use. Unfortunately perl seems not to have the function utimensat per default. And since perl is always already installed, it's not likely it gets to the touch case. I asked about this already [1], as I was working on this issue.
If you would agree to make src/vfs/fish/helpers/utime
more complex, adding something like this:
...Hm. But even with this and even if File::lchown turns out to be pre-installed (and lutimes would have been improved to support nanosecond precision), I doubt about the true nanosecond precision. Time values are stored in floats, which cannot hold the full range (seconds with nanoseconds at once), resulting only in correct microseconds. This would only introduce confusion. With zeroed subseconds, you know they're likely just zeroed, not really different.
If you believe there are enough systems without perl and with touch honoring nanoseconds in timestamps, I'll fix this. BTW, is it possible to properly configure MC to not use perl only for one helper script? Or maybe to revert the default only for this script?
[1] #2625 comment 9 |
This depends on what do you mean by "configure". Users can override helper scripts per host, but there is no way to tell mc to not to use Perl in ways other than make perl return 1 or so as far as I know.
This is possible, but these evil scripts are such a can of worms... Do you have access to various systems to test how different touches behave? My understanding is that -t doesn't support nanoseconds, and -d is not available at least on macOS and HP-UX, whereas NetBSD apparently ignores the nanosecond part. Maybe reverse touch & perl for utime, first touch with -t and then with -d (it seems that busybox doesn't have -t o_O), and if neither worked then use perl?
By the way, what about all other VFSes? |
No.
This is from coreutils' NEWS file:
Thus the support is there since 8+ years. Long enough. Or MacOS doesn't use coreutils? Is HP-UX supported by MC? When was the last time a MC developer compiled and used MC on HP-UX (otherwise I doubt one can call this "supported".) If you're not against, I'd suggest to continue this part of discussion on the mailing list [1] (though it's not mc-devel, but mc, so CC: mc-devel, if it is important).
You mean first try with -d (for nanoseconds), then with -t and then perl? This seems to be a good solution. For unknown reason the following script
makes mc to hang after a file is copied:
Thus I haven't yet included this change into patch.
Though running it on the command line works just fine:
I believe that local operations are being used more often than VFSes ones. Thus it is important to fix the first case in first place. Then one can move forward and look into others. And this one could be also someone else(*). So, my goal is to improve one thing while not breaking others. Also I can easily test "Shell link" locally, since sshd is a pretty default daemon. How would you setup a test for other VFSes?
[1] https://mail.gnome.org/archives/mc/2016-October/msg00029.html
(*) I experience problems even with archive VFS and try to avoid them. E.g. looking into an archive results into its unpacking(!) and can take too much time. Looking into an archive which turns out to be password protected results in hanging MC. |
|
Hi,
just to ensure we're not deadlocked: I'm standby awaiting feedback on the last patch.
I'm wondering, where to document two different defaults for utime script: "touch -d"/perl for communicating to GNU/Linux systems (with good touch from coreutils) and perl/"touch -t" for others?
Ideally, MC should inspect the remote system once connected to choose the best script, but that would be another story. And in this case each script should drop alternatives, since every alternative will be in a different script. |
Replying to ag:
The reason is buffer overflow. buf[1024] in fish_utime() is too small to store entire updated FISH utime command. Now command is built as dynamycally allocated string not static buffer.
Branch: 3575_nanoseconds
Please review.
P.S. The audit is required for all other FISH commands. Those buffers might be overflowed too. |
Replying to andrew_b:
Sorry, buffer was not overflowed, sure. But command was just truncated at buffer length and server received only 1023 bytes and waited for remain part of command. |
Andrew,
such truncation (in opposite to segfault or reading out-of-bounds memory) would be really hard to track without being very familiar with the code base. Thanks for figuring this out and fixing! If there would be a convenient assert for truncations...
And I'm very appreciating, you've already taken care of rebasing the patch and replacing the helper script and its default definition in fishdef.h.
The only minor thing, I've noticed, is:
Since the helper scripts can be altered by users, all such buffers must be allocated dynamically (but with some reasonable maximal size)? And if truncation could not be avoided, instead of calling fish_send_command() -1 must be returned immediately to prevent hanging. On the other hand, if user made some mistake in a script, fish_send_command() could hang. I'm wondering, whether this could be avoided generally.
If this is not an one-liner fix, such as:
would it be better to track this in a new bug?
BTW, is vfs_s_get_line() not generally risky or there were some reasons to use it? |
Replying to ag:
}}}
You can avoid hang of client, but what about server side?
You send a shell script to server. Then you go to read server's answer. You don't know that you send truncated script. Server still wait for script end. Now we get the following state: you are reading socket to get the server's answer, but server is reading socket to get the end of your script. What happens in server if you terminate your read? |
Replying to andrew_b:
Once you pointed out an issue with FISH, I've been searching on the FISH protocol and found either FISH shell or the fact, that FISH was invented for and in MC. Now you confirmed this. Looking how it works: just sends script and runs it over SSH (good question about RSH), I assumed the first commented line is a very short manual what environment variables gets the script. Sorry, I was wrong about this. So this seems to be done now.
As I described above, I assumed we can detect truncation and by immediately return -1 we can avoid sending, which would block. But even if we couldn't detect the truncation, we can append/overwrite the last character with EOL (this is enough to signal the end of a script?). So the question remains, what could happen if the script itself is buggy. It can just miss an echo. Then if the read is terminated, we terminate the connection and return error? Or in another case if the script doesn't finish at all, would it be possible to terminate the connection either? I have not yet the necessary overview of the related code to be sure, how the connection establishment and finishing are handled. |
Replying to ag:
These questions are off topic of nanosecond precision and topic for another ticket.
So, if there are no objections here, I'm going to merge the 3575_nanoseconds branch into master one and close this thicket. |
|
Merged to master: [4a6fb3b]. |
|
Important
This issue was migrated from Trac:
ag(andrey.gursky@…-mail.ua)timestamp,nanosecond,utime,utimensatReproducer:
$ touch dummy.txt
$ mc
(copy with shift+f5 to dummy-copied-with-mc.txt)
$ ls --full-time dummy*
-rw-r--r-- 1 andrey andrey 0 2015-12-06 21:48:49.000000000 +0100 dummy-copied-with-mc.txt
-rw-r--r-- 1 andrey andrey 0 2015-12-06 21:48:49.242551392 +0100 dummy.txt
$
The Linux kernel has utimensat() since 2.6.22, and since 2.6.26 (7+ years) it is conform to POSIX.1-2008. Even NTFS supports 100-ns precision.
Note
Original attachments:
ag(andrey.gursky@…-mail.ua) onDec 12, 2015 at 12:48 UTCag(andrey.gursky@…-mail.ua) onOct 29, 2016 at 2:05 UTCThe text was updated successfully, but these errors were encountered: