From 63d7743de08a3fc359d4945e1c7006a6de80af20 Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Mon, 6 Jan 2025 17:50:30 +0000 Subject: [PATCH 01/10] DAOS-16923 cq: make clang-format required Make clang-format GHA required on PRs. Skip-build: true Signed-off-by: Dalton Bohning --- .github/workflows/linting.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index 2aec15d841c..9f608e635e6 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -228,7 +228,7 @@ jobs: - doxygen - pylint - codespell - # - clang-format # not required + - clang-format - yaml-lint - copyright if: (!cancelled()) From 1e6a7cdaa6a68d9e7a46e8c23aecc5cd0da44311 Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Mon, 6 Jan 2025 19:00:15 +0000 Subject: [PATCH 02/10] update githook README Skip-build: true Signed-off-by: Dalton Bohning --- utils/cq/requirements.txt | 1 + utils/githooks/README.md | 89 +++++++++++++++++++++------------------ 2 files changed, 48 insertions(+), 42 deletions(-) diff --git a/utils/cq/requirements.txt b/utils/cq/requirements.txt index 20f5c5ba003..d5c264218d7 100644 --- a/utils/cq/requirements.txt +++ b/utils/cq/requirements.txt @@ -10,3 +10,4 @@ codespell==2.3.0 # Used by ci/jira_query.py which pip installs it standalone. jira torch>=2.2.0 +scons diff --git a/utils/githooks/README.md b/utils/githooks/README.md index 55ccb56eec6..577350c1303 100644 --- a/utils/githooks/README.md +++ b/utils/githooks/README.md @@ -8,68 +8,79 @@ The DAOS repo contains several built-in githooks that are intended to help developers conform to DAOS community coding standards and practices and to avoid some common mistakes in the development cycle. -To enable these standard githooks requires a two step process: +## Install DAOS Git Hooks +Installing is a two-step process: -1. Install the hooks - -Configure your core.hookspath as follows (Recommended): +### 1. Install the hooks +Recommended: Configure your `core.hookspath`. +Any new githooks added to the repository will automatically run, +but possibly require additional software to produce the desired effect. +Additionally, as the branch changes, the githooks change with it. ```sh git config core.hookspath utils/githooks ``` Additionally, one can copy the files into an already configured path. -With the first option, any new githooks added to the repository will -automatically run, but possibly require additional software to produce the -desired effect. Additionally, as the branch changes, the githooks -change with it. - -2. Install all of the required tools +### 2. Install the required tools The Githooks framework in DAOS is such that the hooks will all run. However, some hooks will simply check for required software and are effectively a noop if such is not installed. -On many systems, the required packages can be installed through standard means -but customization may be required. Some are specified in -[requirements.txt](../../requirements.txt) so can be installed using -`pip install -r requirements.txt` and `pip install -r utils/cq/requirements.txt` -which can also be done using a virtual environment. The following -packages are used by built-in githooks. +Requirements come from a combination of `pip` and system packages and can usually be installed through standard means. +To install `pip` packages specified in [utils/cq/requirements.txt](../../utils/cq/requirements.txt): +```sh +python3 -m pip install -r utils/cq/requirements.txt +``` +To install system packages with your package manager - for example: +```sh +dnf install git-clang-format +``` +### Installed tools +The following packages are used by built-in githooks. -1. clang-format version 14.0.5 or higher. If the check is unable to parse -the version output, it will fail. Try running +1. `clang-format` version 14.0.5 or higher - Formatter for C code. If the check is unable to parse +the version output, it will fail. Try running `/site_scons/site_tools/extra/extra.py` to check. -2. pylint -3. flake8 -4. yamllint -5. gofmt - -There is a daos wrapper around pylint at `utils/cq/daos_pylint.py` which will perform standard -pylint checks whilst managing scons and PYTHONPATH setup changes automatically. Installing -the python packages in `utils/cq/requirements.txt` will allow it to test for all the python dependencies. +2. `pylint` +3. `flake8` +4. `isort` +5. `yamllint` +6. `gofmt` +7. `codespell` +8. `scons` -It is important to check the output on commit for any errors that may indicate -any one of the required tools is missing. +### Optional tools Additionally, [find_base.sh](find_base.sh) attempts to determine the base branch using `gh`, the Github CLI. If this isn't installed, it will use -`master` as the base which can result in a larger diff and more files being +`master` or a `release` branch as the base, which can result in a larger diff and more files being checked than expected. ## Checks performed by built-in scripts ### pre-commit -1. clang-format will update any C/C++ files changed using configuration in -[.clang-format](../../.clang-format). If anything changed, it will exit, +It is important to check the output on commit for any errors that may indicate +any one of the required tools is missing. + +1. copyright - Custom tool that automatically updates copyrights in modified files. +2. `codespell` - Linter for spelling mistakes in modified files + - See [codespell.ignores](../../ci/codespell.ignores) for ignored words. + - See [words.dict](../../utils/cq/words.dict) for words added to the DAOS project. +3. Jenkinsfile - Custom linter if the Jenkinsfile is modified +4. `yamllint` - Linter for modified YAML configs +5. `clang-format` - Automatically formats for C/C++ files modified. If anything changed it will exit, allowing the user to inspect the changes and retry the commit. -2. pylint will check python changes and fail if there are errors. -3. flake8 will check python changes and fail if there are errors. -4. yamllint will check YAML file changes and fail if there are errors. -5. gofmt will check Go files changed and fail if there are errors. -6. Copyrights will be checked and updated if needed. +6. `gofmt` - Automatically formats for modified GO files + - See [.clang-format](../../.clang-format) for configuration +7. `isort` - Linter for python imports on modified python files +8. `flake8` - Linter for python files +9. `pylint` - Additional linter for modified python files + - See [daos_pylint.py](../../utils/cq/daos_pylint.py) for a custom wrapper around `pylint` which manages `PYTHONPATH` setup internally. +10. ftest - Custom linter for modified ftest files ### prepare-commit-msg @@ -79,12 +90,6 @@ the comment can be ignored. If not, it's likely someone else changed it and an update is needed before commit. In such a case, abort the commit by exiting without saving. -### commit-msg - -1. Checks to see if githooks are installed locally and adds a watermark -to the commit message that is checked in CI. It is not fatal but -gatekeepers may ask that githooks be used. - ## Adding user specific hooks The framework is extensible. In order to add a custom user hook, a developer From 16536e504d435d5db71f65a8e5b0421b2623860e Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Mon, 6 Jan 2025 19:09:01 +0000 Subject: [PATCH 03/10] backout scons from requirements.txt Since it may conflict with system. Skip-build: true Signed-off-by: Dalton Bohning --- utils/cq/requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/cq/requirements.txt b/utils/cq/requirements.txt index d5c264218d7..20f5c5ba003 100644 --- a/utils/cq/requirements.txt +++ b/utils/cq/requirements.txt @@ -10,4 +10,3 @@ codespell==2.3.0 # Used by ci/jira_query.py which pip installs it standalone. jira torch>=2.2.0 -scons From fbad815754a93a7bd3843e0b114114d248046f79 Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Mon, 6 Jan 2025 19:10:27 +0000 Subject: [PATCH 04/10] ... and in the readme Skip-build: true Signed-off-by: Dalton Bohning --- utils/githooks/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/githooks/README.md b/utils/githooks/README.md index 577350c1303..0b397c63863 100644 --- a/utils/githooks/README.md +++ b/utils/githooks/README.md @@ -50,7 +50,6 @@ the version output, it will fail. Try running 5. `yamllint` 6. `gofmt` 7. `codespell` -8. `scons` ### Optional tools From 13354b3f9af39c7e52e24d7b593b0eac3c349a8b Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Tue, 21 Jan 2025 18:45:13 +0000 Subject: [PATCH 05/10] reference readme if clang-format not installed Skip-build: true Signed-off-by: Dalton Bohning --- utils/githooks/README.md | 2 +- utils/githooks/pre-commit.d/50-clang-format.sh | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/utils/githooks/README.md b/utils/githooks/README.md index 0b397c63863..3bee1ffc6bc 100644 --- a/utils/githooks/README.md +++ b/utils/githooks/README.md @@ -73,8 +73,8 @@ any one of the required tools is missing. 4. `yamllint` - Linter for modified YAML configs 5. `clang-format` - Automatically formats for C/C++ files modified. If anything changed it will exit, allowing the user to inspect the changes and retry the commit. -6. `gofmt` - Automatically formats for modified GO files - See [.clang-format](../../.clang-format) for configuration +6. `gofmt` - Automatically formats for modified GO files 7. `isort` - Linter for python imports on modified python files 8. `flake8` - Linter for python files 9. `pylint` - Additional linter for modified python files diff --git a/utils/githooks/pre-commit.d/50-clang-format.sh b/utils/githooks/pre-commit.d/50-clang-format.sh index 82b725d2624..0b47e5a31e1 100755 --- a/utils/githooks/pre-commit.d/50-clang-format.sh +++ b/utils/githooks/pre-commit.d/50-clang-format.sh @@ -1,6 +1,7 @@ #!/bin/bash # # Copyright 2022-2024 Intel Corporation. +# Copyright 2025 Hewlett Packard Enterprise Development LP # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -13,11 +14,11 @@ if [ -e .git/MERGE_HEAD ]; then fi if ! command -v git-clang-format > /dev/null 2>&1; then - echo "git-clang-format not installed. Skipping" + echo "git-clang-format not installed. See ./utils/githooks/README.md for instructions" exit 0 fi if ! command -v clang-format > /dev/null 2>&1; then - echo "clang-format not installed. Skipping" + echo "clang-format not installed. See ./utils/githooks/README.md for instructions" exit 0 fi From 23df06ee4e043fc0f547e181337d0f2b0d39a918 Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Wed, 22 Jan 2025 16:40:27 +0000 Subject: [PATCH 06/10] review updates Skip-build: true Signed-off-by: Dalton Bohning --- utils/githooks/README.md | 33 ++++++++++++------- .../githooks/pre-commit.d/50-clang-format.sh | 6 ++-- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/utils/githooks/README.md b/utils/githooks/README.md index 3bee1ffc6bc..068415b5f3f 100644 --- a/utils/githooks/README.md +++ b/utils/githooks/README.md @@ -9,6 +9,7 @@ to help developers conform to DAOS community coding standards and practices and to avoid some common mistakes in the development cycle. ## Install DAOS Git Hooks + Installing is a two-step process: ### 1. Install the hooks @@ -23,7 +24,7 @@ git config core.hookspath utils/githooks Additionally, one can copy the files into an already configured path. -### 2. Install the required tools +### 2. Install the tools The Githooks framework in DAOS is such that the hooks will all run. However, some hooks will simply check for required software and are @@ -36,9 +37,11 @@ python3 -m pip install -r utils/cq/requirements.txt ``` To install system packages with your package manager - for example: ```sh -dnf install git-clang-format +sudo dnf install git-clang-format -y ``` -### Installed tools + +#### Installed tools + The following packages are used by built-in githooks. 1. `clang-format` version 14.0.5 or higher - Formatter for C code. If the check is unable to parse @@ -51,7 +54,7 @@ the version output, it will fail. Try running 6. `gofmt` 7. `codespell` -### Optional tools +#### Optional tools Additionally, [find_base.sh](find_base.sh) attempts to determine the base branch using `gh`, the Github CLI. If this isn't installed, it will use @@ -65,19 +68,25 @@ checked than expected. It is important to check the output on commit for any errors that may indicate any one of the required tools is missing. -1. copyright - Custom tool that automatically updates copyrights in modified files. -2. `codespell` - Linter for spelling mistakes in modified files +1. update-copyright - Custom tool that automatically updates copyrights in modified files. +2. codespell - Linter for spelling mistakes in modified files - See [codespell.ignores](../../ci/codespell.ignores) for ignored words. - See [words.dict](../../utils/cq/words.dict) for words added to the DAOS project. 3. Jenkinsfile - Custom linter if the Jenkinsfile is modified -4. `yamllint` - Linter for modified YAML configs -5. `clang-format` - Automatically formats for C/C++ files modified. If anything changed it will exit, +4. yamllint - Linter for modified YAML configs +5. clang-format - Automatically formats for C/C++ files modified. If anything changed it will exit, allowing the user to inspect the changes and retry the commit. - See [.clang-format](../../.clang-format) for configuration -6. `gofmt` - Automatically formats for modified GO files -7. `isort` - Linter for python imports on modified python files -8. `flake8` - Linter for python files -9. `pylint` - Additional linter for modified python files + - In some cases unwanted formatting changes are made. To disable formatting, for example: + ``` + /* clang-format off */ + ... + /* clang-format on */ + ``` +6. gofmt - Automatically formats for modified GO files +7. isort - Linter for python imports on modified python files +8. flake - Linter for python files +9. pylint - Additional linter for modified python files - See [daos_pylint.py](../../utils/cq/daos_pylint.py) for a custom wrapper around `pylint` which manages `PYTHONPATH` setup internally. 10. ftest - Custom linter for modified ftest files diff --git a/utils/githooks/pre-commit.d/50-clang-format.sh b/utils/githooks/pre-commit.d/50-clang-format.sh index 0b47e5a31e1..2ed2bb95e3f 100755 --- a/utils/githooks/pre-commit.d/50-clang-format.sh +++ b/utils/githooks/pre-commit.d/50-clang-format.sh @@ -14,11 +14,13 @@ if [ -e .git/MERGE_HEAD ]; then fi if ! command -v git-clang-format > /dev/null 2>&1; then - echo "git-clang-format not installed. See ./utils/githooks/README.md for instructions" + echo "git-clang-format not installed." + echo "See ./utils/githooks/README.md#2-install-the-required-tools for instructions." exit 0 fi if ! command -v clang-format > /dev/null 2>&1; then - echo "clang-format not installed. See ./utils/githooks/README.md for instructions" + echo "clang-format not installed." + echo "See ./utils/githooks/README.md#2-install-the-required-tools for instructions." exit 0 fi From d5bf0376aa0dec22ed1fbe61ca901ee0ced0b3e6 Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Wed, 22 Jan 2025 16:59:36 +0000 Subject: [PATCH 07/10] make clang-format githook required Skip-build: true Signed-off-by: Dalton Bohning --- utils/githooks/pre-commit.d/50-clang-format.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/githooks/pre-commit.d/50-clang-format.sh b/utils/githooks/pre-commit.d/50-clang-format.sh index 2ed2bb95e3f..87600b6c330 100755 --- a/utils/githooks/pre-commit.d/50-clang-format.sh +++ b/utils/githooks/pre-commit.d/50-clang-format.sh @@ -16,12 +16,12 @@ fi if ! command -v git-clang-format > /dev/null 2>&1; then echo "git-clang-format not installed." echo "See ./utils/githooks/README.md#2-install-the-required-tools for instructions." - exit 0 + exit 1 fi if ! command -v clang-format > /dev/null 2>&1; then echo "clang-format not installed." echo "See ./utils/githooks/README.md#2-install-the-required-tools for instructions." - exit 0 + exit 1 fi echo "Formatting C files" From 83e791e3ff15a0f1bb88f0cca4ee23be0c65a52d Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Wed, 22 Jan 2025 17:05:31 +0000 Subject: [PATCH 08/10] reference build env Skip-build: true Signed-off-by: Dalton Bohning --- utils/githooks/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/githooks/README.md b/utils/githooks/README.md index 068415b5f3f..49cbc01300a 100644 --- a/utils/githooks/README.md +++ b/utils/githooks/README.md @@ -31,7 +31,8 @@ However, some hooks will simply check for required software and are effectively a noop if such is not installed. Requirements come from a combination of `pip` and system packages and can usually be installed through standard means. -To install `pip` packages specified in [utils/cq/requirements.txt](../../utils/cq/requirements.txt): +To install `pip` packages specified in [utils/cq/requirements.txt](../../utils/cq/requirements.txt) it is recommended to setup a virtual environment and install with pip. +If you already have a [virtual environment for building](../../docs/QSG/build_from_scratch.md#python-packages) you can simply install the requirements: ```sh python3 -m pip install -r utils/cq/requirements.txt ``` From e34b6c1d1ddbce1f845442f366b0829e607a34d5 Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Thu, 23 Jan 2025 16:47:49 +0000 Subject: [PATCH 09/10] remove unnecessary check Skip-build: true Signed-off-by: Dalton Bohning --- utils/githooks/pre-commit.d/50-clang-format.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/utils/githooks/pre-commit.d/50-clang-format.sh b/utils/githooks/pre-commit.d/50-clang-format.sh index 87600b6c330..8f3d21ff92a 100755 --- a/utils/githooks/pre-commit.d/50-clang-format.sh +++ b/utils/githooks/pre-commit.d/50-clang-format.sh @@ -18,11 +18,6 @@ if ! command -v git-clang-format > /dev/null 2>&1; then echo "See ./utils/githooks/README.md#2-install-the-required-tools for instructions." exit 1 fi -if ! command -v clang-format > /dev/null 2>&1; then - echo "clang-format not installed." - echo "See ./utils/githooks/README.md#2-install-the-required-tools for instructions." - exit 1 -fi echo "Formatting C files" From 66dd1b812c4370cb02d724815f4e0c0b2499864c Mon Sep 17 00:00:00 2001 From: Dalton Bohning Date: Fri, 24 Jan 2025 15:06:13 +0000 Subject: [PATCH 10/10] review Skip-build: true Signed-off-by: Dalton Bohning --- utils/githooks/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/githooks/README.md b/utils/githooks/README.md index 49cbc01300a..f5e726e99af 100644 --- a/utils/githooks/README.md +++ b/utils/githooks/README.md @@ -36,7 +36,7 @@ If you already have a [virtual environment for building](../../docs/QSG/build_fr ```sh python3 -m pip install -r utils/cq/requirements.txt ``` -To install system packages with your package manager - for example: +Install system packages with your package manager - for example: ```sh sudo dnf install git-clang-format -y ``` @@ -45,8 +45,8 @@ sudo dnf install git-clang-format -y The following packages are used by built-in githooks. -1. `clang-format` version 14.0.5 or higher - Formatter for C code. If the check is unable to parse -the version output, it will fail. Try running +1. `git-clang-format` and `clang-format` version 14.0.5 or higher - Formatter for C code. +If the check is unable to parse the version output, it will fail. Try running `/site_scons/site_tools/extra/extra.py` to check. 2. `pylint` 3. `flake8`