Fix --manifest-path for verbatim paths

[dylni]

This lets --manifest-path be a verbatim path (\\?\C:\full\path\to\manifest) without causing:

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

There are 2 commits:

• The first commit is the minimum required to fix the issue.

• The second commit also improves normalization of that section to resolve .. correctly on Unix, but it causes some test regressions. They can be fixed by using the original path when normalization fails, which Cargo does in other cases.

  Should I keep the regressions or use that fallback for this PR too?

Closes #6198

cc @Eh2406

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Eh2406 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[dylni]

This is the failing line:

cargo/tests/testsuite/member_errors.rs

Line 107 in 9051345

assert_eq!(causes.len(), 2, "{:?}", causes);

I don't know enough about this test to figure out the right fix.

[alexcrichton]

There's some more discussion about this on #7729, but unfortunately path normalization is tricky in Cargo and is something we try to review pretty closely. We primarily try to normalize as little as possible as well as only having one method of normalization rather than multiple.

I think these issues would be great to fix but I haven't investigated them enough myself to know how to best do so. I think it would be good though to add a suite of test if possible. If this is just fixing one issue for Windows UNC paths then the fix can probably be a bit more isolated?

[dylni]

@alexcrichton The new function uses the GetFullPathNameW syscall on Windows, which normpath provides with PathExt::normalize. It should always return a correct normalized path, but others reviewing the code is always welcome. My intention is to eventually remove normalize_path_legacy and convert all uses to the safer syscall version.

I can add some tests here if this change is decided to be useful, but I've made sure to test normpath extensively to make sure it can be used in these situations. The test from the bug report has already been added.

[bors]

☔ The latest upstream changes (presumably #8907) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

[bors]

☔ The latest upstream changes (presumably #9051) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #9292) made this pull request unmergeable. Please resolve the merge conflicts.

[dylni]

@alexcrichton @Eh2406 Does this PR require anything else? It's a start to fixing more normalization issues.

[64]

Bump.

[ehuss]

Closing as this PR is quite old, and unfortunately it isn't entirely clear on how to proceed with fixing the Windows paths issues (and unfortunately none of us have much time to invest in this). I have opened #9769 as a meta issue to cover this. I understand this can be a frustrating issue to deal with, and this isn't saying we don't want it fixed (we very much do!), but that we want to better understand how to proceed. Thanks!