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

[BUG] Wrong result when --full-path and .. #1513

Open
1 task done
Rom888 opened this issue Mar 10, 2024 · 3 comments
Open
1 task done

[BUG] Wrong result when --full-path and .. #1513

Rom888 opened this issue Mar 10, 2024 · 3 comments
Labels

Comments

@Rom888
Copy link

Rom888 commented Mar 10, 2024

Checks

  • I have read the troubleshooting section and still think this is a bug.

Describe the bug you encountered:

mkdir /tmp/1
cd /tmp/1
touch foo
mkdir /tmp/1/2
cd /tmp/1/2

fd --full-path -t f '/tmp/1/foo' ..
# no result here, but expected: ../foo

fd --full-path -t f '/tmp/1/.*/.*/foo' ..
# it seems to me that this result should not be here, but the result is: ../foo

Describe what you expected to happen:

No response

What version of fd are you using?

fd 9.0.0

Which operating system / distribution are you on?

$ uname -srm
Linux 6.5.9-t2-lunar x86_64

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 23.04
Release:	23.04
Codename:	lunar
@Rom888 Rom888 added the bug label Mar 10, 2024
@tmccombs
Copy link
Collaborator

Hmm i suspect the path it is comparing to might be

/tmp/1/2/../foo

@tavianator
Copy link
Collaborator

@tmccombs That's my guess too. But trying to confirm with --absolute-path causes it to not match any more! I guess that's another bug.

@tavianator
Copy link
Collaborator

So if --absolute-path is specified, we eagerly normalize the command line search paths:

fd/src/cli.rs

Lines 666 to 672 in 4efc05e

fn normalize_path(&self, path: &Path) -> PathBuf {
if self.absolute_path {
filesystem::absolute_path(path.normalize().unwrap().as_path()).unwrap()
} else {
path.to_path_buf()
}
}

Without --absolute-path, the search path stays as-is (..). Then later when matching, if --full-path is given, we call path_absolute_form():

fd/src/walk.rs

Lines 513 to 516 in b874462

let search_str: Cow<OsStr> = if config.search_full_path {
let path_abs_buf = filesystem::path_absolute_form(entry_path)
.expect("Retrieving absolute path succeeds");
Cow::Owned(path_abs_buf.as_os_str().to_os_string())

But path_absolute_form() doesn't normalize:

fd/src/filesystem.rs

Lines 14 to 21 in b874462

pub fn path_absolute_form(path: &Path) -> io::Result<PathBuf> {
if path.is_absolute() {
return Ok(path.to_path_buf());
}
let path = path.strip_prefix(".").unwrap_or(path);
env::current_dir().map(|path_buf| path_buf.join(path))
}

I think having --absolute-path change the result set is a bug we need to fix. But we need to be careful.

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

No branches or pull requests

3 participants