Skip to content

Commit

Permalink
build: Use nix to manage dev tooling starting with clang-tidy (Vowpal…
Browse files Browse the repository at this point in the history
…Wabbit#4292)

* use nix for clang tidy and clang format

* test using nix for clang tidy

* check for only errors

* allow output from action

* dont lint tests with clang-tidy

* grouping and prevent false positives

* dont warn on magic numbers

* rename

* add missing change

* remove a few more lints

* back to 14

* Create .clang-format

* Update .clang-format

* use src dir to allow -fix

* add to format

* improve usability of script
  • Loading branch information
jackgerrits authored Nov 29, 2022
1 parent f8dacd4 commit a295b6e
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"Checks": "-*,readability-*,modernize-*,performance-*,cppcoreguidelines-pro-type-member-init,cppcoreguidelines-init-variables,readability-identifier-naming,-modernize-use-trailing-return-type,-readability-identifier-length,-readability-uppercase-literal-suffix",
"Checks": "-*,readability-*,modernize-*,performance-*,cppcoreguidelines-pro-type-member-init,cppcoreguidelines-init-variables,readability-identifier-naming,-modernize-use-trailing-return-type,-readability-magic-numbers,-readability-function-cognitive-complexity,-readability-identifier-length,-readability-uppercase-literal-suffix",
"FormatStyle": "file",
"WarningsAsErrors": "-*,performance-*,modernize-use-using,readability-braces-around-statements",
"CheckOptions": [
Expand Down
29 changes: 16 additions & 13 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
branches:
- '*'

concurrency:
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.sha }}
cancel-in-progress: true

Expand Down Expand Up @@ -138,18 +138,21 @@ jobs:
fi
run-clang-tidy:
name: lint.c++.clang-tidy
container:
image: vowpalwabbit/ubuntu2004-dev:latest
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
submodules: recursive
- name: Install Ninja
shell: bash
- uses: actions/checkout@v3
- uses: cachix/install-nix-action@v18
- name: Run clang-tidy
run: |
apt update
apt install -y libspdlog-dev libfmt-dev libboost-math-dev
- name: Run clang tidy
shell: bash
run: ./.scripts/linux/run-clang-tidy.sh
echo "::group::nix develop -c vw-clang-tidy"
nix develop -c vw-clang-tidy | tee tidy_out.txt || true
# Prevent false positives for THROW("error: ...") by using the prepended :
grep -A 3 ": error: " tidy_out.txt > tidy_onlyerrors.txt || true
echo "::endgroup::"
if [ -s tidy_onlyerrors.txt ]; then
# file has contents
echo "::group::clang-tidy errors"
cat tidy_onlyerrors.txt
echo "::endgroup::"
exit 11
fi
20 changes: 0 additions & 20 deletions .scripts/linux/run-clang-tidy.sh

This file was deleted.

41 changes: 41 additions & 0 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

110 changes: 110 additions & 0 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
{
# Currently, this flake only contains tools helpful for development.
# Over time, we may add a package definition and other things.
description = "Development utils for Vowpal Wabbit.";

inputs.flake-utils.url = "github:numtide/flake-utils";

outputs = { self, nixpkgs, flake-utils }:
flake-utils.lib.eachDefaultSystem (system:
let pkgs = nixpkgs.legacyPackages.${system}; in
let
generate-compile-commands = ''
echo -n "Generating compile_commands.json... "
rm -rf $TMPDIR/compile_commands_build
mkdir -p $TMPDIR/compile_commands_build
cmake -S . -B "$TMPDIR/compile_commands_build" \
-DCMAKE_EXPORT_COMPILE_COMMANDS=On \
-DRAPIDJSON_SYS_DEP=On \
-DFMT_SYS_DEP=On \
-DSPDLOG_SYS_DEP=On \
-DVW_BOOST_MATH_SYS_DEP=On \
-DVW_ZLIB_SYS_DEP=On \
-DVW_GTEST_SYS_DEP=On \
-DVW_EIGEN_SYS_DEP=On \
-DBUILD_TESTING=Off \
-DVW_BUILD_VW_C_WRAPPER=Off > cmake_compile_commands_output.txt 2>&1
if [ $? -ne 0 ]; then
echo "Failed"
echo
cat cmake_compile_commands_output.txt >&2
exit 1
else
echo "Done"
rm cmake_compile_commands_output.txt
fi
'';
in
let
core-dependencies = [
pkgs.spdlog
pkgs.fmt
pkgs.zlib
pkgs.rapidjson
pkgs.eigen
pkgs.gtest
pkgs.boost
pkgs.cmake
];
in
let
python-clang-tidy-package = pkgs.stdenv.mkDerivation {
name = "python-clang-tidy";
src = pkgs.fetchurl {
url = "https://github.com/llvm/llvm-project/releases/download/llvmorg-14.0.6/clang-tools-extra-14.0.6.src.tar.xz";
sha256 = "sha256-fPO4/1bGXE0erjxWiD/EpsvD/586FTCnTWbkXScnGGY=";
};
sourceRoot = "clang-tools-extra-14.0.6.src";
phases = [ "unpackPhase" "installPhase" "fixupPhase" ];
propagatedBuildInputs = [ pkgs.python3 pkgs.clang-tools_14 ];
installPhase = ''
mkdir -p $out/bin
cp clang-tidy/tool/run-clang-tidy.py $out/bin
cp clang-tidy/tool/run-clang-tidy.py $out/bin/run-clang-tidy
cp clang-tidy/tool/clang-tidy-diff.py $out/bin
cp clang-tidy/tool/clang-tidy-diff.py $out/bin/clang-tidy-diff
'';
};
in
let
python-clang-format-package = pkgs.stdenv.mkDerivation {
name = "python-clang-format";
src = pkgs.fetchurl {
url = "https://github.com/llvm/llvm-project/releases/download/llvmorg-14.0.6/clang-14.0.6.src.tar.xz";
sha256 = "sha256-K1hHtqYxGLnv5chVSDY8gf/glrZsOzZ16VPiY0KuQDE=";
};
sourceRoot = "clang-14.0.6.src";
phases = [ "unpackPhase" "installPhase" "fixupPhase" ];
propagatedBuildInputs = [ pkgs.python3 pkgs.clang-tools_14 ];
installPhase = ''
mkdir -p $out/bin
cp tools/clang-format/clang-format-diff.py $out/bin
cp tools/clang-format/clang-format-diff.py $out/bin/clang-format-diff
'';
};
in
let
clang-tidy-all-script = pkgs.writeShellScriptBin "vw-clang-tidy" ''
${generate-compile-commands}
${python-clang-tidy-package}/bin/run-clang-tidy -p $TMPDIR/compile_commands_build -quiet -header-filter=vw/* "$@"
'';
in
let
clang-tidy-diff-script = pkgs.writeShellScriptBin "vw-clang-tidy-diff" ''
${generate-compile-commands}
${python-clang-tidy-package}/bin/clang-tidy-diff -p1 -path $TMPDIR/compile_commands_build -r '^.*\.(cc|h)\$' -quiet -use-color "$@" <&0
'';
in
{
formatter = pkgs.nixpkgs-fmt;
devShell = pkgs.mkShell {
packages = [
python-clang-tidy-package
python-clang-format-package
clang-tidy-all-script
clang-tidy-diff-script
] ++ core-dependencies;
};
}
);
}
4 changes: 2 additions & 2 deletions utl/clang-format.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/bash
#!/usr/bin/env bash

# Set environment variable GH_WORKFLOW_LOGGING to output logging that Azure pipelines will interpret as a warning.
# Set environment variable WARNING_AS_ERROR to make the script exit with a non-zero code if issues are found.
Expand Down Expand Up @@ -33,7 +33,7 @@ clang-format --version

for FILE in $(find . -type f -not -path './ext_libs/*' -not -path './cs/cli/*' \( -name '*.cc' -o -name "*.h" \) ); do
if [[ "$1" == "check" ]]; then
diff $FILE <(clang-format $FILE);
clang-format --dry-run --Werror $FILE
if [ $? -ne 0 ]; then
ISSUE_FOUND="true"
if [[ -v GH_WORKFLOW_LOGGING ]]; then
Expand Down

0 comments on commit a295b6e

Please sign in to comment.