julia icon indicating copy to clipboard operation
julia copied to clipboard

Document, test, and publicize `Base.rename`

Open nhz2 opened this issue 1 year ago • 4 comments

Fixes #41584. Follow up of #55384

This marks rename as public.

nhz2 avatar Aug 16 '24 00:08 nhz2

Thanks for the docs and tests! Those should be easy to merge :)

For public API, should this be a method/kwarg of the existing mv? When we have multiple names doing similar things, it's important for the difference in names to reflect the difference in behavior. It's not intuitive to me that mv would be less atomic than replace. It's also not clear from the docstrings what, exactly, the semantic differences are between these functions.

LilithHafner avatar Aug 18 '24 11:08 LilithHafner

I'm like the function name rename because this function is basically rename(2) using libuv to provide an approximation of rename on Windows.

I would say the main difference between rename and mv is mv prioritizes being consistent across different filesystems even if that means having TOCTTOU issues, not being CTRL-C safe, or deleting dst before copying src. On the other hand rename prioritizes being atomic if possible, or otherwise ensuring external processes looking at an existing newpath should see either the contents of the previous newpath or oldpath, but never a missing or partially written file/directory, even if that means behavior will be inconsistent between Windows and Unix.

There is some discussion on this in https://github.com/microsoft/STL/pull/2062

nhz2 avatar Aug 18 '24 13:08 nhz2

Here are some examples of where this functionality is used currently.

https://github.com/JuliaIO/HDF5.jl/blob/fbcd95dde2083c9f8c8ef6defaaf7fafd2552123/src/file.jl#L155

https://github.com/yha/AtomicFileWrite.jl/blob/0fbb7cf5b0e8ef899b2682ea70f5483923636504/src/AtomicFileWrite.jl#L24

https://github.com/medyan-dev/MEDYANSimRunner.jl/blob/67d5b42cc599670486d5d640260a95e951091f7a/src/file-saving.jl#L83

https://github.com/JuliaLang/Pkg.jl/pull/4001

nhz2 avatar Aug 20 '24 14:08 nhz2

Folks using something does not make it part of the public API.

Widespread dependence on internals means we should have some API to expose those internals, but does not necessarily mean we should publicize the internals as is.

Tagging triage to bikeshed the name we should give the functionality that currently lives in the internal Base.Filesystem.rename function.

LilithHafner avatar Aug 20 '24 15:08 LilithHafner

From Triage: Thank you so much for adding docs and tests. None of us on triage know filesystem details well enough to say for sure weather this API is appropriate.

If it's not too hard, motivating examples for new API are always appropriate and helpful.

LilithHafner avatar Aug 29 '24 14:08 LilithHafner

Thanks!

LilithHafner avatar Aug 31 '24 13:08 LilithHafner

Feel free to open a followup to publicize

LilithHafner avatar Aug 31 '24 13:08 LilithHafner