[clang] [clang] Canonicalize absolute paths in dependency file (PR #117458)

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 2 23:43:26 PST 2025


kadircet wrote:

It's hard to say what's the right/wrong behavior when it comes to symlink handling, but I am having a hard time understanding what kind of applications can benefit from this new behavior. AFAICT most workflows that involve dependency files use it for "caching" purposes, and some use it to for shipping dependencies to a remote build environment.

As some lit test changes also demonstrate, after this change a symlink like `b/header.h` -> `a/header.h` won't be captured in the dep output. So if the user modifies `b/header.h`, build systems won't invalidate their caches, or a remote build system that sets up a directory layout will fail compilations due to missing header `b/header.h` (as that's how clang will try to retrieve this file).

Hence I think clang's previous behavior that preserved symlinks was actually the only desired behavior for current applications, AFAICT there's no way to correctly capture all of this dependencies if we resolve symlinks. I'd argue that your issue is with the downstream consumers of these deps files and needs to be addressed at their layer.

If you think I misunderstood the situation around what's broken in the downstream users, can you please clarify why do you think the new behavior is "correct"? Otherwise I think we should revert this change to make sure we don't cause breakages that won't be detected until a new release.

https://github.com/llvm/llvm-project/pull/117458


More information about the cfe-commits mailing list