From f3ef0899c744ff5256414c8539c75e798a058ee0 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 1 Aug 2024 15:02:28 -0700 Subject: [PATCH] build: integrate clang-tidy into CI This still has utterly unacceptably bad output format design that I would not inflict on anyone I like, but it *does* now exist, and you *can* find the errors in the log. Future work would obviously be to fix that and integrate the actual errors into Gerrit using codechecker or so. Followup issue: https://git.lix.systems/lix-project/lix/issues/457 Fixes: https://git.lix.systems/lix-project/lix/issues/147 Change-Id: Ifca22e443d357762125f4ad6bc4f568af3a26c62 --- flake.nix | 15 ++++++++++++++ package.nix | 49 ++++++++++++++++++++++++++++++++++----------- src/libutil/hash.cc | 1 - 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/flake.nix b/flake.nix index c160daed9..662469479 100644 --- a/flake.nix +++ b/flake.nix @@ -282,6 +282,10 @@ # cheaper x86_64-linux compute in CI. # It is clangStdenv because clang's sanitizers are nicer. asanBuild = self.packages.x86_64-linux.nix-clangStdenv.override { + # Improve caching of non-code changes by not changing the + # derivation name every single time, since this will never be seen + # by users anyway. + versionSuffix = ""; sanitize = [ "address" "undefined" @@ -310,6 +314,17 @@ touch $out ''; + # clang-tidy run against the Lix codebase using the Lix clang-tidy plugin + clang-tidy = + let + nixpkgs = nixpkgsFor.x86_64-linux.native; + inherit (nixpkgs) pkgs; + in + pkgs.callPackage ./package.nix { + versionSuffix = ""; + lintInsteadOfBuild = true; + }; + # Make sure that nix-env still produces the exact same result # on a particular version of Nixpkgs. evalNixpkgs = diff --git a/package.nix b/package.nix index be2f0010d..0d77eda68 100644 --- a/package.nix +++ b/package.nix @@ -28,6 +28,8 @@ libcpuid, libseccomp, libsodium, + lix-clang-tidy ? null, + llvmPackages, lsof, lowdown, mdbook, @@ -67,6 +69,8 @@ # Turn compiler warnings into errors. werror ? false, + lintInsteadOfBuild ? false, + # Not a real argument, just the only way to approximate let-binding some # stuff for argument defaults. __forDefaults ? { @@ -144,6 +148,7 @@ let (fileset.fileFilter (f: lib.strings.hasPrefix "nix-profile" f.name) ./scripts) ]; in +assert (lintInsteadOfBuild -> lix-clang-tidy != null); stdenv.mkDerivation (finalAttrs: { inherit pname version; @@ -156,12 +161,13 @@ stdenv.mkDerivation (finalAttrs: { topLevelBuildFiles functionalTestFiles ] - ++ lib.optionals (!finalAttrs.dontBuild || internalApiDocs) [ + ++ lib.optionals (!finalAttrs.dontBuild || internalApiDocs || lintInsteadOfBuild) [ ./doc ./misc ./src ./COPYING ] + ++ lib.optionals lintInsteadOfBuild [ ./.clang-tidy ] ) ); }; @@ -175,7 +181,7 @@ stdenv.mkDerivation (finalAttrs: { "doc" ]; - dontBuild = false; + dontBuild = lintInsteadOfBuild; mesonFlags = let @@ -190,14 +196,15 @@ stdenv.mkDerivation (finalAttrs: { "-Dsandbox-shell=${lib.getExe' busybox-sandbox-shell "busybox"}" ] ++ lib.optional hostPlatform.isStatic "-Denable-embedded-sandbox-shell=true" - ++ lib.optional (finalAttrs.dontBuild) "-Denable-build=false" + ++ lib.optional (finalAttrs.dontBuild && !lintInsteadOfBuild) "-Denable-build=false" + ++ lib.optional lintInsteadOfBuild "-Dlix-clang-tidy-checks-path=${lix-clang-tidy}/lib/liblix-clang-tidy.so" ++ [ # mesonConfigurePhase automatically passes -Dauto_features=enabled, # so we must explicitly enable or disable features that we are not passing # dependencies for. (lib.mesonEnable "gc" enableGC) (lib.mesonEnable "internal-api-docs" internalApiDocs) - (lib.mesonBool "enable-tests" finalAttrs.finalPackage.doCheck) + (lib.mesonBool "enable-tests" (finalAttrs.finalPackage.doCheck || lintInsteadOfBuild)) (lib.mesonBool "enable-docs" canRunInstalled) (lib.mesonBool "werror" werror) ] @@ -230,7 +237,13 @@ stdenv.mkDerivation (finalAttrs: { ] ++ lib.optional hostPlatform.isLinux util-linuxMinimal ++ lib.optional (!officialRelease && buildUnreleasedNotes) build-release-notes - ++ lib.optional internalApiDocs doxygen; + ++ lib.optional internalApiDocs doxygen + ++ lib.optionals lintInsteadOfBuild [ + # required for a wrapped clang-tidy + llvmPackages.clang-tools + # required for run-clang-tidy + llvmPackages.clang-unwrapped + ]; buildInputs = [ @@ -257,7 +270,10 @@ stdenv.mkDerivation (finalAttrs: { ++ lib.optional hostPlatform.isx86_64 libcpuid # There have been issues building these dependencies ++ lib.optional (hostPlatform.canExecute buildPlatform) aws-sdk-cpp-nix - ++ lib.optionals (finalAttrs.dontBuild) maybePropagatedInputs; + ++ lib.optionals (finalAttrs.dontBuild) maybePropagatedInputs + # I am so sorry. This is because checkInputs are required to pass + # configure, but we don't actually want to *run* the checks here. + ++ lib.optionals lintInsteadOfBuild finalAttrs.checkInputs; nativeCheckInputs = [ expect ]; @@ -315,7 +331,7 @@ stdenv.mkDerivation (finalAttrs: { enableParallelBuilding = true; - doCheck = canRunInstalled; + doCheck = canRunInstalled && !lintInsteadOfBuild; mesonCheckFlags = [ "--suite=check" @@ -327,8 +343,19 @@ stdenv.mkDerivation (finalAttrs: { # Make sure the internal API docs are already built, because mesonInstallPhase # won't let us build them there. They would normally be built in buildPhase, # but the internal API docs are conventionally built with doBuild = false. - preInstall = lib.optional internalApiDocs '' - meson ''${mesonBuildFlags:-} compile "$installTargets" + preInstall = + (lib.optionalString internalApiDocs '' + meson ''${mesonBuildFlags:-} compile "$installTargets" + '') + # evil, but like above, we do not want to run an actual build phase + + lib.optionalString lintInsteadOfBuild '' + ninja clang-tidy + ''; + + installPhase = lib.optionalString lintInsteadOfBuild '' + runHook preInstall + touch $out + runHook postInstall ''; postInstall = @@ -396,12 +423,10 @@ stdenv.mkDerivation (finalAttrs: { mkShell, bashInteractive, - clang-tools, clangbuildanalyzer, doxygen, glibcLocales, just, - llvmPackages, nixfmt-rfc-style, skopeo, xonsh, @@ -454,7 +479,7 @@ stdenv.mkDerivation (finalAttrs: { ++ [ (lib.mesonBool "enable-pch-std" stdenv.cc.isClang) ]; packages = - lib.optional (stdenv.cc.isClang && hostPlatform == buildPlatform) clang-tools + lib.optional (stdenv.cc.isClang && hostPlatform == buildPlatform) llvmPackages.clang-tools ++ [ # Why are we providing a bashInteractive? Well, when you run # `bash` from inside `nix develop`, say, because you are using it diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 0ce82f273..46d5425ee 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -109,7 +109,6 @@ static std::string printHash32(const Hash & hash) std::string printHash16or32(const Hash & hash) { - assert(hash.type); return hash.to_string(hash.type == htMD5 ? Base16 : Base32, false); }