Option for recursive rmdir

[hayd]

At the moment to rm -rf you need to shell out, with this:

julia> mkpath("foo/bar/baz.jl")

julia> rmdir("foo")
ERROR: rmdir: Directory not empty
 in rmdir at file.jl:63
 in rmdir at file.jl:55

julia> rmdir("foo", true) # rm -rf

see also #7046.

[stevengj]

I see how this will work, because rm(p) doesn't return a boolean, but its value is needed in order for Julia whether to evaluate the rmdir in the ||. (On my machine with Julia 0.3, it gives ERROR: type: non-boolean (Nothing) used in boolean context.)

Better to just use a plain if ... else ... end expression here or a isfile(p) ? rm(p) : rmdir(p,true) if you want a one-liner.

[stevengj]

This also doesn't work for symbolic links, since isfile(p) currently returns false for a symbolic link to a directory, and yet rmdir(p) doesn't work for symbolic links to directories.

[hayd]

Thanks! you're right of course! I will break into a for loop.

If I use isdir here instead would this work with sym links (are they are counted as files - so we just want to rm them) ?

[stevengj]

Yes, you want to just rm links. But isdir won't work: it returns true for a symlink to a directory.

I think you need isfile(p) || islink(p) ? rm(p) : rmdir(p, true)

[stevengj]

Or, to avoid a second call to stat, it might be useful to add something like

isfileorlink(st::StatStruct) = st.mode & 0xd000 == 0x8000
isfileorlink(path...) = isfileorlink(lstat(path...))

to stat.jl

[stevengj]

Also, you should add a test case to test/file.jl.

[hayd]

@stevengj I've added isfileorlink to stat like you suggest and a test of rmdir(.., true).

This now works for me with links/files/dirs, but ATM there aren't any tests which use linked files. I could add that some tests for islink (but don't know how to do this without shelling out: ln -s) hayd@474c025 (which passes travis)?

[hayd]

missing true here

[stevengj]

Note that you also need to update the docs for rmdir.

[JeffBezanson]

Bump. Looks pretty good.
cc @StefanKarpinski

[StefanKarpinski]

Two API issues:

1. I'm not convinced that we need isfileorlink. What does that exclude from ispath? That the path is a directory? If the path is a link it could be a link to a file or a directory.
2. I wonder if we shouldn't just make rm delete anything, always recursively. The fact that the shell rm command doesn't do this, is largely because it's intended to be used interactively.

[JeffBezanson]

Yeah, maybe the right test is islink(p) || !isdir(p). fifos and so on should be removed by rm too.

[nalimilan]

About 2., I'd rather add a recursive option to rm, defaulting to false. It may offer additional safety when you want to remove a single file, either from a function or interactively. If for some reason the path gets wrong, you'd avoid destroying a whole directory tree.

[StefanKarpinski]

There's a combinatorial explosion of things you can test a file stat for. We provide convenience accessors for the basic ones, but this is starting down the path of providing all 2^n of them. When performance is an issue, you can use the stat function explicitly and then combines the tests.

[StefanKarpinski]

@nalimilan, you may be right that it's best not to recursively delete directories without some insistence.

[hayd]

Happy to remove the isfileorlink if we don't think perf is an issue here.

IMO the API is quite nice atm that rm removes files or links, and rmdir removes directories (in this branch adding the recursive flag if you specifically want to remove it's contents), to me this seems a useful distinction. Allowing rm with recursive on non-empty directories but rmdir on empty ones seems a strange choice (or are you suggesting removing rmdir entirely?).

[StefanKarpinski]

I was suggesting removing rmdir entirely. I agree that keeping rmdir and adding recursive rm is a weird intermediate situation.

[nalimilan]

I agree that keeping rmdir and adding recursive rm is a weird intermediate situation.

And yet that's the situation of all Linux (POSIX, even?) systems I know of. :-)

I'm OK with keeping only rm, which would remove only single files or empty directories by default, with a recursive argument to remove more than that.

[StefanKarpinski]

I keep not being able to find this because it doesn't link to #7041 anywhere. Now it does.

[staticfloat]

I too suggest an rm with a recursive option to enable deletion of non-empty directories. The splitting out of rmdir has always seemed a very arbitrary thing to me in every programming language I've encountered it in.

[StefanKarpinski]

How about having rm to delete any single thing – a file, link, empty directory – and rm(path; recurse=true) to delete something recursively, which deletes non-empty directories recursively?

[staticfloat]

Works for me.

[tkelman]

There is now symlink functionality in base (#4396), so the tests here should probably be updated to account for that. Will want to test that this doesn't recurse into the targets of symlinks or NTFS junctions on Windows, either.

[StefanKarpinski]

Yeah? You decided isfileorlink belongs in Base?

[tkelman]

He merged then fixed it a2d4dc9

[StefanKarpinski]

Ah, cool.