[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable
Fangrui Song via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 8 13:36:29 PST 2023
MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.
In D144179#4166909 <https://reviews.llvm.org/D144179#4166909>, @mgabka wrote:
> In D144179#4146599 <https://reviews.llvm.org/D144179#4146599>, @MaskRay wrote:
>
>> This looks like introducing a footgun (when something behaves differently from an upstream Clang, it would be difficult for toolchain maintainers to know why).
>> Why can't your user specify `CLANG_CONFIG_FILE_SYSTEM_DIR`?
>
> hi @MaskRay,
> The reason we wanted to suggest use of environment variable is that the CLANG_CONFIG_FILE_SYSTEM_DIR is only defined at compilation time, after discussing it once again we would rather lean towards introducing an environment variable with similar semantics as CLANG_CONFIG_FILE_SYSTEM_DIR or rather ``CLANG_CONFIG_FILE_USER_DIR``, the motivation here is that it will allow to specify the directory to search for config files in a dynamic way, without need to recompile the compiler.
> It is for user convenience in situations when they are using a system wide installation in a location where they do not have access right, and the ``CLANG_CONFIG_FILE_SYSTEM_DIR`` and ``CLANG_CONFIG_FILE_USER_DIR`` were not defined at build time.
>
> We realised that environment variables are already used in this area, for example CLANG_NO_DEFAULT_CONFIG, so adding another one is not breaking existing convention.
>
> What do you think about it?
Environment variables are evil and make troubleshooting difficult. `CLANG_NO_DEFAULT_CONFIG` disables configuration files and makes the behavior is less magical, so I am fine with it.
It's different from the environment variable being introduced in this patch.
If your users want to use a system installed clang with an environment variable, why not give them a Clang wrapper like:
#!/bin/zsh
exec path/to/clang ${CONFIG_FILE:+--config=$CONFIG_FILE "$@"
You can even replace the current `clang` symlink with the wrapper.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144179/new/
https://reviews.llvm.org/D144179
More information about the cfe-commits
mailing list