BatchDatabase::load_cargo fails on Windows

[vipentti]

Currently BatchDatabase::load_cargo fails on Windows due to cargo not handling extended-length paths well, when either the current directory or manifest-path is an extended-length path (paths starting with \\?\).

See rust-lang/cargo#6198 for more details.

This causes both test_loading_rust_analyzer and cargo run --package ra_cli analysis-stats to fail.

The transform to an extended-length path on Windows is caused by canonicalize in:

https://github.com/rust-analyzer/rust-analyzer/blob/65266c644a31e6b321e5afb3c5a2ee75be76cb0c/crates/ra_batch/src/lib.rs#L98

The actual fail happens in:
https://github.com/rust-analyzer/rust-analyzer/blob/65266c644a31e6b321e5afb3c5a2ee75be76cb0c/crates/ra_project_model/src/cargo_workspace.rs#L124

Output from running test_loading_rust_analyzer:

$ cargo test -p ra_batch --lib
    Finished dev [unoptimized + debuginfo] target(s) in 1m 25s
     Running target\debug\deps\ra_batch-0f1c014d7cd7f7f9.exe

running 1 test
test tests::test_loading_rust_analyzer ... FAILED

failures:

---- tests::test_loading_rust_analyzer stdout ----
thread 'tests::test_loading_rust_analyzer' panicked at 'called
`Result::unwrap()` on an `Err` value: ErrorMessage { msg: "cargo metadata
failed: error during execution of `cargo metadata`: error: failed to read
`\\\\?\\C:\\Stuff\\rust-analyzer\\crates/*\\Cargo.toml`\n\nCaused by:\n The
filename, directory name, or volume label syntax is incorrect. (os error 123)\n"
}', src\libcore\result.rs:1009:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    tests::test_loading_rust_analyzer

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '-p ra_batch --lib'

[matklad]

might be useful: https://gitlab.com/kornelski/dunce

[vipentti]

might be useful: https://gitlab.com/kornelski/dunce

That indeed looks something that could fix this, at least temporarily until std gets a method for getting the absolute path without the extended-length path format.

[matklad]

actually, @flodiebold, why do we call canonicalize there in the first place? It usually is better to just use paths as is: the fewer modifications to the path we do, the higher is the change that paths will work correctly. Original path might not be canonical, but it usually "works".

[flodiebold]

The

        let local_roots = roots
            .into_iter()
            .filter(|r| vfs.root2path(*r).starts_with(&root))
            .map(vfs_root_to_id)
            .collect();

to find the local roots didn't work otherwise. There's probably a better way to do that, though...

[matklad]

I need to check this, but I think what Cargo does is that it absolutize the path (using canonicalized path to cargo.exe) and doesn't do canonicalization

[matklad]

yep, Cargo doesn't do canonicalization, it does only normalization: https://github.com/rust-lang/cargo/blob/2e4cfc2b7d43328b207879228a2ca7d427d188bb/src/cargo/util/paths.rs#L65

EDIT: here's how it is called https://github.com/rust-lang/cargo/blob/2e4cfc2b7d43328b207879228a2ca7d427d188bb/src/cargo/util/command_prelude.rs#L255

[matklad]

My suggested fix is to, first, try use the normalize function from Cargo (by just copy-pasting it), and then fallback to dunce.

[vipentti]

Currently, normalize does not work in the cargo run -p ra_cli analysis-stats case, since the path there is given using ".". We could consider using similar method as in test_loading_rust_analyzer e.g.

https://github.com/rust-analyzer/rust-analyzer/blob/ebfa26658e9f65491e79f6853bb7c77030f5b0fe/crates/ra_batch/src/lib.rs#L136-L140

[matklad]

Could we join std::env::current_dir() to the path to make it absolute?

[vipentti]

cargo run -p ra_cli analysis-stats does work with

let path = std::env::current_dir()?;
let (db, roots) = BatchDatabase::load_cargo(path)?;

In this case the call to normalize does nothing, both the test_loading_rust_analyzer and the cli invocation seem to pass without needing canonicalization, at least on windows.

[matklad]

So that means that inside load_cargo we can do std::env::current_dir()?.join(path)?

[vipentti]

That does seem to work. In both cases.

[vipentti]

Closed the old PR and created new one with the join fix #825