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

Invalid absolute path on windows when searching from the drive root #931

Closed
gbarta opened this issue Jan 5, 2022 · 5 comments
Closed

Invalid absolute path on windows when searching from the drive root #931

gbarta opened this issue Jan 5, 2022 · 5 comments

Comments

@gbarta
Copy link
Contributor

gbarta commented Jan 5, 2022

This works as expected with 8.2.1 but not with 8.3.0. I'll list output from the two versions downloaded from releases:

C:\Windows>\temp\fd\fd-v8.2.1-x86_64-pc-windows-msvc.exe --version
fd 8.2.1

C:\Windows>\temp\fd\fd-v8.3.0-x86_64-pc-windows-msvc.exe --version
fd 8.3.0

From within a specific folder, both versions output valid absolute paths:

C:\>cd \windows

C:\Windows>\temp\fd\fd-v8.2.1-x86_64-pc-windows-msvc.exe -a -e dirmngr
C:\Windows\S.dirmngr

C:\Windows>\temp\fd\fd-v8.3.0-x86_64-pc-windows-msvc.exe -a -e dirmngr
C:\Windows\S.dirmngr

From the drive root, 8.3.0 outputs an invalid absolute path from the drive root, missing the initial path separator:

C:\Windows>cd \

C:\>\temp\fd\fd-v8.2.1-x86_64-pc-windows-msvc.exe -a -e dirmngr
C:\Windows\S.dirmngr

C:\>\temp\fd\fd-v8.3.0-x86_64-pc-windows-msvc.exe -a -e dirmngr
C:Windows\S.dirmngr
@gbarta gbarta added the question label Jan 5, 2022
@gbarta
Copy link
Contributor Author

gbarta commented Jan 5, 2022

I don't have rust expertise to know the best way to fix this, but I have found a few things.

  • git bisect shows the bug introduced in commit aeff525 with the switch from canonicalize to normalize for search paths. In this case the search path is simply ".".
  • normpath::PathExt::normalize returns "C:" for input "." when the current directory is "C:\". As I explain in the next point this is not strictly speaking incorrect, but it is surely one of the less useful things it could return. For other current directories, normalize will return the actual name of the current directory. I imagine that it is returning "C:" because of some desire to remove trailing \ chars for directories.
  • filesystem::absolute_path seems to return "C:" unchanged for input "C:" but this is incorrect, since in windows "C:" is not an absolute path. It represents the current directory of the C drive and thus is a relative path, specifically it represents "." for drive C. See the last example in the table at https://docs.microsoft.com/en-us/dotnet/standard/io/file-path-formats

@sharkdp
Copy link
Owner

sharkdp commented Jan 6, 2022

Thank you very much for the excellent bug report and for taking this upstream. Let's wait for the upstream discussion to settle.

@gbarta
Copy link
Contributor Author

gbarta commented Jan 6, 2022

Thanks! This has now been fixed in upstream normpath 0.3.2

tavianator added a commit that referenced this issue Jan 7, 2022
Update normpath to 0.3.2 to fix issue #931
@tavianator
Copy link
Collaborator

This should be fixed now with #936

@sharkdp
Copy link
Owner

sharkdp commented Jan 29, 2022

Released as https://github.com/sharkdp/fd/releases/tag/v8.3.2

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

No branches or pull requests

3 participants