[libcxx] Use LIBCXX_EXECUTOR in new test configs

[mstorsjo]

Summary

This allows cross-testing (by setting LIBCXX_EXECUTOR to point
to ssh.py) without making an entirely new test config file.

Implicitly, this also fixes quoting of the python executable name
(which is quoted in test/CMakeLists.txt).

[ldionne]

Can you make similar changes to libcxxabi?

libcxx/test/configs/cmake-bridge.cfg.in

33–34

About these two -- I would actually love to get rid of them. Instead, you could create your own simple .cfg.in file for MingGW where you set the required flags.

I loathe forwarding flags from CMake to the Lit config, IMO that's just unnecessary plumbing.

[mstorsjo]

In D115398#3181137, @ldionne wrote:

Can you make similar changes to libcxxabi?

Sure, I can give that a go tomorrow.

libcxx/test/configs/cmake-bridge.cfg.in

33–34

I know you're not a fan of such settings, but I find them vital. I, on the other hand, loathe setups where it's only possible to build and run things in a finite, fixed set of configurations, instead of passing individual options for configuring one's build.

Sure we can only test and support a finite, fixed set of configurations, but it should be possible for vendors to build and test in other configs by just configuring things with additional flags on top of the existing configs, as long as it's not different enough warranting a separate config.

As a very concrete example; for the clang-cl builds, we currently build and test with setting -DCMAKE_CXX_FLAGS="-D_LIBCPP_HAS_NO_INT128" -DLIBCXX_TEST_COMPILER_FLAGS="-D_LIBCPP_HAS_NO_INT128". I've considered extending it so we'd test both in the default configuration (with filesystem disabled) and this config (with int128 disabled so that msvc environments can test filesystem). That would require adding a separate test config file for that too.

And when such a detail is configured by adding things to CMAKE_CXX_FLAGS, it's best if the corresponding flag for tests can be passed in tandem right next to it, via LIBCXX_TEST_COMPILER_FLAGS to cmake, instead of using a separate test config for it.

[mstorsjo]

libcxx/test/configs/apple-libc++-shared.cfg.in

22

I see that LIBCXX_EXECUTOR already included the python interpreter (the case I tested pointed just straight at a python script) so this failed in CI - when updating it, I'll retry it with leaving out sys.executable insertion here.

[mstorsjo]

Fix failures, don't specify the python interpreter twice.

Take LIBCXXABI_EXECUTOR into account in libcxxabi test configs. (Libcxxabi lacks cmake options for injecting test compiler/linker flags.)

[ldionne]

libcxx/test/configs/cmake-bridge.cfg.in

33–34

Needing to pass additional compiler flags to the test suite is always a way to hack around some deficiency somewhere else. For instance, _LIBCPP_HAS_NO_INT128 comes from:

# TODO: Clang-cl in MSVC configurations don't have access to compiler_rt
# builtins helpers for int128 division. See
# https://reviews.llvm.org/D91139#2429595 for a comment about longterm
# intent for handling the issue. In the meantime, define
# -D_LIBCPP_HAS_NO_INT128 (both when building the library itself and
# when building tests) to allow enabling filesystem for running tests,
# even if it uses a non-permanent ABI.

If we have a proper option that is not a hack, then it should be supported 1st class in features.py or in params.py. I don't want to bloat the number of base substitutions required to run the libc++ test suite in order to facilitate doing hacks -- I'm fine with these hacks remaining in the state of "being a temporary hack".

[mstorsjo]

libcxx/test/configs/cmake-bridge.cfg.in

33–34

Right, _LIBCPP_HAS_NO_INT128 is indeed a bit of a workaround.

I don't want to bloat the number of base substitutions required to run the libc++ test suite

Well, technically, these substitutions aren't base substitutions needed for running the libc++ test suite - you could write a test config from scratch that don't use them, but it's more of a contract between cmake-bridge and the individual configs. But this is splitting hairs...

Anyway, back on track constructively:

• LIBCXX_EXECUTOR isn't essential for the windows test configs, but it's just something I'd appreciate: I'm not a fan of needing to make a duplicate copy of a file (which will be left out of sync when the original changes) just to adjust an option that is supposed to be user settable, when one could just specify it among with the rest of options for the build on the cmake command line.

• For setting defines like _LIBCPP_HAS_NO_INT128, if one sets something like that via CMAKE_CXX_FLAGS when building the library, any user of the library should be setting it in the same way too - so that's brittle indeed. I know this particular case is a bit of a workaround, but more generalized, I think a vendor should be able to easily add custom defines into __config_site. That way, such a custom configuration gets applied consistently both for the library itself and for users of it. Would you be ok with such a patch, adding a cmake option like LIBCXX_EXTRA_SITE_DEFINES?

• Just like for LIBCXX_EXECUTOR, one could want to set specific linker flags with LIBCXX_TEST_LINKER_FLAGS, not about how to link libc++ itself, but for how to link other things (libunwind, libc etc) for the specific testing scenario. (E.g. for cross testing, one wants to link as much as possible statically.) Mainly a case of me wanting to specify things on the cmake command line instead of making a duplicate copy of a test config file that I'd tweak.

[ldionne]

libcxx/test/configs/cmake-bridge.cfg.in

33–34

I like the idea of LIBCXX_EXTRA_SITE_DEFINES because it solves the problem of specifying macros while building the library and while running the test suite (aka when using the library if you're a user) in a single place. I dislike that it makes it easier to keep hacks around, but I guess that's just a side effect. So I'd welcome exploring that.

I'm also fine with LIBCXX_EXECUTOR -- the set of executors is very finite so this is not the same as opening the door for arbitrary craziness.

I don't think LIBCXX_TEST_LINKER_FLAGS has a good reason to exist, and I think we shouldn't support it in this patch -- and we should remove it in a different patch.

The goal of these from-scratch configuration is specifically that they are easy to author. Instead of making them more customizable, and hence more complicated, I want to simplify them. For instance, I know it's really annoying to have to manually include the test/support directory and stuff like that -- we should work on removing that duplication and making from-scratch configs easier to author instead of making them more customizable.

[mstorsjo]

libcxx/test/configs/cmake-bridge.cfg.in

33–34

Ok, good then, I'll try out that approach.

Reducing the amount of boilerplate in the individual config files would be nice - then it'd be a bit less of a hassle to keep extra copies of them around.

[mstorsjo]

Reduced to only handle LIBCXXABI_EXECUTOR

libcxx/test/configs/apple-libc++-shared.cfg.in

 # Testing configuration for Apple's system libc++.
 #
 # This configuration differs from a normal LLVM shared library configuration in
 # that we must use DYLD_LIBRARY_PATH to run the tests against the just-built library,
 # since Apple's libc++ has an absolute install_name.
 #
 # We also don't use a per-target include directory layout, so we have only one
 # include directory for the libc++ headers.
 
-import sys
-
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -isystem %{install}/include/c++/v1 -I %{libcxx}/test/support -include %{libcxx}/test/support/nasty_macros.h'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{install}/lib -lc++'
 ))
 config.substitutions.append(('%{exec}',
-    '{} %{{libcxx}}/utils/run.py --execdir %T --env DYLD_LIBRARY_PATH=%{{install}}/lib -- '.format(sys.executable)
+    '%{executor} --execdir %T --env DYLD_LIBRARY_PATH=%{install}/lib -- '
 ))
 
 import os, site
 site.addsitedir(os.path.join('@LIBCXX_SOURCE_DIR@', 'utils'))
 import libcxx.test.params, libcxx.test.newconfig
 libcxx.test.newconfig.configure(
     libcxx.test.params.DEFAULT_PARAMETERS,
     libcxx.test.features.DEFAULT_FEATURES,
     config,
     lit_config
 )

libcxx/test/configs/cmake-bridge.cfg.in

 
 # Add substitutions for bootstrapping the test suite configuration
 config.substitutions.append(('%{cxx}', '@CMAKE_CXX_COMPILER@'))
 config.substitutions.append(('%{libcxx}', '@LIBCXX_SOURCE_DIR@'))
 config.substitutions.append(('%{install}', '@CMAKE_BINARY_DIR@'))
 config.substitutions.append(('%{include}', '%{install}/@LIBCXX_INSTALL_INCLUDE_DIR@'))
 config.substitutions.append(('%{target-include}', '%{install}/@LIBCXX_INSTALL_INCLUDE_TARGET_DIR@'))
 config.substitutions.append(('%{lib}', '%{install}/@LIBCXX_INSTALL_LIBRARY_DIR@'))
+config.substitutions.append(('%{executor}', '@LIBCXX_EXECUTOR@'))

libcxx/test/configs/ibm-libc++-shared.cfg.in

 # This testing configuration handles running the test suite against libc++ on
 # AIX using a shared library.
 #
 
-import sys
-
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', ''))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -D__LIBC_NO_CPP_MATH_OVERLOADS__ -isystem %{install}/include/c++/v1 -I %{libcxx}/test/support -include %{libcxx}/test/support/nasty_macros.h'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{install}/lib -lc++ -lc++abi -latomic -Wl,-bbigtoc'
 ))
 config.substitutions.append(('%{exec}',
-    '{} %{{libcxx}}/utils/run.py --execdir %T --env LIBPATH=%{{install}}/lib -- '.format(sys.executable)
+    '%{executor} --execdir %T --env LIBPATH=%{install}/lib -- '
 ))
 
 # LIBCXX-AIX-FIXME is the feature name used to XFAIL the
 # initial AIX failures until they can be properly diagnosed
 # and fixed. This allows easier detection of new test failures
 # and regressions. Note: New failures should not be suppressed
 # using this feature. (Also see llvm.org/PR52188)
 config.available_features.add('LIBCXX-AIX-FIXME')

libcxx/test/configs/llvm-libc++-shared-gcc.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a shared library, with GCC. This is done differently from Clang because
 # GCC does not support the -nostdlib++ command-line flag.
 
-import sys
-
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', ''))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -include %{libcxx}/test/support/nasty_macros.h'
 ))
 config.substitutions.append(('%{link_flags}',
     '-L %{lib} -Wl,-rpath,%{lib} -nodefaultlibs -lc++ -lm -lgcc_s -lgcc -lpthread -lc -lgcc_s -lgcc -latomic'
 ))
 config.substitutions.append(('%{exec}',
-    '{} %{{libcxx}}/utils/run.py --execdir %T -- '.format(sys.executable)
+    '%{executor} --execdir %T -- '
 ))
 
 import os, site
 site.addsitedir(os.path.join('@LIBCXX_SOURCE_DIR@', 'utils'))
 import libcxx.test.params, libcxx.test.newconfig
 libcxx.test.newconfig.configure(
     libcxx.test.params.DEFAULT_PARAMETERS,
     libcxx.test.features.DEFAULT_FEATURES,
     config,
     lit_config
 )

libcxx/test/configs/llvm-libc++-shared.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a shared library.
 
-import sys
-
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -include %{libcxx}/test/support/nasty_macros.h'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -Wl,-rpath,%{lib} -lc++ -pthread'
 ))
 config.substitutions.append(('%{exec}',
-    '{} %{{libcxx}}/utils/run.py --execdir %T -- '.format(sys.executable)
+    '%{executor} --execdir %T -- '
 ))
 
 import os, site
 site.addsitedir(os.path.join('@LIBCXX_SOURCE_DIR@', 'utils'))
 import libcxx.test.params, libcxx.test.newconfig
 libcxx.test.newconfig.configure(
     libcxx.test.params.DEFAULT_PARAMETERS,
     libcxx.test.features.DEFAULT_FEATURES,
     config,
     lit_config
 )

libcxx/test/configs/llvm-libc++-static.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a static library.
 
-import sys
-
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -include %{libcxx}/test/support/nasty_macros.h'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -lc++ -lc++abi -pthread'
 ))
 config.substitutions.append(('%{exec}',
-    '{} %{{libcxx}}/utils/run.py --execdir %T -- '.format(sys.executable)
+    '%{executor} --execdir %T -- '
 ))
 
 import os, site
 site.addsitedir(os.path.join('@LIBCXX_SOURCE_DIR@', 'utils'))
 import libcxx.test.params, libcxx.test.newconfig
 libcxx.test.newconfig.configure(
     libcxx.test.params.DEFAULT_PARAMETERS,
     libcxx.test.features.DEFAULT_FEATURES,
     config,
     lit_config
 )

libcxxabi/test/configs/apple-libc++abi-shared.cfg.in

 # Testing configuration for Apple's system libc++abi.
 
-import sys
-
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
 ))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -isystem %{install}/include/c++/v1 -DLIBCXXABI_NO_TIMER -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS ' +
     '-I %{libcxx}/test/support -I %{libcxx}/src'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{install}/lib -lc++ -lc++abi'
 ))
 config.substitutions.append(('%{exec}',
-    '{} %{{libcxx}}/utils/run.py --execdir %T --env DYLD_LIBRARY_PATH=%{{install}}/lib -- '.format(sys.executable)
+    '%{executor} --execdir %T --env DYLD_LIBRARY_PATH=%{install}/lib -- '
 ))
 
 import os, site
 site.addsitedir(os.path.join('@LIBCXXABI_LIBCXX_PATH@', 'utils'))
 import libcxx.test.params, libcxx.test.newconfig, libcxx.test.newconfig
 libcxx.test.newconfig.configure(
     libcxx.test.params.DEFAULT_PARAMETERS,
     libcxx.test.features.DEFAULT_FEATURES,
     config,
     lit_config
 )

libcxxabi/test/configs/cmake-bridge.cfg.in

 config.test_exec_root = '@CMAKE_BINARY_DIR@'
 
 # TODO: This is a non-standard Lit attribute and we should have another way of accessing this.
 config.host_triple = '@LLVM_HOST_TRIPLE@'
 
 config.substitutions.append(('%{cxx}', '@CMAKE_CXX_COMPILER@'))
 config.substitutions.append(('%{libcxx}', '@LIBCXXABI_LIBCXX_PATH@'))
 config.substitutions.append(('%{install}', '@CMAKE_BINARY_DIR@'))
+config.substitutions.append(('%{executor}', '@LIBCXXABI_EXECUTOR@'))

libcxxabi/test/configs/ibm-libc++abi-shared.cfg.in

 # Testing configuration for libc++abi on AIX.
 
-import sys
-
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}',''))
 config.substitutions.append(('%{compile_flags}',
     '-nostdinc++ -isystem %{install}/include/c++/v1 ' + 
     '-D__LIBC_NO_CPP_MATH_OVERLOADS__ -DLIBCXXABI_NO_TIMER ' +
     '-D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS ' +
     '-I %{libcxx}/test/support -I %{libcxx}/src'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{install}/lib -lc++ -lc++abi -Wl,-bbigtoc'
 ))
 config.substitutions.append(('%{exec}',
-    '{} %{{libcxx}}/utils/run.py --execdir %T --env LIBPATH=%{{install}}/lib -- '.format(sys.executable)
+    '%{executor} --execdir %T --env LIBPATH=%{install}/lib -- '
 ))
 
 import os, site
 site.addsitedir(os.path.join('@LIBCXXABI_LIBCXX_PATH@', 'utils'))
 import libcxx.test.params, libcxx.test.newconfig, libcxx.test.newconfig
 libcxx.test.newconfig.configure(
     libcxx.test.params.DEFAULT_PARAMETERS,
     libcxx.test.features.DEFAULT_FEATURES,
     config,
     lit_config
 )