search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
95.4k stars 12.29k forks source link

rust-lang/rust CI build failed when generating test file paths exceeding about 260 chars on Windows #77576

Open richkadel opened 3 years ago

richkadel commented 3 years ago

The problem and a proposed guard to avoid wasting time and resources

This bug (panic message shown below) resulted from a combination of components with long names, resulting in mir-dump output that tried to create a file path exceeding Windows' 260 byte (give or take?) max path length limit.

The bors CI builds for Windows, under both MSVC and mingw/gnu are susceptible to this kind of failure.

I worked around the problem by abbreviating directory names, rust program file names, and symbols inside the rust program, all of which contributed to the long path name.

The error messages produced by Rust and/or Windows (NotFound) offer no obvious clue that the problem is the path length. (Thankfully I knew enough to suspect it, but others may not.)

This was not caught until I ran the tests under CI, and it happened more than once (at least once under MSVC, which I fixed, and then again under mingw, because I didn't shrink the paths enough the first time.

  1. It is hard to diagnose.
  2. It costs the developer a lot of time to find and fix the problem.
  3. It wastes CI resources, and backs up the bors queue for others.

This is something that could be caught early, during development (even before sending to bors).

My recommendation is, when compiletest runs a test on any platform (e.g., Linux or MacOS, where this problem does not crop up naturally because they don't have path limits), compiletest should check the lengths of all generated files in the build/<target>/src/test directory. Strip the directory prefix up through build, and if the remaining subpath of any file path is greater than, say, 215 characters, generate an error or warning (something the developer won't miss) that their test is generating paths that may exceed path limits on Windows, and may not pass CI.

This should be enough to get the developer to update their tests and avoid the entire problem.

Meta

./build/x86_64-unknown-linux-gnu/stage1/bin/rustc --version --verbose
rustc 1.49.0-dev
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-unknown-linux-gnu
release: 1.49.0-dev
LLVM version: 11.0

Error output

(Newlines inserted for readability.)


thread 'rustc' panicked at 'Unexpected error creating MIR spanview HTML file: 
Custom { kind: NotFound, error: "IO error creating MIR dump file: 
\"D:/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/
instrument-coverage-mir-cov-html-link-dead-code/instrument-coverage-mir-cov-html-link-dead-code/
mir_dump.inner_items\\\\inner_items.main-InnerTrait-default_trait_func.-------.InstrumentCoverage.0.html\";
The system cannot find the path specified. (os error 3)" }', 
compiler\rustc_mir\src\transform\instrument_coverage.rs:613:14```
mati865 commented 3 years ago

Possible solutions are listed in https://github.com/rust-lang/rust/issues/76586#issuecomment-691949376

Long paths feature is enabled on Github Actions so if Rust added special manifest to it's binaries CI would not hit this issue. However this feature is not enabled by the default on any Windows so developers can still hit this issue unless they have explicitly enabled this feature.

As a not ideal workaround for this specific case you could convert the path to the normalised format (it starts with \\?\) by running canonicalize() right before writing to that path. "Right before writing to that path" is important here because normalised paths are always absolute and don't support pushing .. (one dir up) so they are often horrible to work with.

cc https://github.com/rust-lang/rust/issues/67403 https://github.com/rust-lang/rust/issues/76586

richkadel commented 3 years ago

Thanks @mati865 ! I saw that first issue you referenced. Glad you linked the possible solutions comment here.

A general solution for Rust would be most welcome.

I was just thinking that, in the mean time, we might be able to add a simple check somewhere, like in compiletest, to save some future poor soul from the kind of trouble I ran into... haha.

Agree the canonicalize() is not great. I worked around the problem already though.

Thank you!

ChrisDenton commented 2 years ago

Hopefully this has been fixed by #89174.