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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
