Mordante added a comment.

In D126907#3739923 <https://reviews.llvm.org/D126907#3739923>, @aaron.ballman 
wrote:

> In D126907#3739750 <https://reviews.llvm.org/D126907#3739750>, @Mordante 
> wrote:
>
>> In D126907#3738383 <https://reviews.llvm.org/D126907#3738383>, @erichkeane 
>> wrote:
>>
>>> There was a test I dealt with previously where a ton of the header files 
>>> were run with modules enabled (and an auto generated files), so I'm shocked 
>>> to find there is MORE differences here.  @ldionne : This is another example 
>>> where trying to test libcxx is particularly difficult to do apparently.
>>
>> I disagree; testing libcxx is easy.
>
> FWIW, that's not been my experience. I can't even get libcxx to build for me 
> on Windows with Visual Studio cmake integration, let alone test it.

Fair point, I can't vouch for how well that build is supported, since I don't 
have access to Windows. This version isn't tested in libc++ CI. The other two 
Windows builds `CMake + ninja (MSVC)` and `CMake + ninja (MSVC)` are tested in 
libc++ CI. The wording on the website is similar to what is used in libc++ CI 
`libcxx/utils/ci/run-buildbot` but it misses the ` 
-DLIBCXX_EXTRA_SITE_DEFINES="_LIBCPP_HAS_NO_INT128"`.

>> Unfortunately what's missing is good documentation explaining how to test 
>> different configurations. A similar libc++ related issue came up in another 
>> Clang review recently where the libc++ CI setup was unclear. Afterwards I 
>> had a talk with @aaron.ballman and I agreed to improve the documentation. 
>> While improving the documentation I noticed some issues with our CI setup. 
>> So before publishing documentation that doesn't reflect reality, I first 
>> wanted to fix these issues. While testing my fixes I ran into the build 
>> failure due to this patch. So now we're at a full circle.
>
> And I greatly appreciate the improvements that have been going on here!

You're welcome!

In D126907#3739967 <https://reviews.llvm.org/D126907#3739967>, @erichkeane 
wrote:

> In D126907#3739750 <https://reviews.llvm.org/D126907#3739750>, @Mordante 
> wrote:
>
>> If you need further assistance feel free to reach out to me or other libc++ 
>> developers, the easiest way to reach us is #libcxx on Discord.
>
> This is the 3rd time some non-obvious-how-to-run libcxx test failure has 
> caused a revert request to this patch, which is obviously getting 
> frustrating. Additionally, in none of the 3 cases was I alerted automatically.

I understand that it's frustrating. I haven't seen the other two reverts, but 
by default non libc++ patches aren't tested in the libc++ pre-commit CI. The 
reason is simple; a pre-commit build runs 50+ builds and takes about one hour 
to complete. So the CI doesn't have the capacity to test every Clang diff. If 
you want to run the libc++ pre-commit CI the easiest way is to add a dummy file 
in the libc++ tree. Note that will add the libc++ review group to this review.

Next to testing pre-commit changes it tests main every 4 hours, but doesn't 
send e-mails when an error occurs. This is something @ldionne wanted to look 
into.

This behaviour is exactly the kind of documentation that is missing for Clang 
developers and what @aaron.ballman and I talked about. (There is more 
documentation that should be added, but that is generic information.)

> I appreciate that there are actually ways to RUN these tests on my machine 
> (Aaron is not as lucky, see above), but the lack of a "test all the libcxx 
> things" flag is shocking.  It seems that I'd need at least 2 separate CMake 
> directories to test these configurations?  That seems shocking to me.

You can use one libc++ installation to test multiple different configuration as 
long as they don't use different build-time options. So testing with the 
different language standards c++03, c++11, c++17, c++20, c++2b, the modular 
build, and several other options can be done from one build. But you need to 
invoke the tests for each option or combination of options separately.

If you want to test without `wchar_t` support you need to build a different 
libc++, simply since that removes parts of the dylib, like the `wchar_t` facets 
from the locales. (Basically the same as when you want to test Clang with or 
without assertions, there you need two builds too.)

What do you exactly mean with "test all the libcxx things" flag?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to