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

predicates::path::missing() should handle broken symlinks #118

Open
gibfahn opened this issue Dec 29, 2021 · 2 comments
Open

predicates::path::missing() should handle broken symlinks #118

gibfahn opened this issue Dec 29, 2021 · 2 comments
Labels
enhancement Improve the expected

Comments

@gibfahn
Copy link

gibfahn commented Dec 29, 2021

The docs for predicates::path::missing() say:

Creates a new Predicate that ensures the path doesn’t exist.

However, if the path is a symlink pointing to nothing, then it will be treated as not existing, when in fact the symlink is a file that exists.

Open to the idea that a broken link should count as not existing, but either way it would be useful for the function's docs to cover this.

It also gets complicated as a symlink may start to exist or not exist based on changes to other parts of the filesystem.

I tried this code:

use predicates::prelude::*;
use std::{fs, os::unix, path::Path};

fn main() {
    let _ = fs::remove_file("symlink-to-non-existent-path");
    unix::fs::symlink("non-existent-path", "symlink-to-non-existent-path").unwrap();

    let predicate_fn = predicate::path::missing();
    // Shouldn't be missing as symlink is a real file.
    assert_eq!(false, predicate_fn.eval(Path::new("non-existent-file.foo")));
}

I expected the assert to pass, but it failed.

Meta

predicates-rs version: 2.1.0
rustc --version: 1.57.0

I think instead of:

    fn eval(&self, path: &path::Path) -> bool {
        path.exists() == self.exists
    }
    fn eval(&self, path: &path::Path) -> bool {
        ( path.exists() || path.symlink_metadata().is_ok() ) == self.exists
    }
@gibfahn
Copy link
Author

gibfahn commented Dec 30, 2021

Happy to raise a PR for this.

@epage epage added the enhancement Improve the expected label Dec 30, 2021
@epage
Copy link
Contributor

epage commented Dec 30, 2021

Thanks for creating this issue!

I feel like matching path.exists() will most meet people's expectations and I worry changing it will break someone and would rather not break compatibility.

I can see us exposing an API with more specific checks (is_file, is_dir, is_symlink) and the corresponding predicate struct could have a follow_symlinks(false) function to customize it, like we do with utf8 on other predicates.

The main question is ow to expose this. Some ideas:

  • is_file, is_dir, is_symlink
  • file_type(FileType::File)
  • file_type(FileType::FILE | FileType::DIR).

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

No branches or pull requests

2 participants