Added preserve_time parameter to move, copy, and mirror #463
Conversation
If `preserve_time` is set to True, the call tries to preserve the atime, ctime, and mtime timestamps on the file after the copy. The value is currently not actually used; places in code that have to be adapted are marked with `TODO(preserve_time)`.
At this point in time, all the new tests are failing right now; as they should, because the functionality of `preserve_time` has not been implemented yet.
All logic for the added `preserve_time` parameter is contained in that function, so other pieces of code just have to make a single call. It's still not implemented at this point.
…changing the access time itself was an access.
|
I wanted to add https://github.com/wolever/parameterized as a dependency because parameterization was done already anyway, just in a hacky self-implemented way. I don't actually get how to add test dependencies though? |
|
If you want to add a dependency for the tests, add it to |
… and `MemoryFS.movedir`.
…me guarantees. As it turns out, there are platforms which do not allow changing either atime or ctime to custom values. Thus, the proposed behavior was impossible to implement. `preserve_time` now only makes guarantees about mtime.
|
@atollk : that was not expected, I patched |
…ified time of a file.
|
I just saw that |
…ng used, rather than being assumed to be True always.
There was a problem hiding this comment.
I'm not 100% satisfied with the copy_mtime name, because we never refer to mtime anywhere else in the API (we use modified everywhere). Yet copy_modified does not convey the function usecase very well. copy_modified_time maybe? Might be a bit too long.
There are just a few minore issues here and there but overall this looks fine.
# Conflicts: # fs/copy.py # tests/test_copy.py
In a certain special case, the parameter was not passed on correctly but always `False` instead.
Before, `setinfo` would always start by sending a MLST or LIST command to find whether the resources exists. This is actually not necessary if MFMT is sent anyway, so the command is now skipped in that case.
|
I merged this one because I have already been too nit-picky here, but for the remaining PRs, would you be able to rebase them to |
|
@althonos Sure can do. But I'd also be happy to write a more thorough commit message for a squash merge, since I feel a good part of the commits in those PRs are just one-file fixes anyway. Or do you not do squashes in this repo? |
|
The developer interaction was minimal here for some time so we just went with merges. I do squashes in other projects but for PyFilesystem I didn't wanna change what was there so branch/merges are fine. As long as they are properly rebased, the network is still fine: https://github.com/PyFilesystem/pyfilesystem2/network |
|
Thanks, perfect, i'll have a look tomorrow ! |
Type of changes
Checklist
Description
Fixes #450 by adding a
preserve_timeparameter to all relevant functions. The parameter isFalseby default to retain previous behaviour. However, it could be argued that the default should beTrueto mimic more common behavior found in operating systems.