split: Don't overwrite files #3719
Conversation
|
This addresses #3627 |
|
Could you please add a test ? :) |
|
Working on it now - I realized I misunderstood the contract first and changed it, so it mostly works, except for the case of a pipe input. Should have something ready shortly |
09f466d to
9f5eb3a
Compare
|
Test added - it now seems to exactly match the behavior of gnu split for the error cases. I have not been able to test on windows, but this should not make things any worse. |
f75436e to
3c460f2
Compare
|
This PR should be good to go now and fix the issue #3627. The review comment is old, but please review now as I think its ready. |
| } | ||
| } | ||
|
|
||
| pub fn paths_refer_to_same_file(p1: &str, p2: &str) -> bool { |
There was a problem hiding this comment.
paths_refer_to_same_file seems to be the same, no ?
There was a problem hiding this comment.
Unfortunately, that doesn't handle the "-" case where we are trying to tell if something like
split < xaa is run. This is the reason it needs to compare to /dev/fd/0. There doesn't seem to be any other way to handle this case correctly (which is part of the gnu test suite).
$ echo 12345 > xaa && split < xaa
split: 'xaa' would overwrite input; aborting
It might be worth extending that method to handle this case, but I'm not sure if this is something we generally want to do. With the code as I have it here it handles that case correctly.
There was a problem hiding this comment.
or you keep the "-" mgmt and plug the rest of the function on paths_referr_to_same_file ?
|
So after a lot of weirdness understanding how macos/bsd processes /dev/fd/0. I finally have a workable solution. On linux "fd(0) == /dev/fd/0" however on macos/Darwin they are not. Specifically, see this: The first 3 lines should have been identical (the Inode is), but the Device (st_dev) is different on the third one. The workaround that I added seems to be the only way to get this to work as there is no "path" that will have the same inode and st_dev. |
|
As reference, here are the equivalent commands on linux: Note that indeed the first three are the same. |
Check that a file exists by calling create_new and changing the interface of instantiate_current_writer to return a Result rather than calling unwrap.
|
I added the same call for windows too (note that I don't have a way to test it though) |
|
Nice: |
|
Let me know if you need anything more done on this. Thanks! |
Check that a file exists by calling create_new and changing the
interface of instantiate_current_writer to return a Result rather
than calling unwrap.