[libcxx-commits] [libcxx] 1f8e286 - [libc++] Add a CMake target to re-generate files and revamp CONTRIBUTING.rst

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jul 15 09:07:36 PDT 2021


Author: Louis Dionne
Date: 2021-07-15T12:07:26-04:00
New Revision: 1f8e286cdc14488c80eeb4a92ff791510d19a0d3

URL: https://github.com/llvm/llvm-project/commit/1f8e286cdc14488c80eeb4a92ff791510d19a0d3
DIFF: https://github.com/llvm/llvm-project/commit/1f8e286cdc14488c80eeb4a92ff791510d19a0d3.diff

LOG: [libc++] Add a CMake target to re-generate files and revamp CONTRIBUTING.rst

As we automate more and more things in the library, it becomes useful for
contributors to have a single target for running all the automation as
part of their workflow. This commit adds a new `libcxx-generate-files`
target that should re-generate all the auto-generated files in the library.

As a fly-by, I also revamped the documentation on Contributing to account
for this new target and present it as a bullet list of things to check
before committing. I also added a few things that are often overlooked
to that list, such as updating the synopsis and the status files.

Differential Revision: https://reviews.llvm.org/D106067

Added: 
    libcxx/utils/CMakeLists.txt

Modified: 
    libcxx/CMakeLists.txt
    libcxx/docs/Contributing.rst
    libcxx/utils/ci/run-buildbot

Removed: 
    


################################################################################
diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 654bed5de88ce..10caf0f8e3025 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -930,6 +930,7 @@ endfunction()
 #===============================================================================
 add_subdirectory(include)
 add_subdirectory(src)
+add_subdirectory(utils)
 
 set(LIBCXX_TEST_DEPS "")
 

diff  --git a/libcxx/docs/Contributing.rst b/libcxx/docs/Contributing.rst
index 6bad6a656115c..0cf752f7cc907 100644
--- a/libcxx/docs/Contributing.rst
+++ b/libcxx/docs/Contributing.rst
@@ -4,15 +4,13 @@
 Contributing to libc++
 ======================
 
-.. contents::
-  :local:
+This file contains notes about various tasks and processes specific to contributing
+to libc++. If this is your first time contributing, please also read `this document
+<https://www.llvm.org/docs/Contributing.html>`__ on general rules for contributing to LLVM.
 
-Please read `this document <https://www.llvm.org/docs/Contributing.html>`__ on general rules to contribute to LLVM projects.
-
-Tasks and processes
-===================
-
-This file contains notes about various tasks and processes specific to libc++.
+For libc++, please make sure you follow `these instructions <https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line>`_
+for submitting a code review from the command-line using ``arc``, since we have some
+automation (e.g. CI) that depends on the review being submitted that way.
 
 Looking for pre-existing reviews
 ================================
@@ -24,36 +22,33 @@ and clicking on ``Libc++ Open Reviews`` in the sidebar to the left. If you see
 that your feature is already being worked on, please consider chiming in instead
 of duplicating work!
 
-Post-Release TODO
-=================
+Pre-commit check list
+=====================
 
-After branching for an LLVM release:
+Before committing or creating a review, please go through this check-list to make
+sure you don't forget anything:
 
-1. Update ``_LIBCPP_VERSION`` in ``include/__config``
-2. Update the ``include/__libcpp_version`` file
-3. Update the version number in ``docs/conf.py``
+- Do you have tests for every public class and/or function you're adding or modifying?
+- Did you update the synopsis of the relevant headers?
+- Did you update the relevant files to track implementation status (in ``docs/Status/``)?
+- If you added a header:
 
-Modifying feature-test macros
-=============================
+  - Did you add it to ``include/module.modulemap``?
+  - Did you add it to ``include/CMakeLists.txt``?
+  - If it's a public header, did you add a test under ``test/libcxx`` that the new header defines ``_LIBCPP_VERSION``? See ``test/libcxx/algorithms/version.pass.cpp`` for an example. NOTE: This should be automated.
+  - If it's a public header, did you update ``utils/generate_header_inclusion_tests.py``?
 
-When adding or updating feature-test macros, you should update the corresponding tests.
-To do that, modify ``feature_test_macros`` table in the script
-``utils/generate_feature_test_macro_components.py``, run it, and commit updated
-files. Running ``utils/generate_feature_test_macro_components.py`` should never
-generate 
diff s in a clean checkout; feel free to run it in your local checkout
-any time you want.
+- Did you add the relevant feature test macro(s) for your feature? Did you update the ``generate_feature_test_macro_components.py`` script with it?
+- Did you run the ``libcxx-generate-files`` target and verify its output?
 
+Post-release check list
+=======================
 
-Adding a new header TODO
-========================
-
-When adding a new header to libc++:
+After branching for an LLVM release:
 
-1. Add a test under ``test/libcxx`` that the new header defines ``_LIBCPP_VERSION``. See ``test/libcxx/algorithms/version.pass.cpp`` for an example.
-2. Run ``python utils/generate_header_tests.py``; verify and commit the changes.
-3. Modify ``python utils/generate_header_inclusion_tests.py``; run it; verify and commit the changes.
-4. Create a submodule in ``include/module.modulemap`` for the new header.
-5. Update the ``include/CMakeLists.txt`` file to include the new header.
+1. Update ``_LIBCPP_VERSION`` in ``include/__config``
+2. Update the ``include/__libcpp_version`` file
+3. Update the version number in ``docs/conf.py``
 
 Exporting new symbols from the library
 ======================================

diff  --git a/libcxx/utils/CMakeLists.txt b/libcxx/utils/CMakeLists.txt
new file mode 100644
index 0000000000000..1a1fbe8dfa966
--- /dev/null
+++ b/libcxx/utils/CMakeLists.txt
@@ -0,0 +1,18 @@
+
+add_custom_target(libcxx-generate-public-header-transitive-inclusion-tests
+    COMMAND "${Python3_EXECUTABLE}" "${LIBCXX_SOURCE_DIR}/utils/generate_header_inclusion_tests.py"
+    COMMENT "Generate tests checking for mandated transitive includes in public headers.")
+
+add_custom_target(libcxx-generate-public-header-tests
+    COMMAND "${Python3_EXECUTABLE}" "${LIBCXX_SOURCE_DIR}/utils/generate_header_tests.py"
+    COMMENT "Generate tests for including public headers.")
+
+add_custom_target(libcxx-generate-feature-test-macros
+    COMMAND "${Python3_EXECUTABLE}" "${LIBCXX_SOURCE_DIR}/utils/generate_feature_test_macro_components.py"
+    COMMENT "Generate the <version> header and tests for feature test macros.")
+
+add_custom_target(libcxx-generate-files
+    DEPENDS libcxx-generate-public-header-transitive-inclusion-tests
+            libcxx-generate-public-header-tests
+            libcxx-generate-feature-test-macros
+    COMMENT "Create all the auto-generated files in libc++ and its tests.")

diff  --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index 15488e942527c..da1a53e07fff2 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -164,19 +164,19 @@ check-generated-output)
     # `! foo` doesn't work properly with `set -e`, use `! foo || false` instead.
     # https://stackoverflow.com/questions/57681955/set-e-does-not-respect-logical-not
     clean
-    echo "+++ Checking the output of the generator scripts"
-    mkdir -p ${BUILD_DIR}
-    # Reject patches that don't update the generated output correctly.
-    python3 libcxx/utils/generate_feature_test_macro_components.py
-    python3 libcxx/utils/generate_header_inclusion_tests.py
-    python3 libcxx/utils/generate_header_tests.py
+    generate-cmake
+
+    # Reject patches that forgot to re-run the generator scripts.
+    echo "+++ Making sure the generator scripts were run"
+    ${NINJA} -vC "${BUILD_DIR}" libcxx-generate-files
     git 
diff  | tee ${BUILD_DIR}/generated_output.patch
-    # Check if the 
diff s are empty, fail otherwise.
     ! grep -q '^--- a' ${BUILD_DIR}/generated_output.patch || false
+
     # Reject patches that introduce non-ASCII characters or hard tabs.
     # Depends on LC_COLLATE set at the top of this script.
     ! grep -rn '[^ -~]' libcxx/include/ || false
-    # Check that no dependency cycles have been introduced.
+
+    # Reject patches that introduce dependency cycles in the headers.
     python3 libcxx/utils/graph_header_deps.py >/dev/null
 ;;
 generic-cxx03)


        


More information about the libcxx-commits mailing list