Tracking Issue for fs_try_exists

[Kixunil]

Feature gate: #![feature(path_try_exists)]

This is a tracking issue for try_exists() method on std::path::Path.

This method is similar to exists() method except it does not silently ignore errors that made it impossible to find out if the path actually exists (e.g. lack of permission on parent dir). Thus it naturally returns io::Result<bool> instead of just bool.

Public API

mod fs {
    pub fn try_exists<P: AsRef<Path>>(path: P) -> Result<bool>;
}

impl Path {
    #[stable(since = "1.63.0")]
    pub fn try_exists(&self) -> io::Result<bool>;
}

Steps / History

• Added try_exists() method to std::path::Path #81822
• Windows implementation of feature path_try_exists #85060
• Stabilize Path::try_exists() and improve doc #97912

Unresolved Questions

• None yet.

[ChrisDenton]

Could try_exists be added as a free function to std::fs and then the std::path::Path method forwards to that? This would be similar to the metadata function.

My motivation for this is to make it easier for platforms to override the metadata based implementation.

[Kixunil]

In the internals discussion @matklad pointed out that is_file() and is_dir() have same issue. I believe it may be even worse in their case. I wonder would it be reasonable to add those method within the scope of this feature or as a separate change?

[roblabla]

Since #85060, there's now std::fs::try_exists but no std::fs::exists (the method only exists on Path). This is somewhat confusing, especially since the documentation for std::fs::try_exists says:

As opposed to the exists() method, this one doesn’t silently ignore errors unrelated to the path not existing.

Without linking the appropriate exists method.

I think if we're going to have an std::fs::try_exists, we'll want to make an std::fs::exists method as well for symmetry.

[ChrisDenton]

I think there are a number of options here:

• Add std::fs::exists as you suggest.
• Don't stabilize std::fs::try_exists. I'd still want there to be some kind of fs::try_exists so that different platforms can specialize the implementation. But that can be purely internal to the standard library.
• Keep the current API but improve the docs. The original rationale for not having an std::fs::exists is that Path::exists is just a convenience wrapper around std::fs::metadata(path).is_ok(). A similar rationale could be applied when Path::exists is implemented as std::fs::try_exists(path).unwrap_or(false).

[Kixunil]

@roblabla clearly the doc was copied from Path::try_exist. Would be nice to fix.

[ChrisDenton]

I've submitted the small PR above which you may want to review.

[joshtriplett]

Nominating to consider stabilization.

[joshtriplett]

We discussed this in today's @rust-lang/libs-api meeting.

We're in favor of stabilizing Path::try_exists. We'd support a partial stabilization for that.

We had a long conversation about whether we should stabilize std::fs::try_exists as-is, stabilize it as std::fs::exists (with the same signature), or providing both std::fs::exists and std::fs::try_exists (with the same signature).

[Kixunil]

@joshtriplett can I send a stabilization PR now or is something else needed first?

[m-ou-se]

@Kixunil We usually do the FCP on the tracking issue, before opening the stabilization PR. But in this case, since it's a partial stabilization, it's probably better to open a stabilization PR first and do the FCP there. Then it's very clear what exactly we are and aren't stabilizing.

So yes, go ahead. :) (Thanks!)

(You might need to change the feature name for std::fs::try_exists, to avoid having the same feature name being both stable and unstable.)

[Kixunil]

Yeah, I suspected something along those lines, submitted!

[kaankoken]

Am I missing something std::Path::new("wrong_path")try_exists()? This always returns false. Should not be returning an error?

[ranile]

No, this should be returning false because the path doesn't exist. As far as I know, the error is only returned when path exists but an error occurs in reading the metadata (e.g. permissions error)

…

On Tue, Apr 11, 2023, 6:42 PM kaan taha köken ***@***.***> wrote: Am I missing something std::Path::try_exists("wrong_path")? This always returns false. Should not be returning an error? — Reply to this email directly, view it on GitHub <#83186 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALJJ7WNLTDV65IZZUAQPFDDXAVNVNANCNFSM4ZIACLTQ> . You are receiving this because you commented.Message ID: ***@***.***>

[Kixunil]

What @hamza1311 says is correct and intended behavior.

[dtolnay]

@rust-lang/libs-api:
@rfcbot fcp merge

I am interested in stabilizing fs::try_exists with a rename to fs::exists.

https://doc.rust-lang.org/nightly/std/fs/fn.try_exists.html

Example usage:

// build.rs

fn main() -> Result<()> {
    let header = "libstable/include/libstable.h";
    println!("cargo:rerun-if-changed={}", header);

    if !fs::exists(header)? {
        Command::new("git")
            .args(["submodule", "update", "--init", "libstable"])
            .status()
            .exit_ok()?;
    }

    // bindgen stuff
}

fs::exists(pathlike) is a more pleasant way to write Path::new(&pathlike).try_exists(), similar to how fs::metadata(pathlike) is equivalent to Path::new(&pathlike).metadata().

std::path::Path::try_exists (https://doc.rust-lang.org/nightly/std/path/struct.Path.html#method.try_exists) has been stable since Rust 1.63, and retains its usefulness when you are already working with a Path, such as:

fn find_cargo_manifest() -> Result<PathBuf> {
    let dir = env::current_dir()?;
    let mut dir = dir.as_path();
    loop {
        let path = dir.join("Cargo.toml");
        if path.try_exists()? {
            return Ok(path);
        }
        dir = match dir.parent() {
            Some(parent) => parent,
            None => return Err(Error::new(ErrorKind::NotFound, "Cargo.toml not found")),
        };
    }
}

Regarding the choice of name fs::exists vs fs::try_exists, I think there is no need for try_ in the name because we can decide to rule out adding a -> bool version of exists in std::fs. If we are able to rule that out now, then keeping try_ in the name no longer serves a purpose. Likewise, fs::metadata is not called fs::try_metadata despite returning Result.

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.