Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add keyword remove_destination to mv function #11145

Closed
peter1000 opened this issue May 6, 2015 · 7 comments
Closed

Add keyword remove_destination to mv function #11145

peter1000 opened this issue May 6, 2015 · 7 comments

Comments

@peter1000
Copy link

This is a follow up issue from comments:
#11024 (comment)

julia could add the kwarg: remove_destination::Bool=false to mv to be in line with cp

cp(src::AbstractString, dst::AbstractString; remove_destination::Bool=false, follow_symlinks::Bool=false)

If that is wanted I could do it.

reply StefanKarpinski
@StefanKarpinski > Yes please, @peter1000!


I just had a fast look and I think there are some other issues too:

  1. current docs say Move a file from src to dst. Related to second question: is it suppose to work also on directories?
  2. The real issue is here again similar we had in the cp discussion. symlinks. I tried a fast mv on a directory and obviously any absolute symlinks within the rootdir get broken if the folder is renamed.

I can fix this but the question is: should we also add the kw: follow_symlinks similar to copy.

cp(src::AbstractString, dst::AbstractString; remove_destination::Bool=false, follow_symlinks::Bool=false)

Relevant code portion: fs.jl
file.jl

@peter1000 peter1000 changed the title add kwarods to mv function add kwargs to mv function May 6, 2015
@peter1000
Copy link
Author

according to my reading of the src: mv with the current implementation was only supposed for files: uv_fs_rename lbuv

Equivalent to rename(2).

rename(2)

Name
rename - change the name or location of a file
Synopsis
#include <stdio.h>

int rename(const char *oldpath, const char *newpath);
Description
rename() renames a file, moving it between directories if required.

@peter1000 peter1000 mentioned this issue May 6, 2015
@simonster
Copy link
Member

remove_destination seems reasonable, but I don't think we should try to do anything about absolute symlinks when moving files. I don't know of anything else that tries to handle this case.

rename should work just fine for directories. From the man page you linked:

oldpath can specify a directory. In this case, newpath must either not exist, or it must specify an empty directory.

But I noticed that mv does not presently perform a recursive copy when the rename throws an error (i.e., when the source and destination are on different file systems). It seems like it should.

@peter1000 peter1000 changed the title add kwargs to mv function add kwargs to mv & rename functions May 6, 2015
@peter1000 peter1000 changed the title add kwargs to mv & rename functions add kwargs to mv functions May 6, 2015
@peter1000
Copy link
Author

just something I noticed: currently the test suit uses: Base.samefile to test drectories stat info. src

I just wondered if the name Base.samefile is not a bit misleading in such a case. Would it be worth a consideration to have a samedir or just a samepath.

@simonster
Copy link
Member

samefile is the Python name. I think having separate functions for files and directories would be annoying and not very useful. samepath doesn't seem like it would do the same thing since the same file can reside at multiple paths due to hard links. I think sameinode would be more correct (except that NTFS doesn't call them inodes) but "inode" may be too technical for us.

@StefanKarpinski
Copy link
Member

I think sticking with the Python name here is probably best and making sure the documentation is clear.

@peter1000
Copy link
Author

There is no official documentation as it is not exported: I could add a small comment in the srcfile that it is supposed to work for files and directories when I send this PR.

@peter1000 peter1000 changed the title add kwargs to mv functions Add keyword remove_destination to mv function May 6, 2015
tkelman added a commit that referenced this issue May 13, 2015
…o__mv

Adds kw `remove_destination` to `mv` function #11145
mbauman pushed a commit to mbauman/julia that referenced this issue Jun 6, 2015
tkelman pushed a commit to tkelman/julia that referenced this issue Jun 6, 2015
@jakebolewski
Copy link
Member

closed by #11172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants