"Package collision in lockfile" using UNC/root local device/etc paths

[rpjohnst]

Problem
When invoking Cargo with the current directory or manifest path given as one of Windows' more "exotic" kinds of paths, such as a \\?\ "root local device path," internal path comparisons can fail and the build aborts. Ideally, Cargo would be able to handle these paths.

Steps
One way to reproduce the bug is with a directory structure like this:

bug/
  main/
    Cargo.toml
    src/lib.rs
  dep_a/
    Cargo.toml
    src/lib.rs
  dep_b/
    Cargo.toml
    src/lib.rs

main depends, via path = "../dep_a" and path = "../dep_b", on both dep_a and dep_b. dep_a also depends, via path = "../dep_b", on dep_b.

Here is a zip file containing the structure described above: path-bug.zip

Then, the bug can be triggered by running this command:

cargo build --manifest-path \\?\C:\full\path\to\bug\main\Cargo.toml

It produces an error message like this:

error: package collision in the lockfile: packages dep_b v0.1.0 (C:\full\path\to\bug\main\../dep_b) and dep_b v0.1.0 (C:\full\path\to\bug\dep_b) are different, but only one can be written to lockfile unambigiously

wasm-pack hits this issue because it sets the current directory to a canonicalized . when invoking Cargo. See rustwasm/wasm-pack#380, rustwasm/wasm-pack#413, and an attempt to work around this problem in rustwasm/wasm-pack#389.

Notes
I most recently reproduced this with Cargo version cargo 1.31.0-nightly (5dbac9888 2018-10-08), on Windows 10.

[Eh2406]

I would be willing to help diagnose, I'd appreciate if you could make a zip file that I could unzip on my C:\ to reproduce (@lazyweb),

[rpjohnst]

It is a bit laborious to recreate, isn't it? 😅

Here you go: path-bug.zip

[Eh2406]

No I am just feeling brain dead from Rust Belt Rust, and wanted a supportive response that did not involve me thinking for a while. Sorry.

Working from your zip I was able to reproduce locally. Oddly if the tomls are changed from / to \\ it works agen. So it is more like "Cargo does not normalize ../ when they are part of Windows' more 'exotic' kinds of paths."

[rpjohnst]

Oddly if the tomls are changed from / to \ it works agen.

Ah, this is very good to know. It gives us a good workaround for the wasm-pack scenario.

[Eh2406]

@alexcrichton So this looks like a path normalization bug, where do our source paths get normalized?

[alexcrichton]

@Eh2406 I think it has to do with something around in src/cargo/util/toml/*, although I could be wrong!

[rpjohnst]

It looks like this is where the path = "../dep" is handled: https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml/mod.rs#L1346-L1363

And this is the function it calls that's mostly likely to be giving bad results: https://github.com/rust-lang/cargo/blob/master/src/cargo/util/paths.rs#L47-L72

Though it's also possibly a bug in the url crate, since the result of normalize_path is immediately handed to this function: https://github.com/servo/rust-url/blob/master/src/lib.rs#L2296-L2340

[Arnavion]

Canonical paths should not have any more normalization done to them. The point of canonical paths is that all normalization has already been applied to them.

Eg \\?\C:\.\a is a valid path that contains a component named . and a component named a under it. It is not correct to treat the . there as the "current directory" and remove it.

It also means that you cannot take a path and append ..\foo to it with the assumption that you're referencing foo one level above the input path.

Edit: From #winapi IRC:

<Arnavion> A path with   \\?\C:\.\a   has a real component named `.` which is not the same as the . in   C:\.\a   where it means "current directory"
<Arnavion> The only thing you can do with a path like   \\?\C:\foo\bar   is know that it has three components, and you can manually remove the last component and append one named baz to get   \\?\C:\foo\baz
<Arnavion> For non-canonical paths you get the same effect by appending `..\baz` in one shot. That is not the case for canonical paths

[retep998]

The most correct way to join ../foo onto an existing path on Windows is to interpret .. and . while appending. So if you started with \\?\C:\bar and joined ../foo you would iterate the components of ../foo and apply them to the base path, first applying .. to get \\?\C:\ and then applying foo to get \\?\C:\foo. Any method of joining paths that would result in \\?\C:\bar\../foo is wrong. Also you have to make sure that you don't treat / in a \\?\ path as a separator, because in those kinds of paths it is a valid component identifier.

[Eh2406]

We had a conversation about this on IRC, logs,

a. What cargo is currently doing is wrong, doing it correctly is hard and there is not a library for it (if you make it we will use it).
b. wasm-pack probably want to use GetFullPathNameW not canonical, but it is not in std nore is there an easy safe wrapper around it (if you make it, it will get used).
c. There is probably a better hack that cargo can use in the meantime, it will still be incorrect, but it will get the common case working.