Add fs::exists function

[ranile]

Motivation

The std lib implements exists while there's no tokio equivalent. This is possible with metadata function but a wrapper is nice to have.

Solution

Add fs::exists function.

Fixes: #3373

[carllerche]

I don't see an fs::exists fn in std?

[ranile]

I don't see an fs::exists fn in std?

Its under Path::exists in std. I added this as fs::exists because:
a) There is no Path in tokio
b) This discussion on discord

[Kixunil]

TBH, I believe that this API (and the API in std) is broken. Getting e.g. permission denied on metadata() is entirely possible and it doesn't mean that the file doesn't exist. The function should return io::Result<bool> or be called metadata_accessible.

[Darksonn]

Returning io::Result<bool> is much better than metadata_accessible. That's an awful name.

[ranile]

fs::exists(&path).await.unwrap_or(false) isn't too different from fs::metadata(&path).await.is_ok() which defeats the purpose of adding this method. In any case, I think we should just follow the std lib and any broken API should be fixed there first.

[Darksonn]

I mean, are you should people will do unwrap_or(false) if we make it return a Result? They'll probably question mark it instead, propagating it on a permissions error. As for the two snippets, there is actually a quite large difference. One uses the word "exists" for an operation that checks whether something exists, the other uses the word "metadata". That seems like a clear win in readability.

Regarding std, it cannot be fixed in std because of backwards compatibility. Waiting for std to change it is just a roundabout way to say "don't add the method at all". Of course, there's still an argument in following what std does.

I wonder if this was discussed when it was added to std.

[Kixunil]

@hamza1311 in your example you assume broken error handling is preferable, which I don't find convincing. A more reasonable comparison:

fs::exists(&path).await?

instead of:

use std::io::ErrorKind;
fs::metadata(&path)
    .await
    .map(|_| true)
    .or_else(|error| if error.kind() == ErrorKind::NotFound { Ok(false) } else { Err(error) })?

[ranile]

I mean, are you should people will do unwrap_or(false) if we make it return a Result? They'll probably question mark it instead, propagating it on a permissions error. As for the two snippets, there is actually a quite large difference. One uses the word "exists" for an operation that checks whether something exists, the other uses the word "metadata". That seems like a clear win in readability.

That is a valid point.

How about naming the current implementation as accessable instead of exists?

[Darksonn]

Changing the return-type but keeping the name exists seems better than renaming it to accessible to me.

[Kixunil]

I believe another accessible method would be useful, but should not be implemented by just trying to get metadata. Consider this:

touch /tmp/foo
chmod 000 /tmp/foo

metadata still succeeds in this case but the file is not actually accessible, just its metadata. That's why my original suggestion was metadata_accessible. (BTW, I think the name is appropriate but this feature is most likely useless.)

An interesting thing is that accessible should take mode as argument (bitflags: read, write, execute).

I created a thread in internals suggesting deprecating exists.

[kornelski]

What are the use-cases where this matters, and the code isn't broken already?

For example, a code like this:

if !path.exists() {
   write(path);
}

is vulnerable to race conditions. So maybe the lack of exists() is an opportunity to promote alternatives?

[Kixunil]

My real-world use case for scanning FS before actually using it. Basic sanity checks at the start of the program. It's better to report corrupted installation/configuration sooner than it's actually needed and disable the corrupted part (other parts may still work fine!).

Yes this is vulnerable to race conditions, but I find it acceptable as I do the error checking in the code that actually uses the files too and find edge cases unlikely to bother implementing any more safety measures.

This all being said, I don't use exists because I also check permissions and ownership. :)

[mqudsi]

What are the use-cases where this matters, and the code isn't broken already?

It is only racy in terms of the actual creation of the file, but if you are predicating other logic based off the existence or lack thereof of a file, then this is useful.

e.g. detect if an application is installed based off the existence of a file or binary (yes, the application could be installed that second, but you'll account for that by allowing the process to be restarted or something.). Or you need a file to be at a certain location if it isn't already, but generating the file is expensive so you check if it exists before you generate the content to be written (in a non-racy way via, for example, create_new()) so you can at least avoid wasting CPU pre-generating the content.

There are lots of use cases. It's somewhat hubris for an API to presume to know better than the developer about things external to it.

[Kixunil]

@mqudsi exactly, I actually do this in one project. Running touch and rm is simplest way to store one bit of information.

Anyway, try_exists() is now in nightly :)

[LinkTed]

Add the test in the tokio/tests/async_send_sync.rs file.

async_assert_fn!(tokio::fs::exists(&str): Send & Sync & !Unpin);

[LinkTed]

Suggested change

	///
	/// This is an async version of [`std::path::Path::exists`][std]

[LinkTed]

Suggested change

	///
	/// [std]: std::path::Path::exists

[LinkTed]

The example in src/fs/metadata.rs does not hide these lines. Maybe we should be consistent and show these lines?

[LinkTed]

Suggested change

	super::metadata(&path).await.is_ok()
	let path = path.as_ref().to_owned();
	asyncify(|| path.exists()).await

Why not directly use the exists method from std?

[chipsenkbeil]

Is the intention to change this implementation to match the experimental std::fs::try_exists function that was mentioned earlier?

You can find the implementation and see that it's pretty straightforward at the moment:

pub fn try_exists(path: &Path) -> io::Result<bool> {
    match fs::metadata(path) {
        Ok(_) => Ok(true),
        Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(false),
        Err(error) => Err(error),
    }
}

What I'm doing with tokio is pretty much the same with the async version:

match tokio::fs::metadata(path).await {
    Ok(_) => Ok(true),
    Err(x) if x.kind() == io::ErrorKind::NotFound => Ok(false),
    Err(x) => Err(x),
}

[ranile]

Is the intention to change this implementation to match the experimental std::fs::try_exists function that was mentioned earlier?

I don't mind implementing, if that's what should be done. I don't think we need to provide an abstraction for unwrap_or(false) by providing both exists and try_exists. If the maintainers approve it this, I can go ahead and implement it

[Darksonn]

I'm going to close this since #4299 was merged. I'm sorry that this discussion stalled.