Canonicalize path_args src

[nmattia]

Fixes #7078.

When cargo registers information used later on to figure out whether or not to recompile a unit, the path to the source is tracked in the fingerprint. However this path is not canonicalized, meaning that e.g. symlinks aren't resolved before the source paths are added to the fingerprint.

See #7078 for a full description of the issue.

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

Thanks for the PR! I don't think I'm entirely clear on the scenarios though of where this causes issues. Could you perhaps include a test which exhibits the behavior that this is fixing?

[nmattia]

@alexcrichton sure, will do! Looks like the Windows build is failing; the canonical versions of the paths don't seem to be recognized by cargo. Is this a known issue? Should the canonicalizing step be skipped on Windows? (edit: that would make sense since symlinks are anyway a bit of a tricky thing on Windows)

[nmattia]

I wrote a test and rearranged history a bit. I pushed the test and I'll wait for CI to finish (fail) before I re-push the fix.

The test shows that if a crate has a local dependency on crate a, and if a is removed and replaced with a symlink to another crate b/ (with name = "a"), cargo won't rebuild the dependency. By resolving symlinks first we don't lose much performance but yet work around this edge case.

The use case that interests me more is the other way around: the dependency a was already built but the symlink to it changed (before the fix cargo will rebuild a even though it's the exact same content). I could only reproduce when using source replacement, which I didn't quite manage to figure out in the tests (rust newbie here).

This second use case is what allows me to pre build dependencies on CI, which would most likely solve #1139 (see https://github.com/nmattia/naersk/blob/6b41d979039e06eef6fbe940d439c9f09427f11a/default.nix#L57-L61 for a nix solution).

[nmattia]

The job indeed failed, see: https://travis-ci.com/rust-lang/cargo/jobs/213020941#L2554

[alexcrichton]

Hm ok I think I see what's going on here. The problem is that when detecting whether to rebuild something we're not taking into account any symlinks resolved along the way. Ideally we need to not only take into account the mtime of the file that rustc has read but also the mtime of any symlink inbetween.

I don't think that this is the right fix since it only handles renames of the project root, and not renames of files underneath the project root which deal with symlinks. That being said this seems like it's very difficult to fix. I'm not really sure if there's a great fix for this other than content-based fingerprinting.

[nmattia]

Oh, content-based fingerprinting would be awesome! Is there any reason why this isn't currently implemented? Performance concerns?

[alexcrichton]

There's more information about it in #6529, but it wasn't implemented initially due to performance concerns yeah.

[alexcrichton]

Haven't heard back from you in awhile @nmattia so I'm gonna close this, but feel free to resubmit with comments addressed!

[nmattia]

I ended working around this and forgot to close. Thanks for the feedback!