Page MenuHomePhabricator

[LIBCXX] add possibility to load an additional test configuration to CMake
AbandonedPublic

Authored by gargaroff on Jun 15 2020, 12:53 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Summary

Give the user the possibility to load an additional configuration
when setting up the testing environment, which is loaded after the
auto-generated configuration.

Diff Detail

Event Timeline

gargaroff created this revision.Jun 15 2020, 12:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2020, 12:53 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Thanks for the patch! I had been thinking about ways to bypass the default configuration method for a while and I had a local patch brewing for a while.

I've posted it at https://reviews.llvm.org/D81846, please let me know what you think.

The one downside I see with this approach here is that it's still always using the default (old) config all the time, whereas D81846 allows bypassing it completely.

Oh, I just noticed that I forgot to put the link to this Diff in the email earlier. Sorry about that!

I've posted it at https://reviews.llvm.org/D81846, please let me know what you think.

The one downside I see with this approach here is that it's still always using the default (old) config all the time, whereas D81846 allows bypassing it completely.

Might it make sense to combine both of our patches? I certainly like your patch as it gets right of the old config and the need to use lit directly if a custom config should be used. However I also like the ability of not having to copy-paste all of the boilerplate from the default config just to set up some additional parameters that I care about. Therefore I would love to see both approaches combined.

By the way, if you know some additional reviewers which would be interested in this, feel free to add them to the discussion!

From the comments in https://reviews.llvm.org/D81846, it looks like this isn't needed anymore. Let's abandon this review to clear up the review queue.

Thanks for the discussion!

gargaroff abandoned this revision.Jun 18 2020, 9:38 AM

From the comments in https://reviews.llvm.org/D81846, it looks like this isn't needed anymore. Let's abandon this review to clear up the review queue.

Thanks for the discussion!

Yep, thank you for working on making all of our lives easier!
I'll be responding to some of your questions in the other review tomorrow if I get the time. Otherwise next week.