Page MenuHomePhabricator

[libc++] Use explicit module cache path in tests
Needs ReviewPublic

Authored by abrachet on Jun 11 2022, 9:43 PM.

Details

Summary

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.

Diff Detail

Event Timeline

abrachet created this revision.Jun 11 2022, 9:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2022, 9:43 PM
abrachet requested review of this revision.Jun 11 2022, 9:43 PM
phosek accepted this revision.Jun 12 2022, 11:21 AM

LGTM but I'm not libc++ reviewer so you should wait for LGTM from either @ldionne or @EricWF.

This revision is now accepted and ready to land.Jun 12 2022, 11:21 AM

@ldionne ping would it be possible to take a look?

This revision was landed with ongoing or failed builds.Jun 14 2022, 11:08 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 14 2022, 11:08 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne reopened this revision.Jun 14 2022, 12:24 PM

@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)?

@abrachet Please don't commit patches to libc++ without having approval from the libc++ review group.

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!

@abrachet Please don't commit patches to libc++ without having approval from the libc++ review group.

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.

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)?

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?