Merge pull request #409 from lukaszstolarczuk/add-black

bratpiorka · web-flow · commit 05fef9129304 · 2024-04-09T09:48:58.000+02:00
Add black formatter for Python
diff --git a/.github/workflows/pr_push.yml b/.github/workflows/pr_push.yml
@@ -94,7 +94,7 @@ jobs:
     - name: Install apt packages
       run: |
         sudo apt-get update
-        sudo apt-get install -y cmake clang-format-15 cmake-format libhwloc-dev
+        sudo apt-get install -y black cmake clang-format-15 cmake-format libhwloc-dev
 
     - name: Configure CMake
       run: >
@@ -106,12 +106,15 @@ jobs:
         -DUMF_BUILD_LIBUMF_POOL_SCALABLE=OFF
         -DUMF_BUILD_LIBUMF_POOL_JEMALLOC=OFF
 
-    - name: Check clang-format
+    - name: Check C/C++ formatting
       run: cmake --build build --target clang-format-check
 
-    - name: Check cmake-format
+    - name: Check CMake formatting
       run: cmake --build build --target cmake-format-check
 
+    - name: Check Python formatting
+      run: cmake --build build --target black-format-check
+
   DocsBuild:
     name: Build docs
     runs-on: ubuntu-latest
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -25,7 +25,9 @@ option(UMF_BUILD_EXAMPLES "Build UMF examples" ON)
 option(UMF_ENABLE_POOL_TRACKING "Build UMF with pool tracking" ON)
 option(UMF_DEVELOPER_MODE "Enable developer checks, treats warnings as errors"
        OFF)
-option(UMF_FORMAT_CODE_STYLE "Format UMF code with clang-format" OFF)
+option(UMF_FORMAT_CODE_STYLE
+       "Add clang, cmake, and black -format-check and -format-apply targets"
+       OFF)
 option(USE_ASAN "Enable AddressSanitizer checks" OFF)
 option(USE_UBSAN "Enable UndefinedBehaviorSanitizer checks" OFF)
 option(USE_TSAN "Enable ThreadSanitizer checks" OFF)
@@ -255,13 +257,19 @@ if(UMF_FORMAT_CODE_STYLE)
     find_program(CLANG_FORMAT NAMES clang-format-15 clang-format-15.0
                                     clang-format)
     find_program(CMAKE_FORMAT NAMES cmake-format)
+    find_program(BLACK NAMES black)
+
+    set(CLANG_FORMAT_REQUIRED "15.0")
+    set(CMAKE_FORMAT_REQUIRED "0.6")
 
-    if(NOT CLANG_FORMAT AND NOT CMAKE_FORMAT)
+    if(NOT CLANG_FORMAT
+       AND NOT CMAKE_FORMAT
+       AND NOT BLACK)
         message(
             FATAL_ERROR
                 "UMF_FORMAT_CODE_STYLE=ON, but neither clang-format (required version: "
-                "${CLANG_FORMAT_REQUIRED}) nor cmake-format (required version: "
-                "${CMAKE_FORMAT_VERSION}) was found.")
+                "${CLANG_FORMAT_REQUIRED}), nor cmake-format (required version: "
+                "${CMAKE_FORMAT_REQUIRED}), nor black was found.")
     endif()
 
     if(CLANG_FORMAT)
@@ -271,7 +279,6 @@ if(UMF_FORMAT_CODE_STYLE)
 
         # Check if clang-format (in correct version) is available for code
         # formatting.
-        set(CLANG_FORMAT_REQUIRED "15.0")
         if(NOT (CLANG_FORMAT_VERSION VERSION_EQUAL CLANG_FORMAT_REQUIRED))
             message(FATAL_ERROR "Required clang-format version is "
                                 "${CLANG_FORMAT_REQUIRED}")
@@ -301,8 +308,8 @@ if(UMF_FORMAT_CODE_STYLE)
         file(GLOB_RECURSE format_list ${format_clang_glob})
 
         message(
-            STATUS "Adding 'clang-format-check' and 'clang-format-apply' make "
-                   "targets")
+            STATUS
+                "Adding 'clang-format-check' and 'clang-format-apply' targets")
 
         add_custom_target(
             clang-format-check
@@ -312,7 +319,7 @@ if(UMF_FORMAT_CODE_STYLE)
 
         add_custom_target(
             clang-format-apply
-            COMMAND ${CLANG_FORMAT} --style=file --i ${format_list}
+            COMMAND ${CLANG_FORMAT} --style=file -i ${format_list}
             COMMENT "Format files using clang-format")
     endif()
 
@@ -323,7 +330,6 @@ if(UMF_FORMAT_CODE_STYLE)
 
         # Check if cmake-format (in correct version) is available for cmake
         # files formatting.
-        set(CMAKE_FORMAT_REQUIRED "0.6")
         if(NOT (CMAKE_FORMAT_VERSION VERSION_EQUAL CMAKE_FORMAT_REQUIRED))
             message(FATAL_ERROR "Required cmake-format version is"
                                 "${CMAKE_FORMAT_REQUIRED}")
@@ -351,8 +357,8 @@ if(UMF_FORMAT_CODE_STYLE)
         list(APPEND format_cmake_list "${PROJECT_SOURCE_DIR}/CMakeLists.txt")
 
         message(
-            STATUS "Adding 'cmake-format-check' and 'cmake-format-apply' make "
-                   "targets")
+            STATUS
+                "Adding 'cmake-format-check' and 'cmake-format-apply' targets")
 
         add_custom_target(
             cmake-format-check
@@ -365,28 +371,59 @@ if(UMF_FORMAT_CODE_STYLE)
             COMMENT "Format Cmake files using cmake-format")
     endif()
 
-    # Add a convenience target for running both tools at once - available only
-    # if both are found.
-    if(CLANG_FORMAT AND CMAKE_FORMAT)
+    if(BLACK)
+        # black should maintain backward compatibility, we don't have to require
+        # a specific version
+        get_program_version_major_minor(${BLACK} BLACK_VERSION)
+        message(STATUS "Found black: ${BLACK} (version: ${BLACK_VERSION})")
+
+        message(
+            STATUS
+                "Adding 'black-format-check' and 'black-format-apply' targets")
+
+        add_custom_target(
+            black-format-check
+            COMMAND ${BLACK} --check --verbose ${CMAKE_SOURCE_DIR}
+            COMMENT "Check Python files formatting using black formatter")
+
+        add_custom_target(
+            black-format-apply
+            COMMAND ${BLACK} ${CMAKE_SOURCE_DIR}
+            COMMENT "Format Python files using black formatter")
+    endif()
+
+    # Add a convenience target for running all tools at once - available only if
+    # all are found.
+    if(CLANG_FORMAT
+       AND CMAKE_FORMAT
+       AND BLACK)
         add_custom_target(
             format-check
             COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target
                     clang-format-check
             COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target
                     cmake-format-check
-            COMMENT "Running both clang-format-check and cmake-format-check")
+            COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target
+                    black-format-check
+            COMMENT "Running all formatting checks")
 
         add_custom_target(
             format-apply
             COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target
                     clang-format-apply
             COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target
                     cmake-format-apply
-            COMMENT "Format files using clang-format and cmake-format")
+            COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target
+                    black-format-apply
+            COMMENT "Format C/C++, CMake, and Python files")
+        message(
+            STATUS
+                "  Adding convenience targets 'format-check' and 'format-apply'."
+        )
     else()
         message(
             STATUS
-                "  Convenience targets 'make format-check' and 'make format-apply' are "
+                "  Convenience targets 'format-check' and 'format-apply' are "
                 "not available. Use commands specific for found tools (see the log above)."
         )
     endif()
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -62,10 +62,11 @@ To enable additional checks (including `-Werror` / `/WX` compilation flag), swit
 ["CMake standard options"](./README.md#cmake-standard-options) section in the top-level Readme.
 
 ### Code style
-We use `clang-format` to verify and apply code style changes to source files. 
+We use `clang-format` to verify and apply code style changes to C/C++ source files.
 To see all rules we require, please take a look at `.clang-format` file in the 
 root directory of this repository. Similarly, we use `cmake-format` tool and
 `.cmake-format` file to verify and apply code style changes to CMake files.
+For Python source files we use `black` tool.
 
 To enable code style checks and re-formatting, CMake option `UMF_FORMAT_CODE_STYLE` has to
 be switched on. You'll then have additional CMake targets available.
@@ -74,7 +75,7 @@ To verify correct coding style of your changes execute (assuming `build` is your
 
 ```bash
 $ cmake -B build -DUMF_FORMAT_CODE_STYLE=ON
-$ cmake --build build --target format-checks
+$ cmake --build build --target format-check
 ```
 
 We run these checks in our Continuous Integration (CI). So, if any issues were found,
@@ -87,9 +88,10 @@ $ cmake --build build --target format-apply
 # Remember to review introduced changes
 ```
 
-If you wish to use only `clang-format` or only `cmake-format`, you can execute the corresponding
-`clang-format-check` and `clang-format-apply` for source files, or `cmake-format-check` and
-`cmake-format-apply` for CMake files, respectively.
+If you wish to use only `clang-format`, or `cmake-format`, or `black`, you can execute the corresponding
+`clang-format-check` and `clang-format-apply` for C/C++ source files, or `cmake-format-check` and
+`cmake-format-apply` for CMake files, or `black-format-check` and `black-format-apply` for Python
+source files, respectively.
 
 **NOTE**: We use specific versions of formatting tools to ensure consistency across the project. The required versions are:
 - clang-format version **15.0**, which can be installed with the command: `python -m pip install clang-format==15.0.7`.
diff --git a/README.md b/README.md
@@ -102,7 +102,7 @@ List of options provided by CMake:
 | UMF_BUILD_EXAMPLES | Build UMF examples | ON/OFF | ON |
 | UMF_ENABLE_POOL_TRACKING | Build UMF with pool tracking | ON/OFF | ON |
 | UMF_DEVELOPER_MODE | Treat warnings as errors and enables additional checks | ON/OFF | OFF |
-| UMF_FORMAT_CODE_STYLE | Add clang and cmake -format-check and -format-apply targets to make | ON/OFF | OFF |
+| UMF_FORMAT_CODE_STYLE | Add clang, cmake, and black -format-check and -format-apply targets to make | ON/OFF | OFF |
 | USE_ASAN | Enable AddressSanitizer checks | ON/OFF | OFF |
 | USE_UBSAN | Enable UndefinedBehaviorSanitizer checks | ON/OFF | OFF |
 | USE_TSAN | Enable ThreadSanitizer checks | ON/OFF | OFF |
diff --git a/scripts/generate_docs.py b/scripts/generate_docs.py
@@ -7,7 +7,7 @@
 
 from pathlib import Path
 from shutil import rmtree
-import subprocess   # nosec B404
+import subprocess  # nosec B404
 import time
 
 
@@ -41,8 +41,9 @@ def _prepare_docs_dir(docs_path: Path) -> None:
 def _generate_xml(config_path: Path, docs_path: Path) -> None:
     print("Generating XML files with doxygen...", flush=True)
     try:
-        subprocess.run(["doxygen", Path(config_path, "Doxyfile")], text=True
-                       ).check_returncode()    # nosec B603, B607
+        subprocess.run(
+            ["doxygen", Path(config_path, "Doxyfile")], text=True
+        ).check_returncode()  # nosec B603, B607
         print(f"All XML files generated in {docs_path}", flush=True)
     except subprocess.CalledProcessError as ex:
         print("Generating XML files failed!")
@@ -53,8 +54,9 @@ def _generate_xml(config_path: Path, docs_path: Path) -> None:
 def _generate_html(config_path: Path, docs_path: Path) -> None:
     print("Generating HTML pages with sphinx...", flush=True)
     try:
-        subprocess.run(["sphinx-build", config_path, Path(docs_path, "html")], text=True
-                       ).check_returncode()    # nosec B603, B607
+        subprocess.run(
+            ["sphinx-build", config_path, Path(docs_path, "html")], text=True
+        ).check_returncode()  # nosec B603, B607
         print(f"All HTML files generated in {docs_path}", flush=True)
     except subprocess.CalledProcessError as ex:
         print("Generating HTML pages failed!")
diff --git a/test/test_installation.py b/test/test_installation.py
@@ -9,7 +9,7 @@
 import difflib
 from pathlib import Path
 import platform
-import subprocess   # nosec B404
+import subprocess  # nosec B404
 import sys
 from typing import List
 
@@ -61,7 +61,7 @@ def _create_match_list(self) -> List[str]:
             lib_ext_static = "a"
             lib_ext_shared = "so"
             lib_prefix = "lib"
-        else: # MacOS
+        else:  # MacOS
             lib_ext_static = "a"
             lib_ext_shared = "dylib"
             lib_prefix = "lib"
@@ -138,7 +138,7 @@ def install_umf(self) -> None:
 
         install_cmd = f"cmake --install {self.build_dir} --config {self.build_type.title()} --prefix {self.install_dir}"
         try:
-            subprocess.run(install_cmd.split()).check_returncode()      # nosec B603
+            subprocess.run(install_cmd.split()).check_returncode()  # nosec B603
         except subprocess.CalledProcessError:
             sys.exit(f"Error: UMF installation command '{install_cmd}' failed")
 
@@ -175,7 +175,7 @@ def uninstall_umf(self) -> None:
         """
         uninstall_cmd = f"cmake --build {self.build_dir} --target uninstall"
         try:
-            subprocess.run(uninstall_cmd.split()).check_returncode()    # nosec B603
+            subprocess.run(uninstall_cmd.split()).check_returncode()  # nosec B603
         except subprocess.CalledProcessError:
             sys.exit(f"Error: UMF uninstallation command '{uninstall_cmd}' failed")
 
diff --git a/third_party/requirements.txt b/third_party/requirements.txt
@@ -2,6 +2,7 @@
 # Formatting the source code
 clang-format==15.0.7
 cmake-format==0.6.13
+black==23.7.0
 # Generating HTML documentation
 pygments==2.15.1
 sphinxcontrib_applehelp==1.0.4
