Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAOS-16923 cq: make clang-format required #15685

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion .github/workflows/linting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ jobs:
- doxygen
- pylint
- codespell
# - clang-format # not required
- clang-format
- yaml-lint
- copyright
if: (!cancelled())
Expand Down
96 changes: 55 additions & 41 deletions utils/githooks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,68 +8,88 @@ 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

1. Install the hooks
Installing is a two-step process:

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 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) 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
```
Install system packages with your package manager - for example:
```sh
sudo dnf install git-clang-format -y
```

1. clang-format version 14.0.5 or higher. If the check is unable to parse
the version output, it will fail. Try running
`<root>/site_scons/site_tools/extra/extra.py` to check.
2. pylint
3. flake8
4. yamllint
5. gofmt
#### Installed tools

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.
The following packages are used by built-in githooks.

It is important to check the output on commit for any errors that may indicate
any one of the required tools is missing.
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
`<root>/site_scons/site_tools/extra/extra.py` to check.
2. `pylint`
3. `flake8`
4. `isort`
5. `yamllint`
6. `gofmt`
7. `codespell`

#### 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. 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,
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.
- See [.clang-format](../../.clang-format) for configuration
- 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

### prepare-commit-msg

Expand All @@ -79,12 +99,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
Expand Down
10 changes: 4 additions & 6 deletions utils/githooks/pre-commit.d/50-clang-format.sh
Original file line number Diff line number Diff line change
@@ -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
#
Expand All @@ -13,12 +14,9 @@ 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"
exit 0
fi
if ! command -v clang-format > /dev/null 2>&1; then
echo "clang-format not installed. Skipping"
exit 0
echo "git-clang-format not installed."
echo "See ./utils/githooks/README.md#2-install-the-required-tools for instructions."
exit 1
fi

echo "Formatting C files"
Expand Down
Loading