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

Cargo should check for windows-reserved filenames when extracting a package. #8055

Closed
Skgland opened this issue Mar 30, 2020 · 7 comments
Closed
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-bug Category: bug E-easy Experience: Easy O-windows OS: Windows

Comments

@Skgland
Copy link

Skgland commented Mar 30, 2020

EDIT (ehuss): The issue is that navi includes a file named aux.rs which is not a valid filename on windows. Cargo should check for these reserved filenames while extracting and provide a better error message.

Problem

On Windows Cargo failes to install a package because it fails to set mtime fo a file.
The package where the error occourse is navi version 2.2.0

Console Log

Steps

  1. On windows run cargo install navi

Possible Solution(s)

Notes

Output of cargo version:
cargo 1.44.0-nightly (7019b3ed3 2020-03-17)

cargo 1.42.0 (86334295e 2020-01-31) has the same problem under Windows but gives shorter error trace.

Same package and cargo versions work under Linux Mint.
Also I currently cannot reproduce this with other packages.

@Skgland Skgland added the C-bug Category: bug label Mar 30, 2020
@alexcrichton
Copy link
Member

This is likely cropping up due to #7935 (cc @cwndrws)

I'm unfortunately not really sure what this error message means, it seems highly likely to be related to something Windows-specific, though.

@ehuss
Copy link
Contributor

ehuss commented Apr 14, 2020

aux.rs is not a valid filename on windows. As of #7959 Cargo will now display a warning when publishing a package with names like these. But it should probably check for these explicitly when extracting, too, so it is not so confusing.

@ehuss ehuss changed the title cargo install fails failed to set mtime Cargo should check for windows-reserved filenames when extracting a package. Apr 14, 2020
@ehuss ehuss added A-diagnostics Area: Error and warning messages generated by Cargo itself. E-easy Experience: Easy O-windows OS: Windows labels Apr 14, 2020
@hbina
Copy link
Contributor

hbina commented Apr 17, 2020

Since this is marked Easy, I assume an explicit warning of this behavior by Windows is sufficient? Or is there a way to work around this

@ehuss
Copy link
Contributor

ehuss commented Apr 18, 2020

@hbina Sorry, I'm not sure I understand your question.

I think the best thing to do is to just elaborate the error message when extracting a reserved file.

Extracting happens around here. When an error happens, Cargo should check (only on Windows) if the file name (minus any extension), or any of the directory components is reserved (using is_windows_reserved). If so, add a hint to the error message explaining that the package contains a reserved filename on Windows, and cannot be used.

@hbina
Copy link
Contributor

hbina commented Apr 19, 2020

that answered my question. Thanks!

@CPerezz
Copy link
Contributor

CPerezz commented Apr 25, 2020

Hey, if @hbina is not going to work on this, I would like to take that issue! :)

CPerezz added a commit to CPerezz/cargo that referenced this issue Apr 26, 2020
Following issue rust-lang#8055, added a block before the
unpacking stage of `cargo install` to check if any of
the paths that the package has contains files or folders
with reserved Windows filenames.

- Added an error message for these specific cases pointing which is
the path an the file that are actually causing the error.

- Added an error message for failed obtentions of the path as &str
if the Path contains non-UTF8 characters which are non-valid Paths
in Windows.
CPerezz added a commit to CPerezz/cargo that referenced this issue Apr 26, 2020
Following rust-lang#8055 we need to propperly print errors if the
path to any file of the package contains a windows reserved filename.

- Added tests for packages with a file that has a reserved filename on
Windows.

- Added tests for packages with a folder that has a reserved
filaname on Windows.
@ehuss
Copy link
Contributor

ehuss commented Apr 26, 2020

Fixed by #8136.

@ehuss ehuss closed this as completed Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. C-bug Category: bug E-easy Experience: Easy O-windows OS: Windows
Projects
None yet
Development

No branches or pull requests

5 participants