[libcxx] Capture configuration information when installing the libc++ headers

[EricWF]

Summary

Hi all,

This patch is a successor to D11963. However it has changed dramatically and I felt it would be best to start a new review thread.

Please read the design documentation added in this patch for a description of how it works.

[EricWF]

Oh, but don't review the design doc too closely. I'm still working on it.

[EricWF]

Adding subscribes from the old patch to this one.

[EricWF]

I removed all non-essential changes in order to make this easy and fast to review. This patch is blocking a number of people so I would like to have this reviewed soon.

[jroelofs]

Design doc looks good! I think it captures all the design constraints well, and the proposed solution satisfies those constraints as far as I am concerned.

Thumbs up from me... but I'll still defer the final 'LGTM' to @mclow.lists.

[eugenis]

Looks great!

include/CMakeLists.txt

31

I don't think >> would work on windows.
Do you really need __generated_config to be created at build time (as opposed to configure time)? You could use file(read) and file(append) then.

[EricWF]

include/CMakeLists.txt

31

I would strongly prefer the file got generated at build time so that it contains any changes made in the source tree. Any other behavior is developer hostile and non-obvious. In order to keep the installed headers consistent we need to do this. Although I hope there is a better way to achieve this.

[eugenis]

include/CMakeLists.txt

31

Right, good point. Then you could go back to the approach in D11963 where you called cmake in a custom command with a small script that used file(*) commands.

[EricWF]

include/CMakeLists.txt

31

That approach had a problem because the cmake INSTALL command used to invoke the script doesn't take a "COMPONENT" argument which would break the "install-libcxx" rule. On windows we could do it with a call out to python or a shell script. Would that work?

[rnk]

include/CMakeLists.txt

31

The >> operator should work on Windows. It's supported by cmd. However, cat generally isn't available. If you use 'type' in place of 'cat' it should work.

[EricWF]

Use type instead of cat on windows as suggested by @rnk. @eugenis does this address your concern?

[eugenis]

In D13407#261228, @EricWF wrote:

Use type instead of cat on windows as suggested by @rnk. @eugenis does this address your concern?

Absolutely!
LGTM

[EricWF]

Cool. I've reached out to some platform maintainers to make sure they don't see any problems. I'll commit this once I hear back.

[vkalintiris]

include/CMakeLists.txt

9

Kind of silly but I believe that the files used for auto-generated config headers in LLVM have the extesions .cmake and .in, for cmake and autoconf, respectively.

[EricWF]

@eugenis I'll commit this tuesday morning If I don't hear anything over the weekend (Monday is a holiday for me).
If you rebase the ABI patch to work with this I'll make sure to review it ASAP.

include/CMakeLists.txt

9

libc++ uses .cmake for CMake scripts and .in for auto-generated files and I think this is fine. libc++ doesn't support autoconf I'm not worried about it causing confusion.

[eugenis]

Thanks. I'll upload the rebase patch on Monday.

include/CMakeLists.txt

38

I think you need to depend on config_site.in as well. Or on ${LIBCXX_BINARY_DIR}/config_site

[EricWF]

include/CMakeLists.txt

38

I don't think we need it. __config_site is generated with configure_file, which runs at configuration time and re-runs whenever the input file is changed. So __config_site should always be up to date.

However I'll see if we can actually make the change. It seems clearer and safer.

[EricWF]

Make the generated config dependent on the "__config_site" file. It probably didn't need this but it doesn't hurt to have.

[EricWF]

Maintainers from Apple and FreeBSD gave this the thumbs up.

CMakeLists.txt

 
 # Feature flags ===============================================================
 define_if(MSVC -D_CRT_SECURE_NO_WARNINGS)
-define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE -D_LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE)
-define_if_not(LIBCXX_ENABLE_STDIN -D_LIBCPP_HAS_NO_STDIN)
-define_if_not(LIBCXX_ENABLE_STDOUT -D_LIBCPP_HAS_NO_STDOUT)
-define_if_not(LIBCXX_ENABLE_THREADS -D_LIBCPP_HAS_NO_THREADS)
-define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK -D_LIBCPP_HAS_NO_MONOTONIC_CLOCK)
-define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS -D_LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
-
 
 # Sanitizer flags =============================================================
 

     message(WARNING "LLVM_USE_SANITIZER is not supported on this platform.")
   endif()
 endif()
+
+# Configuration file flags =====================================================
+config_define_if_not(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE)
+config_define_if_not(LIBCXX_ENABLE_STDIN _LIBCPP_HAS_NO_STDIN)
+config_define_if_not(LIBCXX_ENABLE_STDOUT _LIBCPP_HAS_NO_STDOUT)
+config_define_if_not(LIBCXX_ENABLE_THREADS _LIBCPP_HAS_NO_THREADS)
+config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK _LIBCPP_HAS_NO_MONOTONIC_CLOCK)
+config_define_if_not(LIBCXX_ENABLE_THREAD_UNSAFE_C_FUNCTIONS _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS)
+
+if (LIBCXX_NEEDS_SITE_CONFIG)
+  configure_file(
+    include/__config_site.in
+    ${LIBCXX_BINARY_DIR}/__config_site
+    @ONLY)
+endif()
+
 #===============================================================================
 # Setup Source Code And Tests
 #===============================================================================

cmake/Modules/HandleLibcxxFlags.cmake

   endif()
 endmacro()
 
+macro(config_define_if condition def)
+  if (${condition})
+    set(${def} ON)
+    add_definitions(-D${def})
+    set(LIBCXX_NEEDS_SITE_CONFIG ON)
+  endif()
+endmacro()
+
+macro(config_define_if_not condition def)
+  if (NOT ${condition})
+    set(${def} ON)
+    add_definitions(-D${def})
+    set(LIBCXX_NEEDS_SITE_CONFIG ON)
+  endif()
+endmacro()
+
 # Add a specified list of flags to both 'LIBCXX_COMPILE_FLAGS' and
 # 'LIBCXX_LINK_FLAGS'.
 macro(add_flags)

docs/DesignDocs/CapturingConfigInfo.rst

+=======================================================
+Capturing configuration information during installation
+=======================================================
+
+.. contents::
+   :local:
+
+The Problem
+===========
+
+Currently the libc++ supports building the library with a number of different
+configuration options.  Unfortunately all of that configuration information is
+lost when libc++ is installed. In order to support "persistent"
+configurations libc++ needs a mechanism to capture the configuration options
+in the INSTALLED headers.
+
+
+Design Goals
+============
+
+* The solution should not INSTALL any additional headers. We don't want an extra
+  #include slowing everybody down.
+
+* The solution should not unduly affect libc++ developers. The problem is limited
+  to installed versions of libc++ and the solution should be as well.
+
+* The solution should not modify any existing headers EXCEPT during installation.
+  It makes developers lives harder if they have to regenerate the libc++ headers
+  every time they are modified.
+
+* The solution should not make any of the libc++ headers dependant on
+  files generated by the build system. The headers should be able to compile
+  out of the box without any modification.
+
+* The solution should not have ANY effect on users who don't need special
+  configuration options. The vast majority of users will never need this so it
+  shouldn't cost them.
+
+
+The Solution
+============
+
+When you first configure libc++ using CMake we check to see if we need to
+capture any options. If we haven't been given any "persistent" options then
+we do NOTHING.
+
+Otherwise we create a custom installation rule that modifies the installed __config
+header. The rule first generates a dummy "__config_site" header containing the required
+#defines. The contents of the dummy header are then prependend to the installed
+__config header. By manually prepending the files we avoid the cost of an
+extra #include and we allow the __config header to be ignorant of the extra
+ configuration all together. An example "__config" header generated when
+-DLIBCXX_ENABLE_THREADS=OFF is given to CMake would look something like:
+
+.. code-block:: cpp
+
+  //===----------------------------------------------------------------------===//
+  //
+  //                     The LLVM Compiler Infrastructure
+  //
+  // This file is dual licensed under the MIT and the University of Illinois Open
+  // Source Licenses. See LICENSE.TXT for details.
+  //
+  //===----------------------------------------------------------------------===//
+
+  #ifndef _LIBCPP_CONFIG_SITE
+  #define _LIBCPP_CONFIG_SITE
+
+  /* #undef _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE */
+  /* #undef _LIBCPP_HAS_NO_STDIN */
+  /* #undef _LIBCPP_HAS_NO_STDOUT */
+  #define _LIBCPP_HAS_NO_THREADS
+  /* #undef _LIBCPP_HAS_NO_MONOTONIC_CLOCK */
+  /* #undef _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS */
+
+  #endif
+  // -*- C++ -*-
+  //===--------------------------- __config ---------------------------------===//
+  //
+  //                     The LLVM Compiler Infrastructure
+  //
+  // This file is dual licensed under the MIT and the University of Illinois Open
+  // Source Licenses. See LICENSE.TXT for details.
+  //
+  //===----------------------------------------------------------------------===//
+
+  #ifndef _LIBCPP_CONFIG
+  #define _LIBCPP_CONFIG

docs/index.rst

 Design Documents
 ----------------
 
+.. toctree::
+   :maxdepth: 1
+
+   DesignDocs/CapturingConfigInfo
+
+
 * `<atomic> design <http://libcxx.llvm.org/atomic_design.html>`_
 * `<type_traits> design <http://libcxx.llvm.org/type_traits_design.html>`_
 * `Status of debug mode <http://libcxx.llvm.org/debug_mode.html>`_

include/CMakeLists.txt

 if (NOT LIBCXX_INSTALL_SUPPORT_HEADERS)
   set(LIBCXX_SUPPORT_HEADER_PATTERN PATTERN "support" EXCLUDE)
 endif()
+
 set(LIBCXX_HEADER_PATTERN
   PATTERN "*"
   PATTERN "CMakeLists.txt" EXCLUDE
   PATTERN ".svn" EXCLUDE
+  PATTERN "__config_site.in" EXCLUDE
   ${LIBCXX_SUPPORT_HEADER_PATTERN}
   )
 

     ${LIBCXX_HEADER_PATTERN}
     PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
   )
+
+  if (LIBCXX_NEEDS_SITE_CONFIG)
+    set(UNIX_CAT cat)
+    if (WIN32)
+      set(UNIX_CAT type)
+    endif()
+    # Generate and install a custom __config header. The new header is created
+    # by  prepending __config_site to the current __config header.
+    add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config
+      COMMAND ${CMAKE_COMMAND} -E copy ${LIBCXX_BINARY_DIR}/__config_site ${LIBCXX_BINARY_DIR}/__generated_config
+      COMMAND ${UNIX_CAT} ${LIBCXX_SOURCE_DIR}/include/__config >> ${LIBCXX_BINARY_DIR}/__generated_config
+      DEPENDS ${LIBCXX_SOURCE_DIR}/include/__config
+    )
+    # Add a target that executes the generation commands.
+    add_custom_target(generate_config_header ALL
+      DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config)
+    # Install the generated header as __config.
+    install(FILES ${LIBCXX_BINARY_DIR}/__generated_config
+      DESTINATION include/c++/v1
+      PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
+      RENAME __config
+      COMPONENT libcxx)
+  endif()
+
 endif()

include/__config_site.in

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP_CONFIG_SITE
+#define _LIBCPP_CONFIG_SITE
+
+#cmakedefine _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE
+#cmakedefine _LIBCPP_HAS_NO_STDIN
+#cmakedefine _LIBCPP_HAS_NO_STDOUT
+#cmakedefine _LIBCPP_HAS_NO_THREADS
+#cmakedefine _LIBCPP_HAS_NO_MONOTONIC_CLOCK
+#cmakedefine _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS
+
+#endif