Use canonical paths for checking equality #3489
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
| p.build(); | ||
|
|
||
| assert_that(p.cargo("build").cwd(p.root().join("foo")), execs().with_status(0)); | ||
| assert_that(p.cargo("build").cwd(p.root().join("bar")), execs().with_status(0)); |
There was a problem hiding this comment.
With current cargo, this invocation will fail with bar believes it is in workspace foo, but foo does not think so. This happens because paths are compared, and they look like foo/Cargo.toml and bar/../foo/Cargo.toml.
|
@alexcrichton this is a stop-gap solution. A better solution I have in mind is to introduce a What do you think? |
matklad
force-pushed
the
rel-ws
branch
2 times, most recently
from
b1b5ba7 to
bd2ea58
Compare
|
Hm, windows tests fail because |
|
It sounds like we may not need literally canonical paths (e.g. a syscall) but perhaps just a normalization instead? |
|
In general I'm very wary of reinterpreting paths as it seems to always introduce a bug one way or another, so I'd be interested in slicing this down to as few normalized paths as possible if we can. |
|
Yes, that's most reasonable! I've actually tried to find a path normalization crate, but it didn't occur to me that this function may already be present in Cargo :) |
|
Hm, I think there's at least one plausible case where the normalization is not enough. If the filesystem is case-insensitive, paths in |
|
@bors: r+ Looks good to me, thanks! I feel like it's ok if Cargo doesn't dive that deeply into filesystem weirdness. At some point you can also just configure it a little differently and have it work out... |
|
📌 Commit 083da14 has been approved by |
Use canonical paths for checking equality
|
☀️ Test successful - status-appveyor, status-travis |
|
@alexcrichton an unrelated question: am I correct that using |
No description provided.