| Message ID | 20261015101012.89244-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabas On Thu, Oct 15, 2026 at 12:10:12PM +0200, Barnabás Pőcze wrote: > Add a job that runs clang-tidy, based on a clang build to avoid issues > arising from unsupported compiler options between gcc and clang. > > Unfortunately running clang-tidy on each source file for every change > is too time consuming, so only the affected files should be checked. > However, knowing which files are affected does not appear to be > particularly simple. The biggest challenge is posed by generated header > files, changing any of the inputs should trigger a clang-tidy run on > every source file that includes it. > > To handle that, both the "static" inputs of a target, and the compiler > generated depfiles are needed. Unfortunately, ninja does not seem to > have a subcommand, that when given a list of "changed" files, would > print the list of targets that need to be potentially rebuilt. > > So this change only implements the handling of the compiler generated > depfiles using the `deps` ninja subtool. This will miss generated > headers, but an implemention handling that seems too complex at the > moment. In practice, this means, for example, that changing e.g. only > `include/libcamera/formats.h.in` will not trigger a clang-tidy run > on any source file that includes it. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Can we first discuss the usefulness of reports like: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160#L246 it seems clear to me that clang-tidy generates a lot of false positive, at least on our current code base. How do we want to treat it ? Optional suggestions for developers/reviewers to skim through ? Mandatory to address all clang-tidy reported issues ? (unlikely, right?) > --- > I am planning to enable more checks in the libcamera repository, > but given the limitations outlined above, I wanted to see if it > is acceptable in the first place. > > Example job: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160 > --- > .gitlab-ci/lint-clang-tidy.py | 112 ++++++++++++++++++++++++++++++++++ > .gitlab-ci/lint-clang-tidy.sh | 22 +++++++ > .gitlab-ci/setup-container.sh | 2 +- > gitlab-ci.yml | 34 ++++++++++- > 4 files changed, 167 insertions(+), 3 deletions(-) > create mode 100755 .gitlab-ci/lint-clang-tidy.py > create mode 100755 .gitlab-ci/lint-clang-tidy.sh > > diff --git a/.gitlab-ci/lint-clang-tidy.py b/.gitlab-ci/lint-clang-tidy.py > new file mode 100755 > index 0000000..aae28db > --- /dev/null > +++ b/.gitlab-ci/lint-clang-tidy.py > @@ -0,0 +1,112 @@ > +#!/usr/bin/env python3 > + > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy > + > +import argparse > +import fnmatch > +import json > +import os > +import os.path > +import subprocess > + > +def main(): > + parser = argparse.ArgumentParser() > + parser.add_argument("--build-dir", type=str, required=True) > + parser.add_argument("--range", nargs=2, required=False) > + parser.add_argument("args", nargs="*", default=[]) > + > + args = parser.parse_args() > + > + try: > + exclude_patterns = [] > + with open(".clang-tidy-exclude", "r") as f: > + for line in f: > + line = line.strip() > + if not line or line.startswith("#"): > + continue > + > + exclude_patterns.append(line) > + except FileNotFoundError: > + pass > + > + # extract source files from the compilation database > + r = subprocess.run( > + [ "ninja", "-t", "compdb" ], > + cwd=args.build_dir, > + stdin=subprocess.DEVNULL, > + stdout=subprocess.PIPE, > + check=True, > + ) > + > + source_files = set() > + > + for x in json.loads(r.stdout): > + f = os.path.abspath(os.path.join(x["directory"], x["file"])) > + if not any(fnmatch.fnmatch(f, p) for p in exclude_patterns): > + source_files.add(f) > + > + print("sources files:", len(source_files)) > + > + if args.range: > + # extract changed files from git > + r = subprocess.run( > + [ "git", "diff", "--name-only", args.range[0], args.range[1] ], > + stdin=subprocess.DEVNULL, > + stdout=subprocess.PIPE, > + check=True, > + ) > + changed_files = { > + os.path.realpath(x.strip()) > + for x in r.stdout.decode("utf-8").splitlines() > + } > + > + print(f"changed files: ({len(changed_files)})") > + for x in changed_files: > + print(f"\t{x}") > + > + # extract changed dependencies > + r = subprocess.run( > + [ "ninja", "-t", "deps" ], > + cwd=args.build_dir, > + stdin=subprocess.DEVNULL, > + stdout=subprocess.PIPE, > + check=True, > + ) > + > + dependency_groups = [] > + dependency_group = set() > + for line in r.stdout.decode("utf-8").splitlines(): > + if not line.startswith(" "): > + if bool(changed_files & dependency_group): > + dependency_groups.append(dependency_group) > + > + dependency_group = set() > + else: > + dependency_group.add( > + os.path.abspath(os.path.join(args.build_dir, line.strip())) > + ) > + > + # select source files that are in changed dependency groups > + files_to_check = set() > + for f in source_files: > + if any(f in dg for dg in dependency_groups): > + files_to_check.add(f) > + else: > + files_to_check = source_files > + > + print(f"running clang-tidy on the following files: ({len(files_to_check)})") > + for f in files_to_check: > + print(f"\t{f}") > + > + if files_to_check: > + argv = [ > + "run-clang-tidy", > + "-p", args.build_dir > + ] + args.args > + > + print("executing:", argv, flush=True) > + os.execvp(argv[0], argv + list(files_to_check)) > + > +if __name__ == "__main__": > + main() > diff --git a/.gitlab-ci/lint-clang-tidy.sh b/.gitlab-ci/lint-clang-tidy.sh > new file mode 100755 > index 0000000..4c88795 > --- /dev/null > +++ b/.gitlab-ci/lint-clang-tidy.sh > @@ -0,0 +1,22 @@ > +#!/bin/bash > + > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy > +# > +# Lint with clang-tidy > + > +set -e > + > +source "$(dirname "$0")/lib.sh" > + > +clang_tidy() { > + local base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA) > + > + $(dirname "$0")/lint-clang-tidy.py \ > + --build-dir build/ \ > + --range $base $CI_COMMIT_SHA \ > + -- \ > + -j $BUILD_CONCURRENT_JOBS > +} > + > +run clang_tidy > diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh > index bb00895..ac37faf 100755 > --- a/.gitlab-ci/setup-container.sh > +++ b/.gitlab-ci/setup-container.sh > @@ -132,7 +132,7 @@ case $FDO_DISTRIBUTION_VERSION in > # Packages required to deploy the documentation > PKGS_LIBCAMERA_RUNTIME+=( openssh-client rsync ) > # Tools requires by the lint jobs. > - PKGS_LIBCAMERA_RUNTIME+=( clang-format jq python3-autopep8 reuse shellcheck ) > + PKGS_LIBCAMERA_RUNTIME+=( clang-format clang-tidy jq python3-autopep8 reuse shellcheck ) > # libclang-rt-dev for the clang ASan runtime. > PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-19-dev ) > # For lc-compliance in the `test-lc-compliance:virtual` job > diff --git a/gitlab-ci.yml b/gitlab-ci.yml > index 840eb74..3e8fe99 100644 > --- a/gitlab-ci.yml > +++ b/gitlab-ci.yml > @@ -74,7 +74,7 @@ include: > .libcamera-ci.debian:13: > variables: > FDO_DISTRIBUTION_VERSION: 'trixie' > - FDO_DISTRIBUTION_TAG: '2026-06-13.0' > + FDO_DISTRIBUTION_TAG: '2026-10-14.0' > > .container-debian: > extends: > @@ -198,9 +198,11 @@ container-arch: > paths: > - build/meson-logs/ > - build/build.ninja > + - $EXTRA_ARTIFACT > variables: > BUILD_TYPE: debug > MESON_OPTIONS: ${MESON_ALL_OPTIONS} > + EXTRA_ARTIFACT: '' > > build-full:debian:12: > extends: > @@ -255,6 +257,13 @@ build-full:debian:13: > ${MESON_ALL_OPTIONS} > -D b_sanitize=none > -D cpp_debugstl=false > + - ARCH: amd64 > + CC: clang > + CXX: clang++ > + MESON_OPTIONS: >- > + ${MESON_ALL_OPTIONS} > + ${MESON_CLANG_LIBSTDCPP_OPTIONS} > + EXTRA_ARTIFACT: build > - ARCH: amd64 > CC: clang > CXX: clang++ > @@ -262,7 +271,6 @@ build-full:debian:13: > # `cpp_debugstl=false` needed approximately between meson 1.4.0 and 1.9.3 > # with clang 18 or later due to https://github.com/mesonbuild/meson/issues/13812 > - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBCPP_OPTIONS} -D qcam=disabled -D cpp_debugstl=false > - - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBSTDCPP_OPTIONS} > - ARCH: amd64 > BUILD_TYPE: release > CC: clang > @@ -449,6 +457,28 @@ build-docs: > # Lint stage - Run checkstyle.py and check merge suitability > # ------------------------------------------------------------------------------ > > +clang-tidy: > + extends: > + - .fdo.distribution-image@debian > + - .history-jobs > + - .libcamera-ci.debian:13 > + - .libcamera-ci.scripts > + stage: lint > + needs: > + - job: build-full:debian:13 > + artifacts: true > + parallel: > + matrix: > + - ARCH: amd64 > + CC: clang > + CXX: clang++ > + MESON_OPTIONS: >- > + ${MESON_ALL_OPTIONS} > + ${MESON_CLANG_LIBSTDCPP_OPTIONS} > + EXTRA_ARTIFACT: build > + script: > + $CI_PROJECT_DIR/.gitlab-ci/lint-clang-tidy.sh > + > license: > extends: > - .fdo.distribution-image@debian > -- > 2.56.0 >
2026. 10. 16. 9:57 keltezéssel, Jacopo Mondi írta: > Hi Barnabas > > On Thu, Oct 15, 2026 at 12:10:12PM +0200, Barnabás Pőcze wrote: >> Add a job that runs clang-tidy, based on a clang build to avoid issues >> arising from unsupported compiler options between gcc and clang. >> >> Unfortunately running clang-tidy on each source file for every change >> is too time consuming, so only the affected files should be checked. >> However, knowing which files are affected does not appear to be >> particularly simple. The biggest challenge is posed by generated header >> files, changing any of the inputs should trigger a clang-tidy run on >> every source file that includes it. >> >> To handle that, both the "static" inputs of a target, and the compiler >> generated depfiles are needed. Unfortunately, ninja does not seem to >> have a subcommand, that when given a list of "changed" files, would >> print the list of targets that need to be potentially rebuilt. >> >> So this change only implements the handling of the compiler generated >> depfiles using the `deps` ninja subtool. This will miss generated >> headers, but an implemention handling that seems too complex at the >> moment. In practice, this means, for example, that changing e.g. only >> `include/libcamera/formats.h.in` will not trigger a clang-tidy run >> on any source file that includes it. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > Can we first discuss the usefulness of reports like: > https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160#L246 > > it seems clear to me that clang-tidy generates a lot of false > positive, at least on our current code base. > > How do we want to treat it ? Optional suggestions for > developers/reviewers to skim through ? Mandatory to address all > clang-tidy reported issues ? (unlikely, right?) I have forgotten to mention that indeed. My thought was to use the `WarningsAsErrors` option in the `.clang-tidy` configuration file to define a set of a checks whose violation will be regarded as an error and will fail the ci pipeline. > >> --- >> I am planning to enable more checks in the libcamera repository, >> but given the limitations outlined above, I wanted to see if it >> is acceptable in the first place. >> >> Example job: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160 >> --- >> .gitlab-ci/lint-clang-tidy.py | 112 ++++++++++++++++++++++++++++++++++ >> .gitlab-ci/lint-clang-tidy.sh | 22 +++++++ >> .gitlab-ci/setup-container.sh | 2 +- >> gitlab-ci.yml | 34 ++++++++++- >> 4 files changed, 167 insertions(+), 3 deletions(-) >> create mode 100755 .gitlab-ci/lint-clang-tidy.py >> create mode 100755 .gitlab-ci/lint-clang-tidy.sh >> >> diff --git a/.gitlab-ci/lint-clang-tidy.py b/.gitlab-ci/lint-clang-tidy.py >> new file mode 100755 >> index 0000000..aae28db >> --- /dev/null >> +++ b/.gitlab-ci/lint-clang-tidy.py >> @@ -0,0 +1,112 @@ >> +#!/usr/bin/env python3 >> + >> +# SPDX-License-Identifier: GPL-2.0-or-later >> +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy >> + >> +import argparse >> +import fnmatch >> +import json >> +import os >> +import os.path >> +import subprocess >> + >> +def main(): >> + parser = argparse.ArgumentParser() >> + parser.add_argument("--build-dir", type=str, required=True) >> + parser.add_argument("--range", nargs=2, required=False) >> + parser.add_argument("args", nargs="*", default=[]) >> + >> + args = parser.parse_args() >> + >> + try: >> + exclude_patterns = [] >> + with open(".clang-tidy-exclude", "r") as f: >> + for line in f: >> + line = line.strip() >> + if not line or line.startswith("#"): >> + continue >> + >> + exclude_patterns.append(line) >> + except FileNotFoundError: >> + pass >> + >> + # extract source files from the compilation database >> + r = subprocess.run( >> + [ "ninja", "-t", "compdb" ], >> + cwd=args.build_dir, >> + stdin=subprocess.DEVNULL, >> + stdout=subprocess.PIPE, >> + check=True, >> + ) >> + >> + source_files = set() >> + >> + for x in json.loads(r.stdout): >> + f = os.path.abspath(os.path.join(x["directory"], x["file"])) >> + if not any(fnmatch.fnmatch(f, p) for p in exclude_patterns): >> + source_files.add(f) >> + >> + print("sources files:", len(source_files)) >> + >> + if args.range: >> + # extract changed files from git >> + r = subprocess.run( >> + [ "git", "diff", "--name-only", args.range[0], args.range[1] ], >> + stdin=subprocess.DEVNULL, >> + stdout=subprocess.PIPE, >> + check=True, >> + ) >> + changed_files = { >> + os.path.realpath(x.strip()) >> + for x in r.stdout.decode("utf-8").splitlines() >> + } >> + >> + print(f"changed files: ({len(changed_files)})") >> + for x in changed_files: >> + print(f"\t{x}") >> + >> + # extract changed dependencies >> + r = subprocess.run( >> + [ "ninja", "-t", "deps" ], >> + cwd=args.build_dir, >> + stdin=subprocess.DEVNULL, >> + stdout=subprocess.PIPE, >> + check=True, >> + ) >> + >> + dependency_groups = [] >> + dependency_group = set() >> + for line in r.stdout.decode("utf-8").splitlines(): >> + if not line.startswith(" "): >> + if bool(changed_files & dependency_group): >> + dependency_groups.append(dependency_group) >> + >> + dependency_group = set() >> + else: >> + dependency_group.add( >> + os.path.abspath(os.path.join(args.build_dir, line.strip())) >> + ) >> + >> + # select source files that are in changed dependency groups >> + files_to_check = set() >> + for f in source_files: >> + if any(f in dg for dg in dependency_groups): >> + files_to_check.add(f) >> + else: >> + files_to_check = source_files >> + >> + print(f"running clang-tidy on the following files: ({len(files_to_check)})") >> + for f in files_to_check: >> + print(f"\t{f}") >> + >> + if files_to_check: >> + argv = [ >> + "run-clang-tidy", >> + "-p", args.build_dir >> + ] + args.args >> + >> + print("executing:", argv, flush=True) >> + os.execvp(argv[0], argv + list(files_to_check)) >> + >> +if __name__ == "__main__": >> + main() >> diff --git a/.gitlab-ci/lint-clang-tidy.sh b/.gitlab-ci/lint-clang-tidy.sh >> new file mode 100755 >> index 0000000..4c88795 >> --- /dev/null >> +++ b/.gitlab-ci/lint-clang-tidy.sh >> @@ -0,0 +1,22 @@ >> +#!/bin/bash >> + >> +# SPDX-License-Identifier: GPL-2.0-or-later >> +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy >> +# >> +# Lint with clang-tidy >> + >> +set -e >> + >> +source "$(dirname "$0")/lib.sh" >> + >> +clang_tidy() { >> + local base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA) >> + >> + $(dirname "$0")/lint-clang-tidy.py \ >> + --build-dir build/ \ >> + --range $base $CI_COMMIT_SHA \ >> + -- \ >> + -j $BUILD_CONCURRENT_JOBS >> +} >> + >> +run clang_tidy >> diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh >> index bb00895..ac37faf 100755 >> --- a/.gitlab-ci/setup-container.sh >> +++ b/.gitlab-ci/setup-container.sh >> @@ -132,7 +132,7 @@ case $FDO_DISTRIBUTION_VERSION in >> # Packages required to deploy the documentation >> PKGS_LIBCAMERA_RUNTIME+=( openssh-client rsync ) >> # Tools requires by the lint jobs. >> - PKGS_LIBCAMERA_RUNTIME+=( clang-format jq python3-autopep8 reuse shellcheck ) >> + PKGS_LIBCAMERA_RUNTIME+=( clang-format clang-tidy jq python3-autopep8 reuse shellcheck ) >> # libclang-rt-dev for the clang ASan runtime. >> PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-19-dev ) >> # For lc-compliance in the `test-lc-compliance:virtual` job >> diff --git a/gitlab-ci.yml b/gitlab-ci.yml >> index 840eb74..3e8fe99 100644 >> --- a/gitlab-ci.yml >> +++ b/gitlab-ci.yml >> @@ -74,7 +74,7 @@ include: >> .libcamera-ci.debian:13: >> variables: >> FDO_DISTRIBUTION_VERSION: 'trixie' >> - FDO_DISTRIBUTION_TAG: '2026-06-13.0' >> + FDO_DISTRIBUTION_TAG: '2026-10-14.0' >> >> .container-debian: >> extends: >> @@ -198,9 +198,11 @@ container-arch: >> paths: >> - build/meson-logs/ >> - build/build.ninja >> + - $EXTRA_ARTIFACT >> variables: >> BUILD_TYPE: debug >> MESON_OPTIONS: ${MESON_ALL_OPTIONS} >> + EXTRA_ARTIFACT: '' >> >> build-full:debian:12: >> extends: >> @@ -255,6 +257,13 @@ build-full:debian:13: >> ${MESON_ALL_OPTIONS} >> -D b_sanitize=none >> -D cpp_debugstl=false >> + - ARCH: amd64 >> + CC: clang >> + CXX: clang++ >> + MESON_OPTIONS: >- >> + ${MESON_ALL_OPTIONS} >> + ${MESON_CLANG_LIBSTDCPP_OPTIONS} >> + EXTRA_ARTIFACT: build >> - ARCH: amd64 >> CC: clang >> CXX: clang++ >> @@ -262,7 +271,6 @@ build-full:debian:13: >> # `cpp_debugstl=false` needed approximately between meson 1.4.0 and 1.9.3 >> # with clang 18 or later due to https://github.com/mesonbuild/meson/issues/13812 >> - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBCPP_OPTIONS} -D qcam=disabled -D cpp_debugstl=false >> - - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBSTDCPP_OPTIONS} >> - ARCH: amd64 >> BUILD_TYPE: release >> CC: clang >> @@ -449,6 +457,28 @@ build-docs: >> # Lint stage - Run checkstyle.py and check merge suitability >> # ------------------------------------------------------------------------------ >> >> +clang-tidy: >> + extends: >> + - .fdo.distribution-image@debian >> + - .history-jobs >> + - .libcamera-ci.debian:13 >> + - .libcamera-ci.scripts >> + stage: lint >> + needs: >> + - job: build-full:debian:13 >> + artifacts: true >> + parallel: >> + matrix: >> + - ARCH: amd64 >> + CC: clang >> + CXX: clang++ >> + MESON_OPTIONS: >- >> + ${MESON_ALL_OPTIONS} >> + ${MESON_CLANG_LIBSTDCPP_OPTIONS} >> + EXTRA_ARTIFACT: build >> + script: >> + $CI_PROJECT_DIR/.gitlab-ci/lint-clang-tidy.sh >> + >> license: >> extends: >> - .fdo.distribution-image@debian >> -- >> 2.56.0 >>
On Fri, Oct 16, 2026 at 10:13:41AM +0200, Barnabás Pőcze wrote: > 2026. 10. 16. 9:57 keltezéssel, Jacopo Mondi írta: > > Hi Barnabas > > > > On Thu, Oct 15, 2026 at 12:10:12PM +0200, Barnabás Pőcze wrote: > > > Add a job that runs clang-tidy, based on a clang build to avoid issues > > > arising from unsupported compiler options between gcc and clang. > > > > > > Unfortunately running clang-tidy on each source file for every change > > > is too time consuming, so only the affected files should be checked. > > > However, knowing which files are affected does not appear to be > > > particularly simple. The biggest challenge is posed by generated header > > > files, changing any of the inputs should trigger a clang-tidy run on > > > every source file that includes it. > > > > > > To handle that, both the "static" inputs of a target, and the compiler > > > generated depfiles are needed. Unfortunately, ninja does not seem to > > > have a subcommand, that when given a list of "changed" files, would > > > print the list of targets that need to be potentially rebuilt. > > > > > > So this change only implements the handling of the compiler generated > > > depfiles using the `deps` ninja subtool. This will miss generated > > > headers, but an implemention handling that seems too complex at the > > > moment. In practice, this means, for example, that changing e.g. only > > > `include/libcamera/formats.h.in` will not trigger a clang-tidy run > > > on any source file that includes it. > > > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > Can we first discuss the usefulness of reports like: > > https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160#L246 > > > > it seems clear to me that clang-tidy generates a lot of false > > positive, at least on our current code base. > > > > How do we want to treat it ? Optional suggestions for > > developers/reviewers to skim through ? Mandatory to address all > > clang-tidy reported issues ? (unlikely, right?) > > I have forgotten to mention that indeed. My thought was to use the > `WarningsAsErrors` option in the `.clang-tidy` configuration file to > define a set of a checks whose violation will be regarded as an error > and will fail the ci pipeline. From this list ? https://clang.llvm.org/extra/clang-tidy/checks/list.html Should we start by deciding what constitute a CI error among these ? Alternatively, the clang-tidy test should be treated as informative only ? > > > > > > > --- > > > I am planning to enable more checks in the libcamera repository, > > > but given the limitations outlined above, I wanted to see if it > > > is acceptable in the first place. > > > > > > Example job: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160 > > > --- > > > .gitlab-ci/lint-clang-tidy.py | 112 ++++++++++++++++++++++++++++++++++ > > > .gitlab-ci/lint-clang-tidy.sh | 22 +++++++ > > > .gitlab-ci/setup-container.sh | 2 +- > > > gitlab-ci.yml | 34 ++++++++++- > > > 4 files changed, 167 insertions(+), 3 deletions(-) > > > create mode 100755 .gitlab-ci/lint-clang-tidy.py > > > create mode 100755 .gitlab-ci/lint-clang-tidy.sh > > > > > > diff --git a/.gitlab-ci/lint-clang-tidy.py b/.gitlab-ci/lint-clang-tidy.py > > > new file mode 100755 > > > index 0000000..aae28db > > > --- /dev/null > > > +++ b/.gitlab-ci/lint-clang-tidy.py > > > @@ -0,0 +1,112 @@ > > > +#!/usr/bin/env python3 > > > + > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy > > > + > > > +import argparse > > > +import fnmatch > > > +import json > > > +import os > > > +import os.path > > > +import subprocess > > > + > > > +def main(): > > > + parser = argparse.ArgumentParser() > > > + parser.add_argument("--build-dir", type=str, required=True) > > > + parser.add_argument("--range", nargs=2, required=False) > > > + parser.add_argument("args", nargs="*", default=[]) > > > + > > > + args = parser.parse_args() > > > + > > > + try: > > > + exclude_patterns = [] > > > + with open(".clang-tidy-exclude", "r") as f: > > > + for line in f: > > > + line = line.strip() > > > + if not line or line.startswith("#"): > > > + continue > > > + > > > + exclude_patterns.append(line) > > > + except FileNotFoundError: > > > + pass > > > + > > > + # extract source files from the compilation database > > > + r = subprocess.run( > > > + [ "ninja", "-t", "compdb" ], > > > + cwd=args.build_dir, > > > + stdin=subprocess.DEVNULL, > > > + stdout=subprocess.PIPE, > > > + check=True, > > > + ) > > > + > > > + source_files = set() > > > + > > > + for x in json.loads(r.stdout): > > > + f = os.path.abspath(os.path.join(x["directory"], x["file"])) > > > + if not any(fnmatch.fnmatch(f, p) for p in exclude_patterns): > > > + source_files.add(f) > > > + > > > + print("sources files:", len(source_files)) > > > + > > > + if args.range: > > > + # extract changed files from git > > > + r = subprocess.run( > > > + [ "git", "diff", "--name-only", args.range[0], args.range[1] ], > > > + stdin=subprocess.DEVNULL, > > > + stdout=subprocess.PIPE, > > > + check=True, > > > + ) > > > + changed_files = { > > > + os.path.realpath(x.strip()) > > > + for x in r.stdout.decode("utf-8").splitlines() > > > + } > > > + > > > + print(f"changed files: ({len(changed_files)})") > > > + for x in changed_files: > > > + print(f"\t{x}") > > > + > > > + # extract changed dependencies > > > + r = subprocess.run( > > > + [ "ninja", "-t", "deps" ], > > > + cwd=args.build_dir, > > > + stdin=subprocess.DEVNULL, > > > + stdout=subprocess.PIPE, > > > + check=True, > > > + ) > > > + > > > + dependency_groups = [] > > > + dependency_group = set() > > > + for line in r.stdout.decode("utf-8").splitlines(): > > > + if not line.startswith(" "): > > > + if bool(changed_files & dependency_group): > > > + dependency_groups.append(dependency_group) > > > + > > > + dependency_group = set() > > > + else: > > > + dependency_group.add( > > > + os.path.abspath(os.path.join(args.build_dir, line.strip())) > > > + ) > > > + > > > + # select source files that are in changed dependency groups > > > + files_to_check = set() > > > + for f in source_files: > > > + if any(f in dg for dg in dependency_groups): > > > + files_to_check.add(f) > > > + else: > > > + files_to_check = source_files > > > + > > > + print(f"running clang-tidy on the following files: ({len(files_to_check)})") > > > + for f in files_to_check: > > > + print(f"\t{f}") > > > + > > > + if files_to_check: > > > + argv = [ > > > + "run-clang-tidy", > > > + "-p", args.build_dir > > > + ] + args.args > > > + > > > + print("executing:", argv, flush=True) > > > + os.execvp(argv[0], argv + list(files_to_check)) > > > + > > > +if __name__ == "__main__": > > > + main() > > > diff --git a/.gitlab-ci/lint-clang-tidy.sh b/.gitlab-ci/lint-clang-tidy.sh > > > new file mode 100755 > > > index 0000000..4c88795 > > > --- /dev/null > > > +++ b/.gitlab-ci/lint-clang-tidy.sh > > > @@ -0,0 +1,22 @@ > > > +#!/bin/bash > > > + > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy > > > +# > > > +# Lint with clang-tidy > > > + > > > +set -e > > > + > > > +source "$(dirname "$0")/lib.sh" > > > + > > > +clang_tidy() { > > > + local base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA) > > > + > > > + $(dirname "$0")/lint-clang-tidy.py \ > > > + --build-dir build/ \ > > > + --range $base $CI_COMMIT_SHA \ > > > + -- \ > > > + -j $BUILD_CONCURRENT_JOBS > > > +} > > > + > > > +run clang_tidy > > > diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh > > > index bb00895..ac37faf 100755 > > > --- a/.gitlab-ci/setup-container.sh > > > +++ b/.gitlab-ci/setup-container.sh > > > @@ -132,7 +132,7 @@ case $FDO_DISTRIBUTION_VERSION in > > > # Packages required to deploy the documentation > > > PKGS_LIBCAMERA_RUNTIME+=( openssh-client rsync ) > > > # Tools requires by the lint jobs. > > > - PKGS_LIBCAMERA_RUNTIME+=( clang-format jq python3-autopep8 reuse shellcheck ) > > > + PKGS_LIBCAMERA_RUNTIME+=( clang-format clang-tidy jq python3-autopep8 reuse shellcheck ) > > > # libclang-rt-dev for the clang ASan runtime. > > > PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-19-dev ) > > > # For lc-compliance in the `test-lc-compliance:virtual` job > > > diff --git a/gitlab-ci.yml b/gitlab-ci.yml > > > index 840eb74..3e8fe99 100644 > > > --- a/gitlab-ci.yml > > > +++ b/gitlab-ci.yml > > > @@ -74,7 +74,7 @@ include: > > > .libcamera-ci.debian:13: > > > variables: > > > FDO_DISTRIBUTION_VERSION: 'trixie' > > > - FDO_DISTRIBUTION_TAG: '2026-06-13.0' > > > + FDO_DISTRIBUTION_TAG: '2026-10-14.0' > > > > > > .container-debian: > > > extends: > > > @@ -198,9 +198,11 @@ container-arch: > > > paths: > > > - build/meson-logs/ > > > - build/build.ninja > > > + - $EXTRA_ARTIFACT > > > variables: > > > BUILD_TYPE: debug > > > MESON_OPTIONS: ${MESON_ALL_OPTIONS} > > > + EXTRA_ARTIFACT: '' > > > > > > build-full:debian:12: > > > extends: > > > @@ -255,6 +257,13 @@ build-full:debian:13: > > > ${MESON_ALL_OPTIONS} > > > -D b_sanitize=none > > > -D cpp_debugstl=false > > > + - ARCH: amd64 > > > + CC: clang > > > + CXX: clang++ > > > + MESON_OPTIONS: >- > > > + ${MESON_ALL_OPTIONS} > > > + ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > + EXTRA_ARTIFACT: build > > > - ARCH: amd64 > > > CC: clang > > > CXX: clang++ > > > @@ -262,7 +271,6 @@ build-full:debian:13: > > > # `cpp_debugstl=false` needed approximately between meson 1.4.0 and 1.9.3 > > > # with clang 18 or later due to https://github.com/mesonbuild/meson/issues/13812 > > > - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBCPP_OPTIONS} -D qcam=disabled -D cpp_debugstl=false > > > - - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > - ARCH: amd64 > > > BUILD_TYPE: release > > > CC: clang > > > @@ -449,6 +457,28 @@ build-docs: > > > # Lint stage - Run checkstyle.py and check merge suitability > > > # ------------------------------------------------------------------------------ > > > > > > +clang-tidy: > > > + extends: > > > + - .fdo.distribution-image@debian > > > + - .history-jobs > > > + - .libcamera-ci.debian:13 > > > + - .libcamera-ci.scripts > > > + stage: lint > > > + needs: > > > + - job: build-full:debian:13 > > > + artifacts: true > > > + parallel: > > > + matrix: > > > + - ARCH: amd64 > > > + CC: clang > > > + CXX: clang++ > > > + MESON_OPTIONS: >- > > > + ${MESON_ALL_OPTIONS} > > > + ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > + EXTRA_ARTIFACT: build > > > + script: > > > + $CI_PROJECT_DIR/.gitlab-ci/lint-clang-tidy.sh > > > + > > > license: > > > extends: > > > - .fdo.distribution-image@debian > > > -- > > > 2.56.0 > > > >
2026. 10. 16. 10:22 keltezéssel, Jacopo Mondi írta: > On Fri, Oct 16, 2026 at 10:13:41AM +0200, Barnabás Pőcze wrote: >> 2026. 10. 16. 9:57 keltezéssel, Jacopo Mondi írta: >>> Hi Barnabas >>> >>> On Thu, Oct 15, 2026 at 12:10:12PM +0200, Barnabás Pőcze wrote: >>>> Add a job that runs clang-tidy, based on a clang build to avoid issues >>>> arising from unsupported compiler options between gcc and clang. >>>> >>>> Unfortunately running clang-tidy on each source file for every change >>>> is too time consuming, so only the affected files should be checked. >>>> However, knowing which files are affected does not appear to be >>>> particularly simple. The biggest challenge is posed by generated header >>>> files, changing any of the inputs should trigger a clang-tidy run on >>>> every source file that includes it. >>>> >>>> To handle that, both the "static" inputs of a target, and the compiler >>>> generated depfiles are needed. Unfortunately, ninja does not seem to >>>> have a subcommand, that when given a list of "changed" files, would >>>> print the list of targets that need to be potentially rebuilt. >>>> >>>> So this change only implements the handling of the compiler generated >>>> depfiles using the `deps` ninja subtool. This will miss generated >>>> headers, but an implemention handling that seems too complex at the >>>> moment. In practice, this means, for example, that changing e.g. only >>>> `include/libcamera/formats.h.in` will not trigger a clang-tidy run >>>> on any source file that includes it. >>>> >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>> >>> Can we first discuss the usefulness of reports like: >>> https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160#L246 >>> >>> it seems clear to me that clang-tidy generates a lot of false >>> positive, at least on our current code base. >>> >>> How do we want to treat it ? Optional suggestions for >>> developers/reviewers to skim through ? Mandatory to address all >>> clang-tidy reported issues ? (unlikely, right?) >> >> I have forgotten to mention that indeed. My thought was to use the >> `WarningsAsErrors` option in the `.clang-tidy` configuration file to >> define a set of a checks whose violation will be regarded as an error >> and will fail the ci pipeline. > > From this list ? > https://clang.llvm.org/extra/clang-tidy/checks/list.html > > Should we start by deciding what constitute a CI error among these ? Correct, I have already started have a list, but first wanted to see if clang-tidy in general seems acceptable. > > Alternatively, the clang-tidy test should be treated as informative only ? Possibly, but I'm not a big believer in those. At the moment I think a good approach is to consider certain checks "important" and consider any violation an error, but leave most reasonable checks enabled, because those can still be useful if one is writing a new piece of code and sees those warnings via clangd or such. > >> >> >>> >>>> --- >>>> I am planning to enable more checks in the libcamera repository, >>>> but given the limitations outlined above, I wanted to see if it >>>> is acceptable in the first place. >>>> >>>> Example job: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160 >>>> --- >>>> .gitlab-ci/lint-clang-tidy.py | 112 ++++++++++++++++++++++++++++++++++ >>>> .gitlab-ci/lint-clang-tidy.sh | 22 +++++++ >>>> .gitlab-ci/setup-container.sh | 2 +- >>>> gitlab-ci.yml | 34 ++++++++++- >>>> 4 files changed, 167 insertions(+), 3 deletions(-) >>>> create mode 100755 .gitlab-ci/lint-clang-tidy.py >>>> create mode 100755 .gitlab-ci/lint-clang-tidy.sh >>>> >>>> diff --git a/.gitlab-ci/lint-clang-tidy.py b/.gitlab-ci/lint-clang-tidy.py >>>> new file mode 100755 >>>> index 0000000..aae28db >>>> --- /dev/null >>>> +++ b/.gitlab-ci/lint-clang-tidy.py >>>> @@ -0,0 +1,112 @@ >>>> +#!/usr/bin/env python3 >>>> + >>>> +# SPDX-License-Identifier: GPL-2.0-or-later >>>> +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy >>>> + >>>> +import argparse >>>> +import fnmatch >>>> +import json >>>> +import os >>>> +import os.path >>>> +import subprocess >>>> + >>>> +def main(): >>>> + parser = argparse.ArgumentParser() >>>> + parser.add_argument("--build-dir", type=str, required=True) >>>> + parser.add_argument("--range", nargs=2, required=False) >>>> + parser.add_argument("args", nargs="*", default=[]) >>>> + >>>> + args = parser.parse_args() >>>> + >>>> + try: >>>> + exclude_patterns = [] >>>> + with open(".clang-tidy-exclude", "r") as f: >>>> + for line in f: >>>> + line = line.strip() >>>> + if not line or line.startswith("#"): >>>> + continue >>>> + >>>> + exclude_patterns.append(line) >>>> + except FileNotFoundError: >>>> + pass >>>> + >>>> + # extract source files from the compilation database >>>> + r = subprocess.run( >>>> + [ "ninja", "-t", "compdb" ], >>>> + cwd=args.build_dir, >>>> + stdin=subprocess.DEVNULL, >>>> + stdout=subprocess.PIPE, >>>> + check=True, >>>> + ) >>>> + >>>> + source_files = set() >>>> + >>>> + for x in json.loads(r.stdout): >>>> + f = os.path.abspath(os.path.join(x["directory"], x["file"])) >>>> + if not any(fnmatch.fnmatch(f, p) for p in exclude_patterns): >>>> + source_files.add(f) >>>> + >>>> + print("sources files:", len(source_files)) >>>> + >>>> + if args.range: >>>> + # extract changed files from git >>>> + r = subprocess.run( >>>> + [ "git", "diff", "--name-only", args.range[0], args.range[1] ], >>>> + stdin=subprocess.DEVNULL, >>>> + stdout=subprocess.PIPE, >>>> + check=True, >>>> + ) >>>> + changed_files = { >>>> + os.path.realpath(x.strip()) >>>> + for x in r.stdout.decode("utf-8").splitlines() >>>> + } >>>> + >>>> + print(f"changed files: ({len(changed_files)})") >>>> + for x in changed_files: >>>> + print(f"\t{x}") >>>> + >>>> + # extract changed dependencies >>>> + r = subprocess.run( >>>> + [ "ninja", "-t", "deps" ], >>>> + cwd=args.build_dir, >>>> + stdin=subprocess.DEVNULL, >>>> + stdout=subprocess.PIPE, >>>> + check=True, >>>> + ) >>>> + >>>> + dependency_groups = [] >>>> + dependency_group = set() >>>> + for line in r.stdout.decode("utf-8").splitlines(): >>>> + if not line.startswith(" "): >>>> + if bool(changed_files & dependency_group): >>>> + dependency_groups.append(dependency_group) >>>> + >>>> + dependency_group = set() >>>> + else: >>>> + dependency_group.add( >>>> + os.path.abspath(os.path.join(args.build_dir, line.strip())) >>>> + ) >>>> + >>>> + # select source files that are in changed dependency groups >>>> + files_to_check = set() >>>> + for f in source_files: >>>> + if any(f in dg for dg in dependency_groups): >>>> + files_to_check.add(f) >>>> + else: >>>> + files_to_check = source_files >>>> + >>>> + print(f"running clang-tidy on the following files: ({len(files_to_check)})") >>>> + for f in files_to_check: >>>> + print(f"\t{f}") >>>> + >>>> + if files_to_check: >>>> + argv = [ >>>> + "run-clang-tidy", >>>> + "-p", args.build_dir >>>> + ] + args.args >>>> + >>>> + print("executing:", argv, flush=True) >>>> + os.execvp(argv[0], argv + list(files_to_check)) >>>> + >>>> +if __name__ == "__main__": >>>> + main() >>>> diff --git a/.gitlab-ci/lint-clang-tidy.sh b/.gitlab-ci/lint-clang-tidy.sh >>>> new file mode 100755 >>>> index 0000000..4c88795 >>>> --- /dev/null >>>> +++ b/.gitlab-ci/lint-clang-tidy.sh >>>> @@ -0,0 +1,22 @@ >>>> +#!/bin/bash >>>> + >>>> +# SPDX-License-Identifier: GPL-2.0-or-later >>>> +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy >>>> +# >>>> +# Lint with clang-tidy >>>> + >>>> +set -e >>>> + >>>> +source "$(dirname "$0")/lib.sh" >>>> + >>>> +clang_tidy() { >>>> + local base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA) >>>> + >>>> + $(dirname "$0")/lint-clang-tidy.py \ >>>> + --build-dir build/ \ >>>> + --range $base $CI_COMMIT_SHA \ >>>> + -- \ >>>> + -j $BUILD_CONCURRENT_JOBS >>>> +} >>>> + >>>> +run clang_tidy >>>> diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh >>>> index bb00895..ac37faf 100755 >>>> --- a/.gitlab-ci/setup-container.sh >>>> +++ b/.gitlab-ci/setup-container.sh >>>> @@ -132,7 +132,7 @@ case $FDO_DISTRIBUTION_VERSION in >>>> # Packages required to deploy the documentation >>>> PKGS_LIBCAMERA_RUNTIME+=( openssh-client rsync ) >>>> # Tools requires by the lint jobs. >>>> - PKGS_LIBCAMERA_RUNTIME+=( clang-format jq python3-autopep8 reuse shellcheck ) >>>> + PKGS_LIBCAMERA_RUNTIME+=( clang-format clang-tidy jq python3-autopep8 reuse shellcheck ) >>>> # libclang-rt-dev for the clang ASan runtime. >>>> PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-19-dev ) >>>> # For lc-compliance in the `test-lc-compliance:virtual` job >>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml >>>> index 840eb74..3e8fe99 100644 >>>> --- a/gitlab-ci.yml >>>> +++ b/gitlab-ci.yml >>>> @@ -74,7 +74,7 @@ include: >>>> .libcamera-ci.debian:13: >>>> variables: >>>> FDO_DISTRIBUTION_VERSION: 'trixie' >>>> - FDO_DISTRIBUTION_TAG: '2026-06-13.0' >>>> + FDO_DISTRIBUTION_TAG: '2026-10-14.0' >>>> >>>> .container-debian: >>>> extends: >>>> @@ -198,9 +198,11 @@ container-arch: >>>> paths: >>>> - build/meson-logs/ >>>> - build/build.ninja >>>> + - $EXTRA_ARTIFACT >>>> variables: >>>> BUILD_TYPE: debug >>>> MESON_OPTIONS: ${MESON_ALL_OPTIONS} >>>> + EXTRA_ARTIFACT: '' >>>> >>>> build-full:debian:12: >>>> extends: >>>> @@ -255,6 +257,13 @@ build-full:debian:13: >>>> ${MESON_ALL_OPTIONS} >>>> -D b_sanitize=none >>>> -D cpp_debugstl=false >>>> + - ARCH: amd64 >>>> + CC: clang >>>> + CXX: clang++ >>>> + MESON_OPTIONS: >- >>>> + ${MESON_ALL_OPTIONS} >>>> + ${MESON_CLANG_LIBSTDCPP_OPTIONS} >>>> + EXTRA_ARTIFACT: build >>>> - ARCH: amd64 >>>> CC: clang >>>> CXX: clang++ >>>> @@ -262,7 +271,6 @@ build-full:debian:13: >>>> # `cpp_debugstl=false` needed approximately between meson 1.4.0 and 1.9.3 >>>> # with clang 18 or later due to https://github.com/mesonbuild/meson/issues/13812 >>>> - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBCPP_OPTIONS} -D qcam=disabled -D cpp_debugstl=false >>>> - - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBSTDCPP_OPTIONS} >>>> - ARCH: amd64 >>>> BUILD_TYPE: release >>>> CC: clang >>>> @@ -449,6 +457,28 @@ build-docs: >>>> # Lint stage - Run checkstyle.py and check merge suitability >>>> # ------------------------------------------------------------------------------ >>>> >>>> +clang-tidy: >>>> + extends: >>>> + - .fdo.distribution-image@debian >>>> + - .history-jobs >>>> + - .libcamera-ci.debian:13 >>>> + - .libcamera-ci.scripts >>>> + stage: lint >>>> + needs: >>>> + - job: build-full:debian:13 >>>> + artifacts: true >>>> + parallel: >>>> + matrix: >>>> + - ARCH: amd64 >>>> + CC: clang >>>> + CXX: clang++ >>>> + MESON_OPTIONS: >- >>>> + ${MESON_ALL_OPTIONS} >>>> + ${MESON_CLANG_LIBSTDCPP_OPTIONS} >>>> + EXTRA_ARTIFACT: build >>>> + script: >>>> + $CI_PROJECT_DIR/.gitlab-ci/lint-clang-tidy.sh >>>> + >>>> license: >>>> extends: >>>> - .fdo.distribution-image@debian >>>> -- >>>> 2.56.0 >>>> >>
On Fri, Oct 16, 2026 at 10:32:38AM +0200, Barnabás Pőcze wrote: > 2026. 10. 16. 10:22 keltezéssel, Jacopo Mondi írta: > > On Fri, Oct 16, 2026 at 10:13:41AM +0200, Barnabás Pőcze wrote: > > > 2026. 10. 16. 9:57 keltezéssel, Jacopo Mondi írta: > > > > Hi Barnabas > > > > > > > > On Thu, Oct 15, 2026 at 12:10:12PM +0200, Barnabás Pőcze wrote: > > > > > Add a job that runs clang-tidy, based on a clang build to avoid issues > > > > > arising from unsupported compiler options between gcc and clang. > > > > > > > > > > Unfortunately running clang-tidy on each source file for every change > > > > > is too time consuming, so only the affected files should be checked. > > > > > However, knowing which files are affected does not appear to be > > > > > particularly simple. The biggest challenge is posed by generated header > > > > > files, changing any of the inputs should trigger a clang-tidy run on > > > > > every source file that includes it. > > > > > > > > > > To handle that, both the "static" inputs of a target, and the compiler > > > > > generated depfiles are needed. Unfortunately, ninja does not seem to > > > > > have a subcommand, that when given a list of "changed" files, would > > > > > print the list of targets that need to be potentially rebuilt. > > > > > > > > > > So this change only implements the handling of the compiler generated > > > > > depfiles using the `deps` ninja subtool. This will miss generated > > > > > headers, but an implemention handling that seems too complex at the > > > > > moment. In practice, this means, for example, that changing e.g. only > > > > > `include/libcamera/formats.h.in` will not trigger a clang-tidy run > > > > > on any source file that includes it. > > > > > > > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > > > > > Can we first discuss the usefulness of reports like: > > > > https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160#L246 > > > > > > > > it seems clear to me that clang-tidy generates a lot of false > > > > positive, at least on our current code base. > > > > > > > > How do we want to treat it ? Optional suggestions for > > > > developers/reviewers to skim through ? Mandatory to address all > > > > clang-tidy reported issues ? (unlikely, right?) > > > > > > I have forgotten to mention that indeed. My thought was to use the > > > `WarningsAsErrors` option in the `.clang-tidy` configuration file to > > > define a set of a checks whose violation will be regarded as an error > > > and will fail the ci pipeline. > > > > From this list ? > > https://clang.llvm.org/extra/clang-tidy/checks/list.html > > > > Should we start by deciding what constitute a CI error among these ? > > Correct, I have already started have a list, but first wanted to see if > clang-tidy in general seems acceptable. > > > > > > Alternatively, the clang-tidy test should be treated as informative only ? > > Possibly, but I'm not a big believer in those. At the moment I think a good > approach is to consider certain checks "important" and consider any violation > an error, but leave most reasonable checks enabled, because those can still > be useful if one is writing a new piece of code and sees those warnings via > clangd or such. Do you think it would be possibile to integrate clang-tidy in checkstyle so that it can also be run locally even before submitting a patch for CI ? > > > > > > > > > > > > > > > > > > > --- > > > > > I am planning to enable more checks in the libcamera repository, > > > > > but given the limitations outlined above, I wanted to see if it > > > > > is acceptable in the first place. > > > > > > > > > > Example job: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160 > > > > > --- > > > > > .gitlab-ci/lint-clang-tidy.py | 112 ++++++++++++++++++++++++++++++++++ > > > > > .gitlab-ci/lint-clang-tidy.sh | 22 +++++++ > > > > > .gitlab-ci/setup-container.sh | 2 +- > > > > > gitlab-ci.yml | 34 ++++++++++- > > > > > 4 files changed, 167 insertions(+), 3 deletions(-) > > > > > create mode 100755 .gitlab-ci/lint-clang-tidy.py > > > > > create mode 100755 .gitlab-ci/lint-clang-tidy.sh > > > > > > > > > > diff --git a/.gitlab-ci/lint-clang-tidy.py b/.gitlab-ci/lint-clang-tidy.py > > > > > new file mode 100755 > > > > > index 0000000..aae28db > > > > > --- /dev/null > > > > > +++ b/.gitlab-ci/lint-clang-tidy.py > > > > > @@ -0,0 +1,112 @@ > > > > > +#!/usr/bin/env python3 > > > > > + > > > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > > > +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy > > > > > + > > > > > +import argparse > > > > > +import fnmatch > > > > > +import json > > > > > +import os > > > > > +import os.path > > > > > +import subprocess > > > > > + > > > > > +def main(): > > > > > + parser = argparse.ArgumentParser() > > > > > + parser.add_argument("--build-dir", type=str, required=True) > > > > > + parser.add_argument("--range", nargs=2, required=False) > > > > > + parser.add_argument("args", nargs="*", default=[]) > > > > > + > > > > > + args = parser.parse_args() > > > > > + > > > > > + try: > > > > > + exclude_patterns = [] > > > > > + with open(".clang-tidy-exclude", "r") as f: > > > > > + for line in f: > > > > > + line = line.strip() > > > > > + if not line or line.startswith("#"): > > > > > + continue > > > > > + > > > > > + exclude_patterns.append(line) > > > > > + except FileNotFoundError: > > > > > + pass > > > > > + > > > > > + # extract source files from the compilation database > > > > > + r = subprocess.run( > > > > > + [ "ninja", "-t", "compdb" ], > > > > > + cwd=args.build_dir, > > > > > + stdin=subprocess.DEVNULL, > > > > > + stdout=subprocess.PIPE, > > > > > + check=True, > > > > > + ) > > > > > + > > > > > + source_files = set() > > > > > + > > > > > + for x in json.loads(r.stdout): > > > > > + f = os.path.abspath(os.path.join(x["directory"], x["file"])) > > > > > + if not any(fnmatch.fnmatch(f, p) for p in exclude_patterns): > > > > > + source_files.add(f) > > > > > + > > > > > + print("sources files:", len(source_files)) > > > > > + > > > > > + if args.range: > > > > > + # extract changed files from git > > > > > + r = subprocess.run( > > > > > + [ "git", "diff", "--name-only", args.range[0], args.range[1] ], > > > > > + stdin=subprocess.DEVNULL, > > > > > + stdout=subprocess.PIPE, > > > > > + check=True, > > > > > + ) > > > > > + changed_files = { > > > > > + os.path.realpath(x.strip()) > > > > > + for x in r.stdout.decode("utf-8").splitlines() > > > > > + } > > > > > + > > > > > + print(f"changed files: ({len(changed_files)})") > > > > > + for x in changed_files: > > > > > + print(f"\t{x}") > > > > > + > > > > > + # extract changed dependencies > > > > > + r = subprocess.run( > > > > > + [ "ninja", "-t", "deps" ], > > > > > + cwd=args.build_dir, > > > > > + stdin=subprocess.DEVNULL, > > > > > + stdout=subprocess.PIPE, > > > > > + check=True, > > > > > + ) > > > > > + > > > > > + dependency_groups = [] > > > > > + dependency_group = set() > > > > > + for line in r.stdout.decode("utf-8").splitlines(): > > > > > + if not line.startswith(" "): > > > > > + if bool(changed_files & dependency_group): > > > > > + dependency_groups.append(dependency_group) > > > > > + > > > > > + dependency_group = set() > > > > > + else: > > > > > + dependency_group.add( > > > > > + os.path.abspath(os.path.join(args.build_dir, line.strip())) > > > > > + ) > > > > > + > > > > > + # select source files that are in changed dependency groups > > > > > + files_to_check = set() > > > > > + for f in source_files: > > > > > + if any(f in dg for dg in dependency_groups): > > > > > + files_to_check.add(f) > > > > > + else: > > > > > + files_to_check = source_files > > > > > + > > > > > + print(f"running clang-tidy on the following files: ({len(files_to_check)})") > > > > > + for f in files_to_check: > > > > > + print(f"\t{f}") > > > > > + > > > > > + if files_to_check: > > > > > + argv = [ > > > > > + "run-clang-tidy", > > > > > + "-p", args.build_dir > > > > > + ] + args.args > > > > > + > > > > > + print("executing:", argv, flush=True) > > > > > + os.execvp(argv[0], argv + list(files_to_check)) > > > > > + > > > > > +if __name__ == "__main__": > > > > > + main() > > > > > diff --git a/.gitlab-ci/lint-clang-tidy.sh b/.gitlab-ci/lint-clang-tidy.sh > > > > > new file mode 100755 > > > > > index 0000000..4c88795 > > > > > --- /dev/null > > > > > +++ b/.gitlab-ci/lint-clang-tidy.sh > > > > > @@ -0,0 +1,22 @@ > > > > > +#!/bin/bash > > > > > + > > > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > > > +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy > > > > > +# > > > > > +# Lint with clang-tidy > > > > > + > > > > > +set -e > > > > > + > > > > > +source "$(dirname "$0")/lib.sh" > > > > > + > > > > > +clang_tidy() { > > > > > + local base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA) > > > > > + > > > > > + $(dirname "$0")/lint-clang-tidy.py \ > > > > > + --build-dir build/ \ > > > > > + --range $base $CI_COMMIT_SHA \ > > > > > + -- \ > > > > > + -j $BUILD_CONCURRENT_JOBS > > > > > +} > > > > > + > > > > > +run clang_tidy > > > > > diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh > > > > > index bb00895..ac37faf 100755 > > > > > --- a/.gitlab-ci/setup-container.sh > > > > > +++ b/.gitlab-ci/setup-container.sh > > > > > @@ -132,7 +132,7 @@ case $FDO_DISTRIBUTION_VERSION in > > > > > # Packages required to deploy the documentation > > > > > PKGS_LIBCAMERA_RUNTIME+=( openssh-client rsync ) > > > > > # Tools requires by the lint jobs. > > > > > - PKGS_LIBCAMERA_RUNTIME+=( clang-format jq python3-autopep8 reuse shellcheck ) > > > > > + PKGS_LIBCAMERA_RUNTIME+=( clang-format clang-tidy jq python3-autopep8 reuse shellcheck ) > > > > > # libclang-rt-dev for the clang ASan runtime. > > > > > PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-19-dev ) > > > > > # For lc-compliance in the `test-lc-compliance:virtual` job > > > > > diff --git a/gitlab-ci.yml b/gitlab-ci.yml > > > > > index 840eb74..3e8fe99 100644 > > > > > --- a/gitlab-ci.yml > > > > > +++ b/gitlab-ci.yml > > > > > @@ -74,7 +74,7 @@ include: > > > > > .libcamera-ci.debian:13: > > > > > variables: > > > > > FDO_DISTRIBUTION_VERSION: 'trixie' > > > > > - FDO_DISTRIBUTION_TAG: '2026-06-13.0' > > > > > + FDO_DISTRIBUTION_TAG: '2026-10-14.0' > > > > > > > > > > .container-debian: > > > > > extends: > > > > > @@ -198,9 +198,11 @@ container-arch: > > > > > paths: > > > > > - build/meson-logs/ > > > > > - build/build.ninja > > > > > + - $EXTRA_ARTIFACT > > > > > variables: > > > > > BUILD_TYPE: debug > > > > > MESON_OPTIONS: ${MESON_ALL_OPTIONS} > > > > > + EXTRA_ARTIFACT: '' > > > > > > > > > > build-full:debian:12: > > > > > extends: > > > > > @@ -255,6 +257,13 @@ build-full:debian:13: > > > > > ${MESON_ALL_OPTIONS} > > > > > -D b_sanitize=none > > > > > -D cpp_debugstl=false > > > > > + - ARCH: amd64 > > > > > + CC: clang > > > > > + CXX: clang++ > > > > > + MESON_OPTIONS: >- > > > > > + ${MESON_ALL_OPTIONS} > > > > > + ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > > > + EXTRA_ARTIFACT: build > > > > > - ARCH: amd64 > > > > > CC: clang > > > > > CXX: clang++ > > > > > @@ -262,7 +271,6 @@ build-full:debian:13: > > > > > # `cpp_debugstl=false` needed approximately between meson 1.4.0 and 1.9.3 > > > > > # with clang 18 or later due to https://github.com/mesonbuild/meson/issues/13812 > > > > > - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBCPP_OPTIONS} -D qcam=disabled -D cpp_debugstl=false > > > > > - - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > > > - ARCH: amd64 > > > > > BUILD_TYPE: release > > > > > CC: clang > > > > > @@ -449,6 +457,28 @@ build-docs: > > > > > # Lint stage - Run checkstyle.py and check merge suitability > > > > > # ------------------------------------------------------------------------------ > > > > > > > > > > +clang-tidy: > > > > > + extends: > > > > > + - .fdo.distribution-image@debian > > > > > + - .history-jobs > > > > > + - .libcamera-ci.debian:13 > > > > > + - .libcamera-ci.scripts > > > > > + stage: lint > > > > > + needs: > > > > > + - job: build-full:debian:13 > > > > > + artifacts: true > > > > > + parallel: > > > > > + matrix: > > > > > + - ARCH: amd64 > > > > > + CC: clang > > > > > + CXX: clang++ > > > > > + MESON_OPTIONS: >- > > > > > + ${MESON_ALL_OPTIONS} > > > > > + ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > > > + EXTRA_ARTIFACT: build > > > > > + script: > > > > > + $CI_PROJECT_DIR/.gitlab-ci/lint-clang-tidy.sh > > > > > + > > > > > license: > > > > > extends: > > > > > - .fdo.distribution-image@debian > > > > > -- > > > > > 2.56.0 > > > > > > > > >
Hi, On Fri, 16 Oct 2026 at 09:52, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > On Fri, Oct 16, 2026 at 10:32:38AM +0200, Barnabás Pőcze wrote: > > 2026. 10. 16. 10:22 keltezéssel, Jacopo Mondi írta: > > > On Fri, Oct 16, 2026 at 10:13:41AM +0200, Barnabás Pőcze wrote: > > > > 2026. 10. 16. 9:57 keltezéssel, Jacopo Mondi írta: > > > > > Hi Barnabas > > > > > > > > > > On Thu, Oct 15, 2026 at 12:10:12PM +0200, Barnabás Pőcze wrote: > > > > > > Add a job that runs clang-tidy, based on a clang build to avoid issues > > > > > > arising from unsupported compiler options between gcc and clang. > > > > > > > > > > > > Unfortunately running clang-tidy on each source file for every change > > > > > > is too time consuming, so only the affected files should be checked. > > > > > > However, knowing which files are affected does not appear to be > > > > > > particularly simple. The biggest challenge is posed by generated header > > > > > > files, changing any of the inputs should trigger a clang-tidy run on > > > > > > every source file that includes it. > > > > > > > > > > > > To handle that, both the "static" inputs of a target, and the compiler > > > > > > generated depfiles are needed. Unfortunately, ninja does not seem to > > > > > > have a subcommand, that when given a list of "changed" files, would > > > > > > print the list of targets that need to be potentially rebuilt. > > > > > > > > > > > > So this change only implements the handling of the compiler generated > > > > > > depfiles using the `deps` ninja subtool. This will miss generated > > > > > > headers, but an implemention handling that seems too complex at the > > > > > > moment. In practice, this means, for example, that changing e.g. only > > > > > > `include/libcamera/formats.h.in` will not trigger a clang-tidy run > > > > > > on any source file that includes it. > > > > > > > > > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > > > > > > > Can we first discuss the usefulness of reports like: > > > > > https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160#L246 > > > > > > > > > > it seems clear to me that clang-tidy generates a lot of false > > > > > positive, at least on our current code base. > > > > > > > > > > How do we want to treat it ? Optional suggestions for > > > > > developers/reviewers to skim through ? Mandatory to address all > > > > > clang-tidy reported issues ? (unlikely, right?) > > > > > > > > I have forgotten to mention that indeed. My thought was to use the > > > > `WarningsAsErrors` option in the `.clang-tidy` configuration file to > > > > define a set of a checks whose violation will be regarded as an error > > > > and will fail the ci pipeline. > > > > > > From this list ? > > > https://clang.llvm.org/extra/clang-tidy/checks/list.html > > > > > > Should we start by deciding what constitute a CI error among these ? > > > > Correct, I have already started have a list, but first wanted to see if > > clang-tidy in general seems acceptable. > > > > > > > > > > Alternatively, the clang-tidy test should be treated as informative only ? > > > > Possibly, but I'm not a big believer in those. At the moment I think a good > > approach is to consider certain checks "important" and consider any violation > > an error, but leave most reasonable checks enabled, because those can still > > be useful if one is writing a new piece of code and sees those warnings via > > clangd or such. > > Do you think it would be possibile to integrate clang-tidy in > checkstyle so that it can also be run locally even before submitting a > patch for CI ? > This is a slight bit tangential, but I've been using the pre-commit tool [1] on other projects to handle things like this + bits of functionality of the checkstyle script. Perhaps it's worth looking into for the libcamera project? Naush [1] https://pre-commit.com/ > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > I am planning to enable more checks in the libcamera repository, > > > > > > but given the limitations outlined above, I wanted to see if it > > > > > > is acceptable in the first place. > > > > > > > > > > > > Example job: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160 > > > > > > --- > > > > > > .gitlab-ci/lint-clang-tidy.py | 112 ++++++++++++++++++++++++++++++++++ > > > > > > .gitlab-ci/lint-clang-tidy.sh | 22 +++++++ > > > > > > .gitlab-ci/setup-container.sh | 2 +- > > > > > > gitlab-ci.yml | 34 ++++++++++- > > > > > > 4 files changed, 167 insertions(+), 3 deletions(-) > > > > > > create mode 100755 .gitlab-ci/lint-clang-tidy.py > > > > > > create mode 100755 .gitlab-ci/lint-clang-tidy.sh > > > > > > > > > > > > diff --git a/.gitlab-ci/lint-clang-tidy.py b/.gitlab-ci/lint-clang-tidy.py > > > > > > new file mode 100755 > > > > > > index 0000000..aae28db > > > > > > --- /dev/null > > > > > > +++ b/.gitlab-ci/lint-clang-tidy.py > > > > > > @@ -0,0 +1,112 @@ > > > > > > +#!/usr/bin/env python3 > > > > > > + > > > > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > > > > +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy > > > > > > + > > > > > > +import argparse > > > > > > +import fnmatch > > > > > > +import json > > > > > > +import os > > > > > > +import os.path > > > > > > +import subprocess > > > > > > + > > > > > > +def main(): > > > > > > + parser = argparse.ArgumentParser() > > > > > > + parser.add_argument("--build-dir", type=str, required=True) > > > > > > + parser.add_argument("--range", nargs=2, required=False) > > > > > > + parser.add_argument("args", nargs="*", default=[]) > > > > > > + > > > > > > + args = parser.parse_args() > > > > > > + > > > > > > + try: > > > > > > + exclude_patterns = [] > > > > > > + with open(".clang-tidy-exclude", "r") as f: > > > > > > + for line in f: > > > > > > + line = line.strip() > > > > > > + if not line or line.startswith("#"): > > > > > > + continue > > > > > > + > > > > > > + exclude_patterns.append(line) > > > > > > + except FileNotFoundError: > > > > > > + pass > > > > > > + > > > > > > + # extract source files from the compilation database > > > > > > + r = subprocess.run( > > > > > > + [ "ninja", "-t", "compdb" ], > > > > > > + cwd=args.build_dir, > > > > > > + stdin=subprocess.DEVNULL, > > > > > > + stdout=subprocess.PIPE, > > > > > > + check=True, > > > > > > + ) > > > > > > + > > > > > > + source_files = set() > > > > > > + > > > > > > + for x in json.loads(r.stdout): > > > > > > + f = os.path.abspath(os.path.join(x["directory"], x["file"])) > > > > > > + if not any(fnmatch.fnmatch(f, p) for p in exclude_patterns): > > > > > > + source_files.add(f) > > > > > > + > > > > > > + print("sources files:", len(source_files)) > > > > > > + > > > > > > + if args.range: > > > > > > + # extract changed files from git > > > > > > + r = subprocess.run( > > > > > > + [ "git", "diff", "--name-only", args.range[0], args.range[1] ], > > > > > > + stdin=subprocess.DEVNULL, > > > > > > + stdout=subprocess.PIPE, > > > > > > + check=True, > > > > > > + ) > > > > > > + changed_files = { > > > > > > + os.path.realpath(x.strip()) > > > > > > + for x in r.stdout.decode("utf-8").splitlines() > > > > > > + } > > > > > > + > > > > > > + print(f"changed files: ({len(changed_files)})") > > > > > > + for x in changed_files: > > > > > > + print(f"\t{x}") > > > > > > + > > > > > > + # extract changed dependencies > > > > > > + r = subprocess.run( > > > > > > + [ "ninja", "-t", "deps" ], > > > > > > + cwd=args.build_dir, > > > > > > + stdin=subprocess.DEVNULL, > > > > > > + stdout=subprocess.PIPE, > > > > > > + check=True, > > > > > > + ) > > > > > > + > > > > > > + dependency_groups = [] > > > > > > + dependency_group = set() > > > > > > + for line in r.stdout.decode("utf-8").splitlines(): > > > > > > + if not line.startswith(" "): > > > > > > + if bool(changed_files & dependency_group): > > > > > > + dependency_groups.append(dependency_group) > > > > > > + > > > > > > + dependency_group = set() > > > > > > + else: > > > > > > + dependency_group.add( > > > > > > + os.path.abspath(os.path.join(args.build_dir, line.strip())) > > > > > > + ) > > > > > > + > > > > > > + # select source files that are in changed dependency groups > > > > > > + files_to_check = set() > > > > > > + for f in source_files: > > > > > > + if any(f in dg for dg in dependency_groups): > > > > > > + files_to_check.add(f) > > > > > > + else: > > > > > > + files_to_check = source_files > > > > > > + > > > > > > + print(f"running clang-tidy on the following files: ({len(files_to_check)})") > > > > > > + for f in files_to_check: > > > > > > + print(f"\t{f}") > > > > > > + > > > > > > + if files_to_check: > > > > > > + argv = [ > > > > > > + "run-clang-tidy", > > > > > > + "-p", args.build_dir > > > > > > + ] + args.args > > > > > > + > > > > > > + print("executing:", argv, flush=True) > > > > > > + os.execvp(argv[0], argv + list(files_to_check)) > > > > > > + > > > > > > +if __name__ == "__main__": > > > > > > + main() > > > > > > diff --git a/.gitlab-ci/lint-clang-tidy.sh b/.gitlab-ci/lint-clang-tidy.sh > > > > > > new file mode 100755 > > > > > > index 0000000..4c88795 > > > > > > --- /dev/null > > > > > > +++ b/.gitlab-ci/lint-clang-tidy.sh > > > > > > @@ -0,0 +1,22 @@ > > > > > > +#!/bin/bash > > > > > > + > > > > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > > > > +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy > > > > > > +# > > > > > > +# Lint with clang-tidy > > > > > > + > > > > > > +set -e > > > > > > + > > > > > > +source "$(dirname "$0")/lib.sh" > > > > > > + > > > > > > +clang_tidy() { > > > > > > + local base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA) > > > > > > + > > > > > > + $(dirname "$0")/lint-clang-tidy.py \ > > > > > > + --build-dir build/ \ > > > > > > + --range $base $CI_COMMIT_SHA \ > > > > > > + -- \ > > > > > > + -j $BUILD_CONCURRENT_JOBS > > > > > > +} > > > > > > + > > > > > > +run clang_tidy > > > > > > diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh > > > > > > index bb00895..ac37faf 100755 > > > > > > --- a/.gitlab-ci/setup-container.sh > > > > > > +++ b/.gitlab-ci/setup-container.sh > > > > > > @@ -132,7 +132,7 @@ case $FDO_DISTRIBUTION_VERSION in > > > > > > # Packages required to deploy the documentation > > > > > > PKGS_LIBCAMERA_RUNTIME+=( openssh-client rsync ) > > > > > > # Tools requires by the lint jobs. > > > > > > - PKGS_LIBCAMERA_RUNTIME+=( clang-format jq python3-autopep8 reuse shellcheck ) > > > > > > + PKGS_LIBCAMERA_RUNTIME+=( clang-format clang-tidy jq python3-autopep8 reuse shellcheck ) > > > > > > # libclang-rt-dev for the clang ASan runtime. > > > > > > PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-19-dev ) > > > > > > # For lc-compliance in the `test-lc-compliance:virtual` job > > > > > > diff --git a/gitlab-ci.yml b/gitlab-ci.yml > > > > > > index 840eb74..3e8fe99 100644 > > > > > > --- a/gitlab-ci.yml > > > > > > +++ b/gitlab-ci.yml > > > > > > @@ -74,7 +74,7 @@ include: > > > > > > .libcamera-ci.debian:13: > > > > > > variables: > > > > > > FDO_DISTRIBUTION_VERSION: 'trixie' > > > > > > - FDO_DISTRIBUTION_TAG: '2026-06-13.0' > > > > > > + FDO_DISTRIBUTION_TAG: '2026-10-14.0' > > > > > > > > > > > > .container-debian: > > > > > > extends: > > > > > > @@ -198,9 +198,11 @@ container-arch: > > > > > > paths: > > > > > > - build/meson-logs/ > > > > > > - build/build.ninja > > > > > > + - $EXTRA_ARTIFACT > > > > > > variables: > > > > > > BUILD_TYPE: debug > > > > > > MESON_OPTIONS: ${MESON_ALL_OPTIONS} > > > > > > + EXTRA_ARTIFACT: '' > > > > > > > > > > > > build-full:debian:12: > > > > > > extends: > > > > > > @@ -255,6 +257,13 @@ build-full:debian:13: > > > > > > ${MESON_ALL_OPTIONS} > > > > > > -D b_sanitize=none > > > > > > -D cpp_debugstl=false > > > > > > + - ARCH: amd64 > > > > > > + CC: clang > > > > > > + CXX: clang++ > > > > > > + MESON_OPTIONS: >- > > > > > > + ${MESON_ALL_OPTIONS} > > > > > > + ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > > > > + EXTRA_ARTIFACT: build > > > > > > - ARCH: amd64 > > > > > > CC: clang > > > > > > CXX: clang++ > > > > > > @@ -262,7 +271,6 @@ build-full:debian:13: > > > > > > # `cpp_debugstl=false` needed approximately between meson 1.4.0 and 1.9.3 > > > > > > # with clang 18 or later due to https://github.com/mesonbuild/meson/issues/13812 > > > > > > - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBCPP_OPTIONS} -D qcam=disabled -D cpp_debugstl=false > > > > > > - - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > > > > - ARCH: amd64 > > > > > > BUILD_TYPE: release > > > > > > CC: clang > > > > > > @@ -449,6 +457,28 @@ build-docs: > > > > > > # Lint stage - Run checkstyle.py and check merge suitability > > > > > > # ------------------------------------------------------------------------------ > > > > > > > > > > > > +clang-tidy: > > > > > > + extends: > > > > > > + - .fdo.distribution-image@debian > > > > > > + - .history-jobs > > > > > > + - .libcamera-ci.debian:13 > > > > > > + - .libcamera-ci.scripts > > > > > > + stage: lint > > > > > > + needs: > > > > > > + - job: build-full:debian:13 > > > > > > + artifacts: true > > > > > > + parallel: > > > > > > + matrix: > > > > > > + - ARCH: amd64 > > > > > > + CC: clang > > > > > > + CXX: clang++ > > > > > > + MESON_OPTIONS: >- > > > > > > + ${MESON_ALL_OPTIONS} > > > > > > + ${MESON_CLANG_LIBSTDCPP_OPTIONS} > > > > > > + EXTRA_ARTIFACT: build > > > > > > + script: > > > > > > + $CI_PROJECT_DIR/.gitlab-ci/lint-clang-tidy.sh > > > > > > + > > > > > > license: > > > > > > extends: > > > > > > - .fdo.distribution-image@debian > > > > > > -- > > > > > > 2.56.0 > > > > > > > > > > > >
2026. 10. 16. 10:52 keltezéssel, Jacopo Mondi írta: > On Fri, Oct 16, 2026 at 10:32:38AM +0200, Barnabás Pőcze wrote: >> 2026. 10. 16. 10:22 keltezéssel, Jacopo Mondi írta: >>> On Fri, Oct 16, 2026 at 10:13:41AM +0200, Barnabás Pőcze wrote: >>>> 2026. 10. 16. 9:57 keltezéssel, Jacopo Mondi írta: >>>>> Hi Barnabas >>>>> >>>>> On Thu, Oct 15, 2026 at 12:10:12PM +0200, Barnabás Pőcze wrote: >>>>>> Add a job that runs clang-tidy, based on a clang build to avoid issues >>>>>> arising from unsupported compiler options between gcc and clang. >>>>>> >>>>>> Unfortunately running clang-tidy on each source file for every change >>>>>> is too time consuming, so only the affected files should be checked. >>>>>> However, knowing which files are affected does not appear to be >>>>>> particularly simple. The biggest challenge is posed by generated header >>>>>> files, changing any of the inputs should trigger a clang-tidy run on >>>>>> every source file that includes it. >>>>>> >>>>>> To handle that, both the "static" inputs of a target, and the compiler >>>>>> generated depfiles are needed. Unfortunately, ninja does not seem to >>>>>> have a subcommand, that when given a list of "changed" files, would >>>>>> print the list of targets that need to be potentially rebuilt. >>>>>> >>>>>> So this change only implements the handling of the compiler generated >>>>>> depfiles using the `deps` ninja subtool. This will miss generated >>>>>> headers, but an implemention handling that seems too complex at the >>>>>> moment. In practice, this means, for example, that changing e.g. only >>>>>> `include/libcamera/formats.h.in` will not trigger a clang-tidy run >>>>>> on any source file that includes it. >>>>>> >>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>>> >>>>> Can we first discuss the usefulness of reports like: >>>>> https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160#L246 >>>>> >>>>> it seems clear to me that clang-tidy generates a lot of false >>>>> positive, at least on our current code base. >>>>> >>>>> How do we want to treat it ? Optional suggestions for >>>>> developers/reviewers to skim through ? Mandatory to address all >>>>> clang-tidy reported issues ? (unlikely, right?) >>>> >>>> I have forgotten to mention that indeed. My thought was to use the >>>> `WarningsAsErrors` option in the `.clang-tidy` configuration file to >>>> define a set of a checks whose violation will be regarded as an error >>>> and will fail the ci pipeline. >>> >>> From this list ? >>> https://clang.llvm.org/extra/clang-tidy/checks/list.html >>> >>> Should we start by deciding what constitute a CI error among these ? >> >> Correct, I have already started have a list, but first wanted to see if >> clang-tidy in general seems acceptable. >> >> >>> >>> Alternatively, the clang-tidy test should be treated as informative only ? >> >> Possibly, but I'm not a big believer in those. At the moment I think a good >> approach is to consider certain checks "important" and consider any violation >> an error, but leave most reasonable checks enabled, because those can still >> be useful if one is writing a new piece of code and sees those warnings via >> clangd or such. > > Do you think it would be possibile to integrate clang-tidy in > checkstyle so that it can also be run locally even before submitting a > patch for CI ? I'm certain it is possible, but I think it will bring its own complications wrt. filtering the output. > >> >> >>> >>>> >>>> >>>>> >>>>>> --- >>>>>> I am planning to enable more checks in the libcamera repository, >>>>>> but given the limitations outlined above, I wanted to see if it >>>>>> is acceptable in the first place. >>>>>> >>>>>> Example job: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160 >>>>>> --- >>>>>> .gitlab-ci/lint-clang-tidy.py | 112 ++++++++++++++++++++++++++++++++++ >>>>>> .gitlab-ci/lint-clang-tidy.sh | 22 +++++++ >>>>>> .gitlab-ci/setup-container.sh | 2 +- >>>>>> gitlab-ci.yml | 34 ++++++++++- >>>>>> 4 files changed, 167 insertions(+), 3 deletions(-) >>>>>> create mode 100755 .gitlab-ci/lint-clang-tidy.py >>>>>> create mode 100755 .gitlab-ci/lint-clang-tidy.sh >>>>>> >>>>>> diff --git a/.gitlab-ci/lint-clang-tidy.py b/.gitlab-ci/lint-clang-tidy.py >>>>>> new file mode 100755 >>>>>> index 0000000..aae28db >>>>>> --- /dev/null >>>>>> +++ b/.gitlab-ci/lint-clang-tidy.py >>>>>> @@ -0,0 +1,112 @@ >>>>>> +#!/usr/bin/env python3 >>>>>> + >>>>>> +# SPDX-License-Identifier: GPL-2.0-or-later >>>>>> +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy >>>>>> + >>>>>> +import argparse >>>>>> +import fnmatch >>>>>> +import json >>>>>> +import os >>>>>> +import os.path >>>>>> +import subprocess >>>>>> + >>>>>> +def main(): >>>>>> + parser = argparse.ArgumentParser() >>>>>> + parser.add_argument("--build-dir", type=str, required=True) >>>>>> + parser.add_argument("--range", nargs=2, required=False) >>>>>> + parser.add_argument("args", nargs="*", default=[]) >>>>>> + >>>>>> + args = parser.parse_args() >>>>>> + >>>>>> + try: >>>>>> + exclude_patterns = [] >>>>>> + with open(".clang-tidy-exclude", "r") as f: >>>>>> + for line in f: >>>>>> + line = line.strip() >>>>>> + if not line or line.startswith("#"): >>>>>> + continue >>>>>> + >>>>>> + exclude_patterns.append(line) >>>>>> + except FileNotFoundError: >>>>>> + pass >>>>>> + >>>>>> + # extract source files from the compilation database >>>>>> + r = subprocess.run( >>>>>> + [ "ninja", "-t", "compdb" ], >>>>>> + cwd=args.build_dir, >>>>>> + stdin=subprocess.DEVNULL, >>>>>> + stdout=subprocess.PIPE, >>>>>> + check=True, >>>>>> + ) >>>>>> + >>>>>> + source_files = set() >>>>>> + >>>>>> + for x in json.loads(r.stdout): >>>>>> + f = os.path.abspath(os.path.join(x["directory"], x["file"])) >>>>>> + if not any(fnmatch.fnmatch(f, p) for p in exclude_patterns): >>>>>> + source_files.add(f) >>>>>> + >>>>>> + print("sources files:", len(source_files)) >>>>>> + >>>>>> + if args.range: >>>>>> + # extract changed files from git >>>>>> + r = subprocess.run( >>>>>> + [ "git", "diff", "--name-only", args.range[0], args.range[1] ], >>>>>> + stdin=subprocess.DEVNULL, >>>>>> + stdout=subprocess.PIPE, >>>>>> + check=True, >>>>>> + ) >>>>>> + changed_files = { >>>>>> + os.path.realpath(x.strip()) >>>>>> + for x in r.stdout.decode("utf-8").splitlines() >>>>>> + } >>>>>> + >>>>>> + print(f"changed files: ({len(changed_files)})") >>>>>> + for x in changed_files: >>>>>> + print(f"\t{x}") >>>>>> + >>>>>> + # extract changed dependencies >>>>>> + r = subprocess.run( >>>>>> + [ "ninja", "-t", "deps" ], >>>>>> + cwd=args.build_dir, >>>>>> + stdin=subprocess.DEVNULL, >>>>>> + stdout=subprocess.PIPE, >>>>>> + check=True, >>>>>> + ) >>>>>> + >>>>>> + dependency_groups = [] >>>>>> + dependency_group = set() >>>>>> + for line in r.stdout.decode("utf-8").splitlines(): >>>>>> + if not line.startswith(" "): >>>>>> + if bool(changed_files & dependency_group): >>>>>> + dependency_groups.append(dependency_group) >>>>>> + >>>>>> + dependency_group = set() >>>>>> + else: >>>>>> + dependency_group.add( >>>>>> + os.path.abspath(os.path.join(args.build_dir, line.strip())) >>>>>> + ) >>>>>> + >>>>>> + # select source files that are in changed dependency groups >>>>>> + files_to_check = set() >>>>>> + for f in source_files: >>>>>> + if any(f in dg for dg in dependency_groups): >>>>>> + files_to_check.add(f) >>>>>> + else: >>>>>> + files_to_check = source_files >>>>>> + >>>>>> + print(f"running clang-tidy on the following files: ({len(files_to_check)})") >>>>>> + for f in files_to_check: >>>>>> + print(f"\t{f}") >>>>>> + >>>>>> + if files_to_check: >>>>>> + argv = [ >>>>>> + "run-clang-tidy", >>>>>> + "-p", args.build_dir >>>>>> + ] + args.args >>>>>> + >>>>>> + print("executing:", argv, flush=True) >>>>>> + os.execvp(argv[0], argv + list(files_to_check)) >>>>>> + >>>>>> +if __name__ == "__main__": >>>>>> + main() >>>>>> diff --git a/.gitlab-ci/lint-clang-tidy.sh b/.gitlab-ci/lint-clang-tidy.sh >>>>>> new file mode 100755 >>>>>> index 0000000..4c88795 >>>>>> --- /dev/null >>>>>> +++ b/.gitlab-ci/lint-clang-tidy.sh >>>>>> @@ -0,0 +1,22 @@ >>>>>> +#!/bin/bash >>>>>> + >>>>>> +# SPDX-License-Identifier: GPL-2.0-or-later >>>>>> +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy >>>>>> +# >>>>>> +# Lint with clang-tidy >>>>>> + >>>>>> +set -e >>>>>> + >>>>>> +source "$(dirname "$0")/lib.sh" >>>>>> + >>>>>> +clang_tidy() { >>>>>> + local base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA) >>>>>> + >>>>>> + $(dirname "$0")/lint-clang-tidy.py \ >>>>>> + --build-dir build/ \ >>>>>> + --range $base $CI_COMMIT_SHA \ >>>>>> + -- \ >>>>>> + -j $BUILD_CONCURRENT_JOBS >>>>>> +} >>>>>> + >>>>>> +run clang_tidy >>>>>> diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh >>>>>> index bb00895..ac37faf 100755 >>>>>> --- a/.gitlab-ci/setup-container.sh >>>>>> +++ b/.gitlab-ci/setup-container.sh >>>>>> @@ -132,7 +132,7 @@ case $FDO_DISTRIBUTION_VERSION in >>>>>> # Packages required to deploy the documentation >>>>>> PKGS_LIBCAMERA_RUNTIME+=( openssh-client rsync ) >>>>>> # Tools requires by the lint jobs. >>>>>> - PKGS_LIBCAMERA_RUNTIME+=( clang-format jq python3-autopep8 reuse shellcheck ) >>>>>> + PKGS_LIBCAMERA_RUNTIME+=( clang-format clang-tidy jq python3-autopep8 reuse shellcheck ) >>>>>> # libclang-rt-dev for the clang ASan runtime. >>>>>> PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-19-dev ) >>>>>> # For lc-compliance in the `test-lc-compliance:virtual` job >>>>>> diff --git a/gitlab-ci.yml b/gitlab-ci.yml >>>>>> index 840eb74..3e8fe99 100644 >>>>>> --- a/gitlab-ci.yml >>>>>> +++ b/gitlab-ci.yml >>>>>> @@ -74,7 +74,7 @@ include: >>>>>> .libcamera-ci.debian:13: >>>>>> variables: >>>>>> FDO_DISTRIBUTION_VERSION: 'trixie' >>>>>> - FDO_DISTRIBUTION_TAG: '2026-06-13.0' >>>>>> + FDO_DISTRIBUTION_TAG: '2026-10-14.0' >>>>>> >>>>>> .container-debian: >>>>>> extends: >>>>>> @@ -198,9 +198,11 @@ container-arch: >>>>>> paths: >>>>>> - build/meson-logs/ >>>>>> - build/build.ninja >>>>>> + - $EXTRA_ARTIFACT >>>>>> variables: >>>>>> BUILD_TYPE: debug >>>>>> MESON_OPTIONS: ${MESON_ALL_OPTIONS} >>>>>> + EXTRA_ARTIFACT: '' >>>>>> >>>>>> build-full:debian:12: >>>>>> extends: >>>>>> @@ -255,6 +257,13 @@ build-full:debian:13: >>>>>> ${MESON_ALL_OPTIONS} >>>>>> -D b_sanitize=none >>>>>> -D cpp_debugstl=false >>>>>> + - ARCH: amd64 >>>>>> + CC: clang >>>>>> + CXX: clang++ >>>>>> + MESON_OPTIONS: >- >>>>>> + ${MESON_ALL_OPTIONS} >>>>>> + ${MESON_CLANG_LIBSTDCPP_OPTIONS} >>>>>> + EXTRA_ARTIFACT: build >>>>>> - ARCH: amd64 >>>>>> CC: clang >>>>>> CXX: clang++ >>>>>> @@ -262,7 +271,6 @@ build-full:debian:13: >>>>>> # `cpp_debugstl=false` needed approximately between meson 1.4.0 and 1.9.3 >>>>>> # with clang 18 or later due to https://github.com/mesonbuild/meson/issues/13812 >>>>>> - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBCPP_OPTIONS} -D qcam=disabled -D cpp_debugstl=false >>>>>> - - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBSTDCPP_OPTIONS} >>>>>> - ARCH: amd64 >>>>>> BUILD_TYPE: release >>>>>> CC: clang >>>>>> @@ -449,6 +457,28 @@ build-docs: >>>>>> # Lint stage - Run checkstyle.py and check merge suitability >>>>>> # ------------------------------------------------------------------------------ >>>>>> >>>>>> +clang-tidy: >>>>>> + extends: >>>>>> + - .fdo.distribution-image@debian >>>>>> + - .history-jobs >>>>>> + - .libcamera-ci.debian:13 >>>>>> + - .libcamera-ci.scripts >>>>>> + stage: lint >>>>>> + needs: >>>>>> + - job: build-full:debian:13 >>>>>> + artifacts: true >>>>>> + parallel: >>>>>> + matrix: >>>>>> + - ARCH: amd64 >>>>>> + CC: clang >>>>>> + CXX: clang++ >>>>>> + MESON_OPTIONS: >- >>>>>> + ${MESON_ALL_OPTIONS} >>>>>> + ${MESON_CLANG_LIBSTDCPP_OPTIONS} >>>>>> + EXTRA_ARTIFACT: build >>>>>> + script: >>>>>> + $CI_PROJECT_DIR/.gitlab-ci/lint-clang-tidy.sh >>>>>> + >>>>>> license: >>>>>> extends: >>>>>> - .fdo.distribution-image@debian >>>>>> -- >>>>>> 2.56.0 >>>>>> >>>> >>
diff --git a/.gitlab-ci/lint-clang-tidy.py b/.gitlab-ci/lint-clang-tidy.py new file mode 100755 index 0000000..aae28db --- /dev/null +++ b/.gitlab-ci/lint-clang-tidy.py @@ -0,0 +1,112 @@ +#!/usr/bin/env python3 + +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy + +import argparse +import fnmatch +import json +import os +import os.path +import subprocess + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--build-dir", type=str, required=True) + parser.add_argument("--range", nargs=2, required=False) + parser.add_argument("args", nargs="*", default=[]) + + args = parser.parse_args() + + try: + exclude_patterns = [] + with open(".clang-tidy-exclude", "r") as f: + for line in f: + line = line.strip() + if not line or line.startswith("#"): + continue + + exclude_patterns.append(line) + except FileNotFoundError: + pass + + # extract source files from the compilation database + r = subprocess.run( + [ "ninja", "-t", "compdb" ], + cwd=args.build_dir, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + check=True, + ) + + source_files = set() + + for x in json.loads(r.stdout): + f = os.path.abspath(os.path.join(x["directory"], x["file"])) + if not any(fnmatch.fnmatch(f, p) for p in exclude_patterns): + source_files.add(f) + + print("sources files:", len(source_files)) + + if args.range: + # extract changed files from git + r = subprocess.run( + [ "git", "diff", "--name-only", args.range[0], args.range[1] ], + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + check=True, + ) + changed_files = { + os.path.realpath(x.strip()) + for x in r.stdout.decode("utf-8").splitlines() + } + + print(f"changed files: ({len(changed_files)})") + for x in changed_files: + print(f"\t{x}") + + # extract changed dependencies + r = subprocess.run( + [ "ninja", "-t", "deps" ], + cwd=args.build_dir, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + check=True, + ) + + dependency_groups = [] + dependency_group = set() + for line in r.stdout.decode("utf-8").splitlines(): + if not line.startswith(" "): + if bool(changed_files & dependency_group): + dependency_groups.append(dependency_group) + + dependency_group = set() + else: + dependency_group.add( + os.path.abspath(os.path.join(args.build_dir, line.strip())) + ) + + # select source files that are in changed dependency groups + files_to_check = set() + for f in source_files: + if any(f in dg for dg in dependency_groups): + files_to_check.add(f) + else: + files_to_check = source_files + + print(f"running clang-tidy on the following files: ({len(files_to_check)})") + for f in files_to_check: + print(f"\t{f}") + + if files_to_check: + argv = [ + "run-clang-tidy", + "-p", args.build_dir + ] + args.args + + print("executing:", argv, flush=True) + os.execvp(argv[0], argv + list(files_to_check)) + +if __name__ == "__main__": + main() diff --git a/.gitlab-ci/lint-clang-tidy.sh b/.gitlab-ci/lint-clang-tidy.sh new file mode 100755 index 0000000..4c88795 --- /dev/null +++ b/.gitlab-ci/lint-clang-tidy.sh @@ -0,0 +1,22 @@ +#!/bin/bash + +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: © 2026 Ideas On Board Oy +# +# Lint with clang-tidy + +set -e + +source "$(dirname "$0")/lib.sh" + +clang_tidy() { + local base=$(find_base origin/$CI_DEFAULT_BRANCH $CI_COMMIT_SHA) + + $(dirname "$0")/lint-clang-tidy.py \ + --build-dir build/ \ + --range $base $CI_COMMIT_SHA \ + -- \ + -j $BUILD_CONCURRENT_JOBS +} + +run clang_tidy diff --git a/.gitlab-ci/setup-container.sh b/.gitlab-ci/setup-container.sh index bb00895..ac37faf 100755 --- a/.gitlab-ci/setup-container.sh +++ b/.gitlab-ci/setup-container.sh @@ -132,7 +132,7 @@ case $FDO_DISTRIBUTION_VERSION in # Packages required to deploy the documentation PKGS_LIBCAMERA_RUNTIME+=( openssh-client rsync ) # Tools requires by the lint jobs. - PKGS_LIBCAMERA_RUNTIME+=( clang-format jq python3-autopep8 reuse shellcheck ) + PKGS_LIBCAMERA_RUNTIME+=( clang-format clang-tidy jq python3-autopep8 reuse shellcheck ) # libclang-rt-dev for the clang ASan runtime. PKGS_LIBCAMERA_RUNTIME_MULTIARCH+=( libclang-rt-19-dev ) # For lc-compliance in the `test-lc-compliance:virtual` job diff --git a/gitlab-ci.yml b/gitlab-ci.yml index 840eb74..3e8fe99 100644 --- a/gitlab-ci.yml +++ b/gitlab-ci.yml @@ -74,7 +74,7 @@ include: .libcamera-ci.debian:13: variables: FDO_DISTRIBUTION_VERSION: 'trixie' - FDO_DISTRIBUTION_TAG: '2026-06-13.0' + FDO_DISTRIBUTION_TAG: '2026-10-14.0' .container-debian: extends: @@ -198,9 +198,11 @@ container-arch: paths: - build/meson-logs/ - build/build.ninja + - $EXTRA_ARTIFACT variables: BUILD_TYPE: debug MESON_OPTIONS: ${MESON_ALL_OPTIONS} + EXTRA_ARTIFACT: '' build-full:debian:12: extends: @@ -255,6 +257,13 @@ build-full:debian:13: ${MESON_ALL_OPTIONS} -D b_sanitize=none -D cpp_debugstl=false + - ARCH: amd64 + CC: clang + CXX: clang++ + MESON_OPTIONS: >- + ${MESON_ALL_OPTIONS} + ${MESON_CLANG_LIBSTDCPP_OPTIONS} + EXTRA_ARTIFACT: build - ARCH: amd64 CC: clang CXX: clang++ @@ -262,7 +271,6 @@ build-full:debian:13: # `cpp_debugstl=false` needed approximately between meson 1.4.0 and 1.9.3 # with clang 18 or later due to https://github.com/mesonbuild/meson/issues/13812 - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBCPP_OPTIONS} -D qcam=disabled -D cpp_debugstl=false - - ${MESON_ALL_OPTIONS} ${MESON_CLANG_LIBSTDCPP_OPTIONS} - ARCH: amd64 BUILD_TYPE: release CC: clang @@ -449,6 +457,28 @@ build-docs: # Lint stage - Run checkstyle.py and check merge suitability # ------------------------------------------------------------------------------ +clang-tidy: + extends: + - .fdo.distribution-image@debian + - .history-jobs + - .libcamera-ci.debian:13 + - .libcamera-ci.scripts + stage: lint + needs: + - job: build-full:debian:13 + artifacts: true + parallel: + matrix: + - ARCH: amd64 + CC: clang + CXX: clang++ + MESON_OPTIONS: >- + ${MESON_ALL_OPTIONS} + ${MESON_CLANG_LIBSTDCPP_OPTIONS} + EXTRA_ARTIFACT: build + script: + $CI_PROJECT_DIR/.gitlab-ci/lint-clang-tidy.sh + license: extends: - .fdo.distribution-image@debian
Add a job that runs clang-tidy, based on a clang build to avoid issues arising from unsupported compiler options between gcc and clang. Unfortunately running clang-tidy on each source file for every change is too time consuming, so only the affected files should be checked. However, knowing which files are affected does not appear to be particularly simple. The biggest challenge is posed by generated header files, changing any of the inputs should trigger a clang-tidy run on every source file that includes it. To handle that, both the "static" inputs of a target, and the compiler generated depfiles are needed. Unfortunately, ninja does not seem to have a subcommand, that when given a list of "changed" files, would print the list of targets that need to be potentially rebuilt. So this change only implements the handling of the compiler generated depfiles using the `deps` ninja subtool. This will miss generated headers, but an implemention handling that seems too complex at the moment. In practice, this means, for example, that changing e.g. only `include/libcamera/formats.h.in` will not trigger a clang-tidy run on any source file that includes it. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- I am planning to enable more checks in the libcamera repository, but given the limitations outlined above, I wanted to see if it is acceptable in the first place. Example job: https://gitlab.freedesktop.org/pobrn/libcamera/-/jobs/112824160 --- .gitlab-ci/lint-clang-tidy.py | 112 ++++++++++++++++++++++++++++++++++ .gitlab-ci/lint-clang-tidy.sh | 22 +++++++ .gitlab-ci/setup-container.sh | 2 +- gitlab-ci.yml | 34 ++++++++++- 4 files changed, 167 insertions(+), 3 deletions(-) create mode 100755 .gitlab-ci/lint-clang-tidy.py create mode 100755 .gitlab-ci/lint-clang-tidy.sh