cleanup of Filesystem-related functions

[vtjnash]

1. groups Filesystem-related functions into a new module "Filesystem" (was "Base.FS")
2. deprecates isreadable/writable/executable for filesystem paths, closing iswritable(::FileName) is useless and broken #7385
3. adds a few missing isopen tests

[tkelman]

capitalization bikeshed - Filesystem or FileSystem ?

[dpsanders]

+1 for FileSystem

[vtjnash]

filesystem is a word

[StefanKarpinski]

https://en.wikipedia.org/wiki/File_system: can be two words or one.

[StefanKarpinski]

More prevalent usage seems to be two words; frequent abbreviation as "FS" seems to fit well with calling it FileSystem.

[stevengj]

+1 for "Filesystem". Since the one-word version is already standard English, using CamelCase to concatenate the two-word version looks unnatural to me.

[ScottPJones]

I don't care either way, just data points: Apache Hadoop API -> FileSystem, Java -> FileSystem.

[StefanKarpinski]

Ruby Zip module: http://www.rubydoc.info/gems/rubyzip/Zip/FileSystem

[stevengj]

There is Boost.Filesystem (although the actual namespace is boost::filesystem. On the other hand, there is the Microsoft Visual Basic FileSystem module, and LuaFileSystem, and QFileSystem. So it seems like FileSystem wins the popularity contest, but I don't have strong feelings either way.

[vtjnash]

Ruby seems pretty inconsistent there, since they use FileSystem, but Fs

[ScottPJones]

Don't care, either way is better than just FS.

[StefanKarpinski]

Since FileSystem seems most common and "file system" is a valid spelling so it's not incorrect, let's go with that.

[vtjnash]

that seems to be a lot of emphasis on "System", which I don't think is warranted given that I rarely see "file system" in a document without also finding "filesystem" in the same document somewhere else (indicating that this has rapidly become a word). I'm not really interested in bikeshedding this, but if someone wants to propose a shorter name, I might be listening. Python uses os.path. I would be OK going with Files.

[quinnj]

What about Bash or Shell? I think it'd be cool to do Shell.cd and Shell.touch. Files isn't too bad either, but I think I'd lean towards FileSystem over Files, despite the brevity.

[vtjnash]

This doesn't use the shell or interact with it in any way. I still just don't like the way FileSystem seems to emphasize the word "System", since filesystem is already a valid regular word.

[tkelman]

No need to rush this, I don't see why this would need to be an 0.4 target. iswritable is a more convenient name and easier to remember than fiddling with octal filemodes, and the user inaccuracy issue is a pretty confusing reason for deprecation.

[ScottPJones]

A file system contains directories and files, and doesn't this API do more than just deal with files?
(which might imply name should be filesystem or FileSystem)
cd is changing process or shell state, not changing anything in a file system, so I wouldn't put it under FileSystem.

[StefanKarpinski]

This is absolutely not going into 0.4.

[vtjnash]

rebased to be ready to merge when CI is happy

[tkelman]

deprecation warnings should actually tell the user what replacement code to use.

[vtjnash]

deprecation warnings should actually tell the user what replacement code to use.

i can't really predict why the user thought they wanted to call these functions, so I pointed them to the man page where it describes why linux thinks of them as deprecated.

[tkelman]

What are they documented to do? Deprecated functions should have something done about their docs, but not sure if that should happen at deprecation time or when the deprecation gets removed.

The deprecation warning is not the right place to send someone to read a man page because they called a function that you don't like. People are also using this function on systems that don't have such a man page. Just suggest the replacement code (st.mode & 0o222) > 0 in the warning.

I'm still not convinced there's a good reason to get rid of these since the replacement is inconvenient, low-level and unix-specific code. ok I think I get the issue now, the past behavior was not specific enough?

[tkelman]

:isexecutable

[vtjnash]

What are they documented to do? ... ok I think I get the issue now, the past behavior was not specific enough?

what are they supposed to do? currently they are defined to return true if there might be a user (excluding root) that could have done the specified action at the instant prior to the time the question was answered. however, i can't think of a place case where that definition is useful.

the current docs say:

Returns `true` if the current user has permission to read `path`, `false` otherwise.

but that is not a question that stat can answer, as discussed in #7385

[tkelman]

We don't usually give reasons for deprecation in warnings, we primarily tell the user what to change their code to. Here the ambiguity means users who care probably should do some nontrivial refactoring, but we should at least spell out in the warning how to accomplish what these functions used to do (even if technically buggy).