[libcxx] Teach libcxx's lit configuration new ways to find lit.site.cfg

[EricWF]

Summary

Currently to run tests in tree you need to symlink the lit.site.cfg file generated by the cmake build into the source tree, and teach your VCS to ignore it.

This allows the user to specify where to find the lit.site.cfg file two different ways:

• lit_site_config lit parameter
• LIT_SITE_CONFIG enviroment variable.

example usage:

lit -sv --param=libcxx_site_config=path/to/libcxx-build/test/lit.site.cfg path/to/tests

Or

export LIBCXX_SITE_CONFIG=path/to/libcxx-build/test/lit.site.cfg
lit -sv path/to/tests

The command line parameter will override the environment variable.
If neither options are present a warning is issued and the lit.cfg file is loaded directly.

[EricWF]

Add error handling for when a bad lit.site.cfg value is found.

[danalbert]

Should this be a feature of LIT proper? $ lit --config path/to/lit.cfg test/dir

The environment variable should stay libc++ specific though.

[EricWF]

In D6255#6, @danalbert wrote:

Should this be a feature of LIT proper? $ lit --config path/to/lit.cfg test/dir

I don't think it can be for a couple of reasons.

First, the way tests are discovered does not seem to allow the config file to be specified manually. I tried implementing it but it just doesn't seem to fit. The reason this works is because the lit.site.cfg file is found naturally through lit's test discovery.

Second, LIT can be invoked on multiple test suites at once. The lit.site.cfg file is specific to a single suite. I don't see how you could specify the configuration file unambiguously. For example:

lit -sv --config path/to/config/lit.site.cfg path/to/libcxx/test/ path/to/clang/test/

I hope that cleared things up.

[jroelofs]

Is this compatible with --config-prefix=?

test/lit.site.cfg

1 ↗

(On Diff #16181)

We don't usually put the vim part of this in llvm things, do we?

[EricWF]

I actually think that by preloading the LIT test suite cache I can make this a proper LIT feature. Sorry @danalbert, you were right. However I can't keep the environment variable libc++ specific.

[EricWF]

So I have a patch for LIT proper that does something like this. However I'm not sure it fits nicely into LIT. I'll put it up for review and get @ddunbar's opinion of it. It may just be best to handle this on our end.

[danalbert]

test/lit.site.cfg

1 ↗

(On Diff #16181)

Should be in all of these imo (actually, it should just be a .py, but this will do).

[EricWF]

I think this is the better direction to actually go. clang's lit.cfg does something similar to this but it uses llvm-config to find lit.site.cfg. I don't think changing LIT proper is the right way to go.

Pinging this patch.

[EricWF]

I updated the patch to function almost exactly the way clang's lit.cfg does it.

[danalbert]

CMakeLists.txt

93

This one changed from BINARY_DIR to CURRENT_BINARY_DIR. Was that intentional? Why do we need both LIBCXX_BINARY_DIR and LIBCXX_LIBRARY_DIR?

94

Ignore the above comment (phabricator keeps locking up whenever I try to delete or edit it).

This one is always joined with '/lib' when it's used. Should just do the join here instead (or move it into lib/CMakeLists.txt).

test/lit.cfg

567

This will never be true, right?

If I understand what you've done here, you've inverted the process of loading the LIT configs. Instead of the lit.site.cfg delegating to lit.cfg, lit.cfg now loads lit.site.cfg. That makes more sense to me, but this check is a no-op then (since we don't have the config loaded). Don't you also need to remove the load_config in lit.site.cfg.in?

572

Is test_exec_root something you've added? If so, it doesn't need to be a part of the config AFACT. If not, where was it being configured before?

589

You're actually raising a class here, not an exception. This would catch on except type as ex rather than except SystemExit as ex :)

[EricWF]

Address most of @danalbert's comments. Changes incoming.

CMakeLists.txt

94

This one is always joined with '/lib' when it's used. Should just do the join here instead (or move it into lib/CMakeLists.txt).

Yeah, probably. I'll make that change.

test/lit.cfg

567

libcxx_obj_root can be present in config at this point.

The make check-libcxx rule actually loads the lit.site.cfg first. This sets libcxx_obj_root, and we don't need to perform any magic to find the build directory.However if you try and run the tests from the source directory then libcxx_obj_root won't be present, and we need to search for and load the site configuration.

Don't you also need to remove the load_config in lit.site.cfg.in?

No. In both cases we need the lit.site.cfg to load lit.cfg. When lit.cfg is the first file loaded, the lit.site.cfg recursively re-loads lit.cfg and the branch on #572 is skipped.

572

test_exec_root and test_source_root are documented members of the config object. They are documented as follows:

• test_source_root The filesystem path to the test suite root. For out-of-dir builds this is the directory that will be scanned for tests.
• test_exec_root For out-of-dir builds, the path to the test suite root inside the object directory. This is where tests will be run and temporary output files placed.

Right now these are unused by our test format, but we should probably set them so we can use other test formats in lit.

http://llvm.org/docs/CommandGuide/lit.html#test-suites

589

Thanks! This needs to be fixed in Clang's lit.cfg as well.

[EricWF]

Address most of @danalbert's comments. Changes incoming.

[EricWF]

test/lit.cfg

589

Actually I think we are both wrong. raise SystemExit seems to be equivalent to raise SystemExit(). If any arguments are added then lit.TestingConfig.load_from_path will exit before running any of the tests. I'll add the braces for clarity.

https://github.com/llvm-mirror/llvm/blob/master/utils/lit/lit/TestingConfig.py#L99

[EricWF]

Address @danalbert's comments. I also sneaked in the removal of config.python_executable which is entirely unused.

[danalbert]

LGTM.

test/lit.cfg

589

Oh, interesting.

CMakeLists.txt

 set(LIBCXX_TARGET_TRIPLE ${LIBCXX_TARGET_TRIPLE} CACHE STRING "Target triple.")
 
 
+set(LIBCXX_COMPILER    ${CMAKE_CXX_COMPILER})
+set(LIBCXX_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
+set(LIBCXX_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
+set(LIBCXX_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib)
+
 # Declare libc++ configuration variables.
 # They are intended for use as follows:
 # LIBCXX_CXX_FLAGS: General flags for both the compiler and linker.

test/CMakeLists.txt

   set(LIT_ARGS "${LLVM_LIT_ARGS}")
   separate_arguments(LIT_ARGS)
 
-  set(LIBCXX_COMPILER ${CMAKE_CXX_COMPILER})
-  set(LIBCXX_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/..)
-  set(LIBCXX_BINARY_DIR ${CMAKE_BINARY_DIR})
-  set(PYTHON_EXECUTABLE ${PYTHON_EXECUTABLE})
   pythonize_bool(LIBCXX_ENABLE_EXCEPTIONS)
   pythonize_bool(LIBCXX_ENABLE_RTTI)
   pythonize_bool(LIBCXX_ENABLE_SHARED)

test/lit.cfg

         self.cxx = None
         self.src_root = None
         self.obj_root = None
+        self.library_root = None
         self.env = {}
         self.compile_flags = ['-nostdinc++']
         self.link_flags = ['-nodefaultlibs']

         self.configure_triple()
         self.configure_src_root()
         self.configure_obj_root()
+        self.configure_library_root()
         self.configure_use_system_lib()
         self.configure_use_clang_verify()
         self.configure_env()

     def configure_obj_root(self):
         self.obj_root = self.get_lit_conf('libcxx_obj_root', self.src_root)
 
+    def configure_library_root(self):
+        self.library_root = self.get_lit_conf('libcxx_library_root', self.obj_root)
+
     def configure_use_system_lib(self):
         # This test suite supports testing against either the system library or
         # the locally built one; the former mode is useful for testing ABI

         # Configure library search paths
         abi_library_path = self.get_lit_conf('abi_library_path', '')
         if not self.use_system_lib:
-            self.link_flags += ['-L' + self.obj_root + '/lib']
-            self.link_flags += ['-Wl,-rpath,' + self.obj_root + '/lib']
+            self.link_flags += ['-L' + self.library_root]
+            self.link_flags += ['-Wl,-rpath,' + self.library_root]
         if abi_library_path:
             self.link_flags += ['-L' + abi_library_path,
                                 '-Wl,-rpath,' + abi_library_path]

 
     def configure_env(self):
         if sys.platform == 'darwin' and not self.use_system_lib:
-            self.env['DYLD_LIBRARY_PATH'] = os.path.join(self.obj_root, 'lib')
+            self.env['DYLD_LIBRARY_PATH'] = self.library_root
 
 # name: The name of this test suite.
 config.name = 'libc++'

 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
 
+# Infer the test_exec_root from the libcxx_object root.
+libcxx_obj_root = getattr(config, 'libcxx_obj_root', None)
+if libcxx_obj_root is not None:
+    config.test_exec_root = os.path.join(libcxx_obj_root, 'test')
+
+# Check that the test exec root is known.
+if config.test_exec_root is None:
+    # Otherwise, we haven't loaded the site specific configuration (the user is
+    # probably trying to run on a test file directly, and either the site
+    # configuration hasn't been created by the build system, or we are in an
+    # out-of-tree build situation).
+    site_cfg = lit_config.params.get('libcxx_site_config',
+                                     os.environ.get('LIBCXX_SITE_CONFIG'))
+    if not site_cfg:
+        lit_config.warning('No site specific configuration file found!'
+                           ' Running the tests in the default configuration.')
+    elif not os.path.isfile(site_cfg):
+        lit_config.fatal(
+            "Specified site configuration file does not exist: '%s'"  %
+            site_cfg)
+    else:
+        lit_config.note('using site specific configuration at %s' % site_cfg)
+        lit_config.load_config(config, site_cfg)
+        raise SystemExit()
+
+
 cfg_variant = getattr(config, 'configuration_variant', '')
 if cfg_variant:
     print 'Using configuration variant: %s' % cfg_variant

test/lit.site.cfg.in

 config.std                      = "@LIBCXX_STD_VERSION@"
 config.libcxx_src_root          = "@LIBCXX_SOURCE_DIR@"
 config.libcxx_obj_root          = "@LIBCXX_BINARY_DIR@"
-config.python_executable        = "@PYTHON_EXECUTABLE@"
+config.libcxx_library_root      = "@LIBCXX_LIBRARY_DIR@"
 config.enable_exceptions        = "@LIBCXX_ENABLE_EXCEPTIONS@"
 config.enable_rtti              = "@LIBCXX_ENABLE_RTTI@"
 config.enable_shared            = "@LIBCXX_ENABLE_SHARED@"

www/index.html

   <ul>
     <li><code>cd path/to/libcxx/libcxx</code></li>
     <li><code>alias lit='python path/to/llvm/utils/lit/lit.py'</code></li>
-    <li><code>ln -s path/to/build/dir/projects/libcxx/test/lit.site.cfg
-        test/lit.site.cfg</code></li>
+    <li><code>export
+        LIBCXX_SITE_CONFIG=path/to/build/dir/projects/libcxx/test/lit.site.cfg
+        </code></li>
     <li><code>lit -sv test/re/ # or whichever subset of tests you're interested
         in</code></li>
   </ul>