[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

Bruno Cardoso Lopes via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 13 18:08:13 PDT 2022


bruno added a comment.

> I'm not sure how to deal with missing `env -u`.
>
> - We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's interpretation of empty string for this variable. I'm not sure if the current behaviour (there will be no module cache in the cc1 at all) is intentional or useful.  Hesitant to change this behaviour.

How about using `self.with_environment('CLANG_MODULE_CACHE_PATH', '')` so we don't need to worry about using `env -u` to unset? My understand is that (1) `getDefaultModuleCachePath` is the only place using `CLANG_MODULE_CACHE_PATH`, and (2) `std::getenv` return nullptr if it's empty, which will fallback to system provided path instead.

> - We could try to enumerate all the environments that don't support `env -u` and disable these two tests on  those platforms.  So far it was just one AIX bot, but I wouldn't be surprised if other less commonly used unixes have the same issue.
>
> - We could make the command inscrutable, like `not env -u X true || env -u ... real command ...` so that if `env -u X true` fails (presumably due to not supporting `-u` option) we won't run the rest of the RUN line.

Not sure it helps much but according to `option-X.test`, `system-aix` support `unset`.

> If someone has a suggestion for a simple fix, I can try again.  But otherwise I doubt it's worth putting much time into this.

Thanks for trying to improve this :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133622/new/

https://reviews.llvm.org/D133622



More information about the cfe-commits mailing list