This is v2 of my series to add clang-format to the CI. The series was mostly inspired by a patch sent recently to 'include kh_foreach* macros in ForEachMacros' [1]. I was wondering why we don't run the formatting on CI and reduce some of the workload of reviewers. We have a '.clang-format' file to provide rules for code formatting. The commits 1-6 aims to add more rules to the file while deprecating old ones. Commit 7 enables CI action on GitHub and GitLab to also check for the code formatting. Currently, it is allowed to fail. This way we can build confidence and fine tune the values as needed and finally enforce the check in a future patch. I'm not well versed with GitHub workflows, and I mostly tried to copy existing work there. Expecting some feedback in that section! Commit 8 fixes an existing issue with the 'check-whitespace' job, which is failing as success in the GitLab CI. 1: https://lore.kernel.org/git/4e7893f5-2dd9-46cf-8a64-cf780f4e1730@xxxxxx/ Changes against v1: * Fixed a lot of typos. * Added more details about the warnings specified by clang-format for 'RemoveBracesLLVM' in commit 5. Added more details too. * The last two commits were modified to use "CI_MERGE_REQUEST_TARGET_BRANCH_SHA" and fallback to "CI_MERGE_REQUEST_DIFF_BASE_SHA" if the former is not available in GitLab's CI. * Use `!/bin/sh` for the run-style-check script. * Modified the last commit to remove block around the && check in the whitespace script. * Use `git rev-parse --verify --quiet` to verify the base commit. Karthik Nayak (8): clang-format: indent preprocessor directives after hash clang-format: avoid spacing around bitfield colon clang-format: ensure files end with newlines clang-format: replace deprecated option with 'SpacesInParens' clang-format: avoid braces on simple single-statement bodies clang-format: formalize some of the spacing rules ci: run style check on GitHub and GitLab check-whitespace: detect if no base_commit is provided .clang-format | 42 +++++++++++++++++++++++++++---- .github/workflows/check-style.yml | 29 +++++++++++++++++++++ .gitlab-ci.yml | 24 +++++++++++++++++- ci/check-whitespace.sh | 10 ++++++-- ci/install-dependencies.sh | 2 +- ci/run-style-check.sh | 8 ++++++ 6 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/check-style.yml create mode 100755 ci/run-style-check.sh Range-diff against v1: 1: 6cf91ffc86 = 1: 6cf91ffc86 clang-format: indent preprocessor directives after hash 2: beb002885f = 2: beb002885f clang-format: avoid spacing around bitfield colon 3: 3031be43e7 = 3: 3031be43e7 clang-format: ensure files end with newlines 4: bc1550e300 = 4: bc1550e300 clang-format: replace deprecated option with 'SpacesInParens' 5: cb6097aa22 ! 5: 840318a4c9 clang-format: avoid braces on simple single-statement bodies @@ Commit message Set the 'RemoveBracesLLVM' to 'true' which ensures that we avoid curly braces for single-statement bodies in conditional blocks. + This option does come with two warnings: + + This option will be renamed and expanded to support other styles. + + and + + Setting this option to true could lead to incorrect code formatting + due to clang-format’s lack of complete semantic information. As + such, extra care should be taken to review code changes made by + this option. + + The latter seems to be of concern. But since we only use clang-format to + verify the format and not to apply formatting, we should be okay here. + Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx> ## .clang-format ## @@ .clang-format: IndentWrappedFunctionNames: false PointerAlignment: Right +# Remove optional braces of control statements (if, else, for, and while) -+# according to the LLVM coding style -+# This avoids braces on simple single-statement bodies of statements. ++# according to the LLVM coding style. This avoids braces on simple ++# single-statement bodies of statements but keeps braces if one side of ++# if/else if/.../else cascade has multi-statement body. +RemoveBracesLLVM: true + # Don't insert a space after a cast 6: c4b6e1e82f ! 6: 0ecfd8d24b clang-format: formalize some of the spacing rules @@ Commit message clang-format: formalize some of the spacing rules There are some spacing rules that we follow in the project and it makes - sen to formalize them: + sense to formalize them: * Ensure there is no space inserted after the logical not '!' operator. - * Ensure there is no space before the case statement's color. + * Ensure there is no space before the case statement's colon. * Ensure there is no space before the first bracket '[' of an array. * Ensure there is no space in empty blocks. 7: 37f1e5c702 ! 7: 11a9ac4935 ci: run style check on GitHub and GitLab @@ Commit message this job to fail, so we can validate if this is useful and eventually enforce it. + For GitLab, we use the 'CI_MERGE_REQUEST_TARGET_BRANCH_SHA' variable by + default to obtain the base SHA of the merged pipeline (which is only + available for merged pipelines [1]). Otherwise we use the + 'CI_MERGE_REQUEST_DIFF_BASE_SHA' variable. + + [1]: https://docs.gitlab.com/ee/ci/variables/predefined_variables.html#predefined-variables-for-merge-request-pipelines + Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx> ## .github/workflows/check-style.yml (new) ## @@ .gitlab-ci.yml: check-whitespace: + before_script: + - ./ci/install-dependencies.sh + script: -+ - ./ci/run-style-check.sh "$CI_MERGE_REQUEST_DIFF_BASE_SHA" ++ - | ++ if [ -z ${CI_MERGE_REQUEST_TARGET_BRANCH_SHA} ]; then ++ ./ci/run-style-check.sh "$CI_MERGE_REQUEST_DIFF_BASE_SHA" ++ else ++ ./ci/run-style-check.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" ++ fi + rules: + - if: $CI_PIPELINE_SOURCE == 'merge_request_event' + @@ ci/install-dependencies.sh: ubuntu-*) ## ci/run-style-check.sh (new) ## @@ -+#!/usr/bin/env sh ++#!/bin/sh +# +# Perform style check +# 8: c6d07402af < -: ---------- check-whitespace: detect if no base_commit is provided -: ---------- > 8: caccbf8264 check-whitespace: detect if no base_commit is provided -- 2.45.1
