For reference, this test creates about 1.5G in the cache directory. By default this will go to ~/.cache/clang/ which can fill up quick. This changes the test to put the cache path in lit temp directories. Size considerations aside it makes sense for tests to be hermetic and not touch global system state.
Details
- Reviewers
phosek EricWF abrachet - Group Reviewers
Restricted Project - Commits
- rG93c46e9632a6: [libc++] Use explicit module cache path in tests
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
@abrachet Please don't commit patches to libc++ without having approval from the libc++ review group.
Thanks for wanting to improve the test suite -- this is indeed a slow test that creates a lot of stuff on the side. My main comment is actually whether it would make sense to reuse a modules-cache path in the whole test suite (even across tests)?
By the way, I'm not scolding, just making sure everybody follows the process :-). Let's have a discussion now that the immediate problem is solved!
No worries I'm not feeling scolded :) I will wait next time.
Sure I could look into that. I'm not too familiar with libcxx's testing infrastructure, do you reckon that would be added to all of the %{compile_flags} substitutions?
[Github PR transition cleanup]
I think this is reasonable enough that we don't need to look into changing anything. I think it would be better to define a substitution like %{modules-cache} which is global to the test suite and then use -fmodules-cache-path=%{modules-cache} here in this test and also in params.py (where we currently define a modules cache path for clang-modules builds).
@abrachet If you want to investigate this, please open that review as a GH PR and we'll have a look. But otherwise the current state is acceptable as well.
I will commandeer this to close it to clear the review queue.