[clang] [clang][driver] Use TheDriver.ClangExecutable in SetInstallDir to point to the actual install folder (PR #68091)

Martin Storsjö via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 4 09:21:47 PDT 2023


mstorsjo wrote:

> Without any other reference, I checked the Apple clang, on my macOS:
> 
> ```
> ilg at wksi ~ % /usr/bin/clang -v
> Apple clang version 14.0.0 (clang-1400.0.29.202)
> Target: x86_64-apple-darwin21.6.0
> Thread model: posix
> InstalledDir: /Library/Developer/CommandLineTools/usr/bin
> 
> 
> ilg at wksi ~ % mkdir tmp/a 
> ilg at wksi ~ % ln -s /usr/bin/clang tmp/a
> 
> ilg at wksi ~ % tmp/a/clang -v
> Apple clang version 14.0.0 (clang-1400.0.29.202)
> Target: x86_64-apple-darwin21.6.0
> Thread model: posix
> InstalledDir: /Library/Developer/CommandLineTools/usr/bin
> ```
> 
> So Apple decided to **follow the links**, and, in my opinion, this is the less ambiguous and expected behaviour.

No, this is a side effect of something else. Note, `/usr/bin/clang` isn't a symlink to `/Library/Developer/CommandLineTools/usr/bin/clang`, but it is a wrapper executable that executes the latter (after setting some env variables, iirc, and after locating where it is installed - it can also be in an Xcode.app bundle somewhere).

Let's recreate your experiment like this:
```console
% mkdir -p tmp/a
% ln -s /Library/Developer/CommandLineTools/usr/bin/clang tmp/a
% tmp/a/clang -v
Apple clang version 12.0.5 (clang-1205.0.22.9)
Target: arm64-apple-darwin21.6.0
Thread model: posix
InstalledDir: /Users/martin/tmp/a
```

There you see that Apple's clang also behaves exactly the same, if you disregard the external wrapper executable.

> Another way to rephrase the question: the current implementation that preserves the symlinks, was it an explicitly required feature, or rather a side effect that came into existence by accident when implementing the current tests?

Regardless of whether it was intentional or not, due to Hyrum's law, it's quite possible that someone has ended up depending on the previous defacto behaviour.

We did a similar cleanup for llvm-rc not long ago, and llvm-rc has much much fewer users than clang. In 8c6a0c8bf50bca6c3a0c4de1b84f21466ee31655 we changed llvm-rc to explicitly look up the executable path (instead of looking for what would be found in `$PATH`, essentially finding the source end of symlinks - just like what clang does right now). But this broke Google's internal uses of the tool, so in e4eb8d97e8afcb879dc5cd0da7a937dbb26fbf12 we reverted to checking both paths, essentially testing both directories.

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


More information about the cfe-commits mailing list