Rip off the bandaid: Format the codebase with clang-format #13108

grahamc · 2025-04-29T13:01:55Z

I accidentally deleted the branch from the original PR. A brief summary:

Directional +'s from @roberth and @Ericson2314. Some discussion about updating the formatting one way or another around bracing, which was conceded by @roberth in interest of moving forward. This was brought up again re Lix's format changes, but no resolution came from that.

Eelco had concerns and @Mic92 provided what seemed like positive support and answers. Those concerns (and answers) were what about existing PRs (nixpkgs has tools to help), should it backport (probably), and if the git history becomes useless (.git-blame-ignore-revs, already done in this PR solves it.)

Anything I can do to help move this forward?

original PR body and the regeneration script follows

Motivation

It is tough to contribute to a project that doesn't use a formatter,
It is extra hard to contribute to a project which has configured the formatter, but ignores it for some files
Code formatting makes it harder to hide obscure / weird bugs by accident or on purpose,

Let's rip the bandaid off?

Note that PRs currently in flight should be able to be merged relatively easily by applying clang-format to their tip prior to merge.
I would also expect it to be not too hard to apply patch backports if the backport targets are similarly formatted with clang-format.

Context

Implementation strategy requires a bit of effort on each branch in active maintenance:

Make sure each maintained branch has the same .clang-format file,
Delete the excluded list of unformatted files as the first commit,
Run clang-format (via ./maintainers/format.sh) until it hits steady state (it took two runs) and commit that change
Add the autoformat change to .git-blame-ignore-revs

Once that is done, backports should be easy to apply.

Maintainers can focus on only the change in the first commit, and optionally completely regenerate the second commit to ensure it wasn't tampered with.

This has been done somewhat: new files must be formatted. However, that makes it challenging to contribute with editor integration for code formatting.

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Regeneration script below...

#!/bin/sh

git fetch upstream
git checkout check-format
git reset --hard upstream/master

git am - <<'EOF'
From 296bbe26e47a68f1e0358003c9c445af4a430e51 Mon Sep 17 00:00:00 2001
From: Graham Christensen <[email protected]>
Date: Sun, 6 Apr 2025 15:36:37 -0400
Subject: [PATCH] Drop a ton of files that should just get formatted

---
 maintainers/flake-module.nix | 463 -----------------------------------
 1 file changed, 463 deletions(-)

diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix
index de71be0d7..78240ff65 100644
--- a/maintainers/flake-module.nix
+++ b/maintainers/flake-module.nix
@@ -75,470 +75,7 @@
               ''^src/[^/]*-tests/data/.*$''
 
               # Don't format vendored code
-              ''^doc/manual/redirects\.js$''
               ''^doc/manual/theme/highlight\.js$''
-
-              # We haven't applied formatting to these files yet
-              ''^doc/manual/redirects\.js$''
-              ''^doc/manual/theme/highlight\.js$''
-              ''^precompiled-headers\.h$''
-              ''^src/build-remote/build-remote\.cc$''
-              ''^src/libcmd/built-path\.cc$''
-              ''^src/libcmd/include/nix/cmd/built-path\.hh$''
-              ''^src/libcmd/common-eval-args\.cc$''
-              ''^src/libcmd/include/nix/cmd/common-eval-args\.hh$''
-              ''^src/libcmd/editor-for\.cc$''
-              ''^src/libcmd/installable-attr-path\.cc$''
-              ''^src/libcmd/include/nix/cmd/installable-attr-path\.hh$''
-              ''^src/libcmd/installable-derived-path\.cc$''
-              ''^src/libcmd/include/nix/cmd/installable-derived-path\.hh$''
-              ''^src/libcmd/installable-flake\.cc$''
-              ''^src/libcmd/include/nix/cmd/installable-flake\.hh$''
-              ''^src/libcmd/installable-value\.cc$''
-              ''^src/libcmd/include/nix/cmd/installable-value\.hh$''
-              ''^src/libcmd/installables\.cc$''
-              ''^src/libcmd/include/nix/cmd/installables\.hh$''
-              ''^src/libcmd/include/nix/cmd/legacy\.hh$''
-              ''^src/libcmd/markdown\.cc$''
-              ''^src/libcmd/misc-store-flags\.cc$''
-              ''^src/libcmd/repl-interacter\.cc$''
-              ''^src/libcmd/include/nix/cmd/repl-interacter\.hh$''
-              ''^src/libcmd/repl\.cc$''
-              ''^src/libcmd/include/nix/cmd/repl\.hh$''
-              ''^src/libexpr-c/nix_api_expr\.cc$''
-              ''^src/libexpr-c/nix_api_external\.cc$''
-              ''^src/libexpr/attr-path\.cc$''
-              ''^src/libexpr/include/nix/expr/attr-path\.hh$''
-              ''^src/libexpr/attr-set\.cc$''
-              ''^src/libexpr/include/nix/expr/attr-set\.hh$''
-              ''^src/libexpr/eval-cache\.cc$''
-              ''^src/libexpr/include/nix/expr/eval-cache\.hh$''
-              ''^src/libexpr/eval-error\.cc$''
-              ''^src/libexpr/include/nix/expr/eval-inline\.hh$''
-              ''^src/libexpr/eval-settings\.cc$''
-              ''^src/libexpr/include/nix/expr/eval-settings\.hh$''
-              ''^src/libexpr/eval\.cc$''
-              ''^src/libexpr/include/nix/expr/eval\.hh$''
-              ''^src/libexpr/function-trace\.cc$''
-              ''^src/libexpr/include/nix/expr/gc-small-vector\.hh$''
-              ''^src/libexpr/get-drvs\.cc$''
-              ''^src/libexpr/include/nix/expr/get-drvs\.hh$''
-              ''^src/libexpr/json-to-value\.cc$''
-              ''^src/libexpr/nixexpr\.cc$''
-              ''^src/libexpr/include/nix/expr/nixexpr\.hh$''
-              ''^src/libexpr/include/nix/expr/parser-state\.hh$''
-              ''^src/libexpr/primops\.cc$''
-              ''^src/libexpr/include/nix/expr/primops\.hh$''
-              ''^src/libexpr/primops/context\.cc$''
-              ''^src/libexpr/primops/fetchClosure\.cc$''
-              ''^src/libexpr/primops/fetchMercurial\.cc$''
-              ''^src/libexpr/primops/fetchTree\.cc$''
-              ''^src/libexpr/primops/fromTOML\.cc$''
-              ''^src/libexpr/print-ambiguous\.cc$''
-              ''^src/libexpr/include/nix/expr/print-ambiguous\.hh$''
-              ''^src/libexpr/include/nix/expr/print-options\.hh$''
-              ''^src/libexpr/print\.cc$''
-              ''^src/libexpr/include/nix/expr/print\.hh$''
-              ''^src/libexpr/search-path\.cc$''
-              ''^src/libexpr/include/nix/expr/symbol-table\.hh$''
-              ''^src/libexpr/value-to-json\.cc$''
-              ''^src/libexpr/include/nix/expr/value-to-json\.hh$''
-              ''^src/libexpr/value-to-xml\.cc$''
-              ''^src/libexpr/include/nix/expr/value-to-xml\.hh$''
-              ''^src/libexpr/include/nix/expr/value\.hh$''
-              ''^src/libexpr/value/context\.cc$''
-              ''^src/libexpr/include/nix/expr/value/context\.hh$''
-              ''^src/libfetchers/attrs\.cc$''
-              ''^src/libfetchers/cache\.cc$''
-              ''^src/libfetchers/include/nix/fetchers/cache\.hh$''
-              ''^src/libfetchers/fetch-settings\.cc$''
-              ''^src/libfetchers/include/nix/fetchers/fetch-settings\.hh$''
-              ''^src/libfetchers/fetch-to-store\.cc$''
-              ''^src/libfetchers/fetchers\.cc$''
-              ''^src/libfetchers/include/nix/fetchers/fetchers\.hh$''
-              ''^src/libfetchers/filtering-source-accessor\.cc$''
-              ''^src/libfetchers/include/nix/fetchers/filtering-source-accessor\.hh$''
-              ''^src/libfetchers/fs-source-accessor\.cc$''
-              ''^src/libfetchers/include/nix/fs-source-accessor\.hh$''
-              ''^src/libfetchers/git-utils\.cc$''
-              ''^src/libfetchers/include/nix/fetchers/git-utils\.hh$''
-              ''^src/libfetchers/github\.cc$''
-              ''^src/libfetchers/indirect\.cc$''
-              ''^src/libfetchers/memory-source-accessor\.cc$''
-              ''^src/libfetchers/path\.cc$''
-              ''^src/libfetchers/registry\.cc$''
-              ''^src/libfetchers/include/nix/fetchers/registry\.hh$''
-              ''^src/libfetchers/tarball\.cc$''
-              ''^src/libfetchers/include/nix/fetchers/tarball\.hh$''
-              ''^src/libfetchers/git\.cc$''
-              ''^src/libfetchers/mercurial\.cc$''
-              ''^src/libflake/config\.cc$''
-              ''^src/libflake/flake\.cc$''
-              ''^src/libflake/include/nix/flake/flake\.hh$''
-              ''^src/libflake/flakeref\.cc$''
-              ''^src/libflake/include/nix/flake/flakeref\.hh$''
-              ''^src/libflake/lockfile\.cc$''
-              ''^src/libflake/include/nix/flake/lockfile\.hh$''
-              ''^src/libflake/url-name\.cc$''
-              ''^src/libmain/common-args\.cc$''
-              ''^src/libmain/include/nix/main/common-args\.hh$''
-              ''^src/libmain/loggers\.cc$''
-              ''^src/libmain/include/nix/main/loggers\.hh$''
-              ''^src/libmain/progress-bar\.cc$''
-              ''^src/libmain/shared\.cc$''
-              ''^src/libmain/include/nix/main/shared\.hh$''
-              ''^src/libmain/unix/stack\.cc$''
-              ''^src/libstore/binary-cache-store\.cc$''
-              ''^src/libstore/include/nix/store/binary-cache-store\.hh$''
-              ''^src/libstore/include/nix/store/build-result\.hh$''
-              ''^src/libstore/include/nix/store/builtins\.hh$''
-              ''^src/libstore/builtins/buildenv\.cc$''
-              ''^src/libstore/include/nix/store/builtins/buildenv\.hh$''
-              ''^src/libstore/include/nix/store/common-protocol-impl\.hh$''
-              ''^src/libstore/common-protocol\.cc$''
-              ''^src/libstore/include/nix/store/common-protocol\.hh$''
-              ''^src/libstore/include/nix/store/common-ssh-store-config\.hh$''
-              ''^src/libstore/content-address\.cc$''
-              ''^src/libstore/include/nix/store/content-address\.hh$''
-              ''^src/libstore/daemon\.cc$''
-              ''^src/libstore/include/nix/store/daemon\.hh$''
-              ''^src/libstore/derivations\.cc$''
-              ''^src/libstore/include/nix/store/derivations\.hh$''
-              ''^src/libstore/derived-path-map\.cc$''
-              ''^src/libstore/include/nix/store/derived-path-map\.hh$''
-              ''^src/libstore/derived-path\.cc$''
-              ''^src/libstore/include/nix/store/derived-path\.hh$''
-              ''^src/libstore/downstream-placeholder\.cc$''
-              ''^src/libstore/include/nix/store/downstream-placeholder\.hh$''
-              ''^src/libstore/dummy-store\.cc$''
-              ''^src/libstore/export-import\.cc$''
-              ''^src/libstore/filetransfer\.cc$''
-              ''^src/libstore/include/nix/store/filetransfer\.hh$''
-              ''^src/libstore/include/nix/store/gc-store\.hh$''
-              ''^src/libstore/globals\.cc$''
-              ''^src/libstore/include/nix/store/globals\.hh$''
-              ''^src/libstore/http-binary-cache-store\.cc$''
-              ''^src/libstore/legacy-ssh-store\.cc$''
-              ''^src/libstore/include/nix/store/legacy-ssh-store\.hh$''
-              ''^src/libstore/include/nix/store/length-prefixed-protocol-helper\.hh$''
-              ''^src/libstore/linux/personality\.cc$''
-              ''^src/libstore/linux/include/nix/store/personality\.hh$''
-              ''^src/libstore/local-binary-cache-store\.cc$''
-              ''^src/libstore/local-fs-store\.cc$''
-              ''^src/libstore/include/nix/store/local-fs-store\.hh$''
-              ''^src/libstore/log-store\.cc$''
-              ''^src/libstore/include/nix/store/log-store\.hh$''
-              ''^src/libstore/machines\.cc$''
-              ''^src/libstore/include/nix/store/machines\.hh$''
-              ''^src/libstore/make-content-addressed\.cc$''
-              ''^src/libstore/include/nix/store/make-content-addressed\.hh$''
-              ''^src/libstore/misc\.cc$''
-              ''^src/libstore/names\.cc$''
-              ''^src/libstore/include/nix/store/names\.hh$''
-              ''^src/libstore/nar-accessor\.cc$''
-              ''^src/libstore/include/nix/store/nar-accessor\.hh$''
-              ''^src/libstore/nar-info-disk-cache\.cc$''
-              ''^src/libstore/include/nix/store/nar-info-disk-cache\.hh$''
-              ''^src/libstore/nar-info\.cc$''
-              ''^src/libstore/include/nix/store/nar-info\.hh$''
-              ''^src/libstore/outputs-spec\.cc$''
-              ''^src/libstore/include/nix/store/outputs-spec\.hh$''
-              ''^src/libstore/parsed-derivations\.cc$''
-              ''^src/libstore/path-info\.cc$''
-              ''^src/libstore/include/nix/store/path-info\.hh$''
-              ''^src/libstore/path-references\.cc$''
-              ''^src/libstore/include/nix/store/path-regex\.hh$''
-              ''^src/libstore/path-with-outputs\.cc$''
-              ''^src/libstore/path\.cc$''
-              ''^src/libstore/include/nix/store/path\.hh$''
-              ''^src/libstore/pathlocks\.cc$''
-              ''^src/libstore/include/nix/store/pathlocks\.hh$''
-              ''^src/libstore/profiles\.cc$''
-              ''^src/libstore/include/nix/store/profiles\.hh$''
-              ''^src/libstore/realisation\.cc$''
-              ''^src/libstore/include/nix/store/realisation\.hh$''
-              ''^src/libstore/remote-fs-accessor\.cc$''
-              ''^src/libstore/include/nix/store/remote-fs-accessor\.hh$''
-              ''^src/libstore/include/nix/store/remote-store-connection\.hh$''
-              ''^src/libstore/remote-store\.cc$''
-              ''^src/libstore/include/nix/store/remote-store\.hh$''
-              ''^src/libstore/s3-binary-cache-store\.cc$''
-              ''^src/libstore/include/nix/store/s3\.hh$''
-              ''^src/libstore/serve-protocol-impl\.cc$''
-              ''^src/libstore/include/nix/store/serve-protocol-impl\.hh$''
-              ''^src/libstore/serve-protocol\.cc$''
-              ''^src/libstore/include/nix/store/serve-protocol\.hh$''
-              ''^src/libstore/sqlite\.cc$''
-              ''^src/libstore/include/nix/store/sqlite\.hh$''
-              ''^src/libstore/ssh-store\.cc$''
-              ''^src/libstore/ssh\.cc$''
-              ''^src/libstore/include/nix/store/ssh\.hh$''
-              ''^src/libstore/store-api\.cc$''
-              ''^src/libstore/include/nix/store/store-api\.hh$''
-              ''^src/libstore/include/nix/store/store-dir-config\.hh$''
-              ''^src/libstore/build/derivation-goal\.cc$''
-              ''^src/libstore/include/nix/store/build/derivation-goal\.hh$''
-              ''^src/libstore/build/drv-output-substitution-goal\.cc$''
-              ''^src/libstore/include/nix/store/build/drv-output-substitution-goal\.hh$''
-              ''^src/libstore/build/entry-points\.cc$''
-              ''^src/libstore/build/goal\.cc$''
-              ''^src/libstore/include/nix/store/build/goal\.hh$''
-              ''^src/libstore/unix/build/hook-instance\.cc$''
-              ''^src/libstore/unix/build/derivation-builder\.cc$''
-              ''^src/libstore/unix/include/nix/store/build/derivation-builder\.hh$''
-              ''^src/libstore/build/substitution-goal\.cc$''
-              ''^src/libstore/include/nix/store/build/substitution-goal\.hh$''
-              ''^src/libstore/build/worker\.cc$''
-              ''^src/libstore/include/nix/store/build/worker\.hh$''
-              ''^src/libstore/builtins/fetchurl\.cc$''
-              ''^src/libstore/builtins/unpack-channel\.cc$''
-              ''^src/libstore/gc\.cc$''
-              ''^src/libstore/local-overlay-store\.cc$''
-              ''^src/libstore/include/nix/store/local-overlay-store\.hh$''
-              ''^src/libstore/local-store\.cc$''
-              ''^src/libstore/include/nix/store/local-store\.hh$''
-              ''^src/libstore/unix/user-lock\.cc$''
-              ''^src/libstore/unix/include/nix/store/user-lock\.hh$''
-              ''^src/libstore/optimise-store\.cc$''
-              ''^src/libstore/unix/pathlocks\.cc$''
-              ''^src/libstore/posix-fs-canonicalise\.cc$''
-              ''^src/libstore/include/nix/store/posix-fs-canonicalise\.hh$''
-              ''^src/libstore/uds-remote-store\.cc$''
-              ''^src/libstore/include/nix/store/uds-remote-store\.hh$''
-              ''^src/libstore/windows/build\.cc$''
-              ''^src/libstore/include/nix/store/worker-protocol-impl\.hh$''
-              ''^src/libstore/worker-protocol\.cc$''
-              ''^src/libstore/include/nix/store/worker-protocol\.hh$''
-              ''^src/libutil-c/nix_api_util_internal\.h$''
-              ''^src/libutil/archive\.cc$''
-              ''^src/libutil/include/nix/util/archive\.hh$''
-              ''^src/libutil/args\.cc$''
-              ''^src/libutil/include/nix/util/args\.hh$''
-              ''^src/libutil/include/nix/util/args/root\.hh$''
-              ''^src/libutil/include/nix/util/callback\.hh$''
-              ''^src/libutil/canon-path\.cc$''
-              ''^src/libutil/include/nix/util/canon-path\.hh$''
-              ''^src/libutil/include/nix/util/chunked-vector\.hh$''
-              ''^src/libutil/include/nix/util/closure\.hh$''
-              ''^src/libutil/include/nix/util/comparator\.hh$''
-              ''^src/libutil/compute-levels\.cc$''
-              ''^src/libutil/include/nix/util/config-impl\.hh$''
-              ''^src/libutil/configuration\.cc$''
-              ''^src/libutil/include/nix/util/configuration\.hh$''
-              ''^src/libutil/current-process\.cc$''
-              ''^src/libutil/include/nix/util/current-process\.hh$''
-              ''^src/libutil/english\.cc$''
-              ''^src/libutil/include/nix/util/english\.hh$''
-              ''^src/libutil/error\.cc$''
-              ''^src/libutil/include/nix/util/error\.hh$''
-              ''^src/libutil/include/nix/util/exit\.hh$''
-              ''^src/libutil/experimental-features\.cc$''
-              ''^src/libutil/include/nix/util/experimental-features\.hh$''
-              ''^src/libutil/file-content-address\.cc$''
-              ''^src/libutil/include/nix/util/file-content-address\.hh$''
-              ''^src/libutil/file-descriptor\.cc$''
-              ''^src/libutil/include/nix/util/file-descriptor\.hh$''
-              ''^src/libutil/include/nix/util/file-path-impl\.hh$''
-              ''^src/libutil/include/nix/util/file-path\.hh$''
-              ''^src/libutil/file-system\.cc$''
-              ''^src/libutil/include/nix/util/file-system\.hh$''
-              ''^src/libutil/include/nix/util/finally\.hh$''
-              ''^src/libutil/include/nix/util/fmt\.hh$''
-              ''^src/libutil/fs-sink\.cc$''
-              ''^src/libutil/include/nix/util/fs-sink\.hh$''
-              ''^src/libutil/git\.cc$''
-              ''^src/libutil/include/nix/util/git\.hh$''
-              ''^src/libutil/hash\.cc$''
-              ''^src/libutil/include/nix/util/hash\.hh$''
-              ''^src/libutil/hilite\.cc$''
-              ''^src/libutil/include/nix/util/hilite\.hh$''
-              ''^src/libutil/source-accessor\.hh$''
-              ''^src/libutil/include/nix/util/json-impls\.hh$''
-              ''^src/libutil/json-utils\.cc$''
-              ''^src/libutil/include/nix/util/json-utils\.hh$''
-              ''^src/libutil/linux/cgroup\.cc$''
-              ''^src/libutil/linux/namespaces\.cc$''
-              ''^src/libutil/logging\.cc$''
-              ''^src/libutil/include/nix/util/logging\.hh$''
-              ''^src/libutil/include/nix/util/lru-cache\.hh$''
-              ''^src/libutil/memory-source-accessor\.cc$''
-              ''^src/libutil/include/nix/util/memory-source-accessor\.hh$''
-              ''^src/libutil/include/nix/util/pool\.hh$''
-              ''^src/libutil/position\.cc$''
-              ''^src/libutil/include/nix/util/position\.hh$''
-              ''^src/libutil/posix-source-accessor\.cc$''
-              ''^src/libutil/include/nix/util/posix-source-accessor\.hh$''
-              ''^src/libutil/include/nix/util/processes\.hh$''
-              ''^src/libutil/include/nix/util/ref\.hh$''
-              ''^src/libutil/references\.cc$''
-              ''^src/libutil/include/nix/util/references\.hh$''
-              ''^src/libutil/regex-combinators\.hh$''
-              ''^src/libutil/serialise\.cc$''
-              ''^src/libutil/include/nix/util/serialise\.hh$''
-              ''^src/libutil/include/nix/util/signals\.hh$''
-              ''^src/libutil/signature/local-keys\.cc$''
-              ''^src/libutil/include/nix/util/signature/local-keys\.hh$''
-              ''^src/libutil/signature/signer\.cc$''
-              ''^src/libutil/include/nix/util/signature/signer\.hh$''
-              ''^src/libutil/source-accessor\.cc$''
-              ''^src/libutil/include/nix/util/source-accessor\.hh$''
-              ''^src/libutil/source-path\.cc$''
-              ''^src/libutil/include/nix/util/source-path\.hh$''
-              ''^src/libutil/include/nix/util/split\.hh$''
-              ''^src/libutil/suggestions\.cc$''
-              ''^src/libutil/include/nix/util/suggestions\.hh$''
-              ''^src/libutil/include/nix/util/sync\.hh$''
-              ''^src/libutil/terminal\.cc$''
-              ''^src/libutil/include/nix/util/terminal\.hh$''
-              ''^src/libutil/thread-pool\.cc$''
-              ''^src/libutil/include/nix/util/thread-pool\.hh$''
-              ''^src/libutil/include/nix/util/topo-sort\.hh$''
-              ''^src/libutil/include/nix/util/types\.hh$''
-              ''^src/libutil/unix/file-descriptor\.cc$''
-              ''^src/libutil/unix/file-path\.cc$''
-              ''^src/libutil/unix/processes\.cc$''
-              ''^src/libutil/unix/include/nix/util/signals-impl\.hh$''
-              ''^src/libutil/unix/signals\.cc$''
-              ''^src/libutil/unix-domain-socket\.cc$''
-              ''^src/libutil/unix/users\.cc$''
-              ''^src/libutil/include/nix/util/url-parts\.hh$''
-              ''^src/libutil/url\.cc$''
-              ''^src/libutil/include/nix/util/url\.hh$''
-              ''^src/libutil/users\.cc$''
-              ''^src/libutil/include/nix/util/users\.hh$''
-              ''^src/libutil/util\.cc$''
-              ''^src/libutil/include/nix/util/util\.hh$''
-              ''^src/libutil/include/nix/util/variant-wrapper\.hh$''
-              ''^src/libutil/widecharwidth/widechar_width\.h$'' # vendored source
-              ''^src/libutil/windows/file-descriptor\.cc$''
-              ''^src/libutil/windows/file-path\.cc$''
-              ''^src/libutil/windows/processes\.cc$''
-              ''^src/libutil/windows/users\.cc$''
-              ''^src/libutil/windows/windows-error\.cc$''
-              ''^src/libutil/windows/include/nix/util/windows-error\.hh$''
-              ''^src/libutil/xml-writer\.cc$''
-              ''^src/libutil/include/nix/util/xml-writer\.hh$''
-              ''^src/nix-build/nix-build\.cc$''
-              ''^src/nix-channel/nix-channel\.cc$''
-              ''^src/nix-collect-garbage/nix-collect-garbage\.cc$''
-              ''^src/nix-env/buildenv.nix$''
-              ''^src/nix-env/nix-env\.cc$''
-              ''^src/nix-env/user-env\.cc$''
-              ''^src/nix-env/user-env\.hh$''
-              ''^src/nix-instantiate/nix-instantiate\.cc$''
-              ''^src/nix-store/dotgraph\.cc$''
-              ''^src/nix-store/graphml\.cc$''
-              ''^src/nix-store/nix-store\.cc$''
-              ''^src/nix/add-to-store\.cc$''
-              ''^src/nix/app\.cc$''
-              ''^src/nix/build\.cc$''
-              ''^src/nix/bundle\.cc$''
-              ''^src/nix/cat\.cc$''
-              ''^src/nix/config-check\.cc$''
-              ''^src/nix/config\.cc$''
-              ''^src/nix/copy\.cc$''
-              ''^src/nix/derivation-add\.cc$''
-              ''^src/nix/derivation-show\.cc$''
-              ''^src/nix/derivation\.cc$''
-              ''^src/nix/develop\.cc$''
-              ''^src/nix/diff-closures\.cc$''
-              ''^src/nix/dump-path\.cc$''
-              ''^src/nix/edit\.cc$''
-              ''^src/nix/eval\.cc$''
-              ''^src/nix/flake\.cc$''
-              ''^src/nix/fmt\.cc$''
-              ''^src/nix/hash\.cc$''
-              ''^src/nix/log\.cc$''
-              ''^src/nix/ls\.cc$''
-              ''^src/nix/main\.cc$''
-              ''^src/nix/make-content-addressed\.cc$''
-              ''^src/nix/nar\.cc$''
-              ''^src/nix/optimise-store\.cc$''
-              ''^src/nix/path-from-hash-part\.cc$''
-              ''^src/nix/path-info\.cc$''
-              ''^src/nix/prefetch\.cc$''
-              ''^src/nix/profile\.cc$''
-              ''^src/nix/realisation\.cc$''
-              ''^src/nix/registry\.cc$''
-              ''^src/nix/repl\.cc$''
-              ''^src/nix/run\.cc$''
-              ''^src/nix/run\.hh$''
-              ''^src/nix/search\.cc$''
-              ''^src/nix/sigs\.cc$''
-              ''^src/nix/store-copy-log\.cc$''
-              ''^src/nix/store-delete\.cc$''
-              ''^src/nix/store-gc\.cc$''
-              ''^src/nix/store-info\.cc$''
-              ''^src/nix/store-repair\.cc$''
-              ''^src/nix/store\.cc$''
-              ''^src/nix/unix/daemon\.cc$''
-              ''^src/nix/upgrade-nix\.cc$''
-              ''^src/nix/verify\.cc$''
-              ''^src/nix/why-depends\.cc$''
-
-              ''^tests/functional/plugins/plugintest\.cc''
-              ''^tests/functional/test-libstoreconsumer/main\.cc''
-              ''^tests/nixos/ca-fd-leak/sender\.c''
-              ''^tests/nixos/ca-fd-leak/smuggler\.c''
-              ''^tests/nixos/user-sandboxing/attacker\.c''
-              ''^src/libexpr-test-support/include/nix/expr/tests/libexpr\.hh''
-              ''^src/libexpr-test-support/tests/value/context\.cc''
-              ''^src/libexpr-test-support/include/nix/expr/tests/value/context\.hh''
-              ''^src/libexpr-tests/derived-path\.cc''
-              ''^src/libexpr-tests/error_traces\.cc''
-              ''^src/libexpr-tests/eval\.cc''
-              ''^src/libexpr-tests/json\.cc''
-              ''^src/libexpr-tests/main\.cc''
-              ''^src/libexpr-tests/primops\.cc''
-              ''^src/libexpr-tests/search-path\.cc''
-              ''^src/libexpr-tests/trivial\.cc''
-              ''^src/libexpr-tests/value/context\.cc''
-              ''^src/libexpr-tests/value/print\.cc''
-              ''^src/libfetchers-tests/public-key\.cc''
-              ''^src/libflake-tests/flakeref\.cc''
-              ''^src/libflake-tests/url-name\.cc''
-              ''^src/libstore-test-support/tests/derived-path\.cc''
-              ''^src/libstore-test-support/include/nix/store/tests/derived-path\.hh''
-              ''^src/libstore-test-support/include/nix/store/tests/nix_api_store\.hh''
-              ''^src/libstore-test-support/tests/outputs-spec\.cc''
-              ''^src/libstore-test-support/include/nix/store/tests/outputs-spec\.hh''
-              ''^src/libstore-test-support/path\.cc''
-              ''^src/libstore-test-support/include/nix/store/tests/path\.hh''
-              ''^src/libstore-test-support/include/nix/store/tests/protocol\.hh''
-              ''^src/libstore-tests/common-protocol\.cc''
-              ''^src/libstore-tests/content-address\.cc''
-              ''^src/libstore-tests/derivation\.cc''
-              ''^src/libstore-tests/derived-path\.cc''
-              ''^src/libstore-tests/downstream-placeholder\.cc''
-              ''^src/libstore-tests/machines\.cc''
-              ''^src/libstore-tests/nar-info-disk-cache\.cc''
-              ''^src/libstore-tests/nar-info\.cc''
-              ''^src/libstore-tests/outputs-spec\.cc''
-              ''^src/libstore-tests/path-info\.cc''
-              ''^src/libstore-tests/path\.cc''
-              ''^src/libstore-tests/serve-protocol\.cc''
-              ''^src/libstore-tests/worker-protocol\.cc''
-              ''^src/libutil-test-support/include/nix/util/tests/characterization\.hh''
-              ''^src/libutil-test-support/hash\.cc''
-              ''^src/libutil-test-support/include/nix/util/tests/hash\.hh''
-              ''^src/libutil-tests/args\.cc''
-              ''^src/libutil-tests/canon-path\.cc''
-              ''^src/libutil-tests/chunked-vector\.cc''
-              ''^src/libutil-tests/closure\.cc''
-              ''^src/libutil-tests/compression\.cc''
-              ''^src/libutil-tests/config\.cc''
-              ''^src/libutil-tests/file-content-address\.cc''
-              ''^src/libutil-tests/git\.cc''
-              ''^src/libutil-tests/hash\.cc''
-              ''^src/libutil-tests/hilite\.cc''
-              ''^src/libutil-tests/json-utils\.cc''
-              ''^src/libutil-tests/logging\.cc''
-              ''^src/libutil-tests/lru-cache\.cc''
-              ''^src/libutil-tests/pool\.cc''
-              ''^src/libutil-tests/references\.cc''
-              ''^src/libutil-tests/suggestions\.cc''
-              ''^src/libutil-tests/url\.cc''
-              ''^src/libutil-tests/xml-writer\.cc''
             ];
           };
           shellcheck = {
-- 
2.39.5 (Apple Git-154)
EOF

while ! nix develop -c ./maintainers/format.sh; do
    echo "again?"
    sleep 1
done

msg=$(cat <<'EOF'
Apply clang-format universally.

* It is tough to contribute to a project that doesn't use a formatter,
* It is extra hard to contribute to a project which has configured the formatter, but ignores it for some files
* Code formatting makes it harder to hide obscure / weird bugs by accident or on purpose,

Let's rip the bandaid off?

Note that PRs currently in flight should be able to be merged relatively easily by applying `clang-format` to their tip prior to merge.
EOF
)

git diff --name-only | xargs git add
git commit -m "$msg"

printf "%s # bulk initial re-formatting with clang-format\n" "$(git rev-parse HEAD)" >> .git-blame-ignore-revs
git add .git-blame-ignore-revs
git commit -m "Update .git-blame-ignore-revs to ignore the mass reformatting"
git push origin check-format --force-with-lease

Mic92 · 2025-04-29T15:23:29Z

@grahamc so I think the consense was that we can merge this if we have a strategy how to deal with existing pull requests. Does the script that works in nixpkgs for nixfmt also works with clang-format to rebate pull requests? I didn't got a chance yet the try it out but if this is done, I'll hit merge.

grahamc · 2025-04-29T18:08:30Z

@Mic92 cool! Can you link me to the script?

Mic92 · 2025-04-29T19:12:42Z

@Mic92 cool! Can you link me to the script?

https://github.com/NixOS/nixpkgs/pull/363759/files
And you are actually the original author of it: https://github.com/grahamc/git-rebase-format

grahamc · 2025-04-29T19:40:16Z

oh, fancy! =) I'll take a look!

Mic92 · 2025-05-12T20:47:59Z

oh, fancy! =) I'll take a look!

Did you had a chance to look at it?

grahamc · 2025-05-20T16:55:14Z

Ok! I gave this a go, and it worked well on some PRs when run like this:

nix develop -c ../../NixOS/nixpkgs/maintainers/scripts/auto-rebase/run.sh check-format

note that I had to add gnused to the dev shell for it to work on a mac.

However, I did try it on a more complicated PR and I don't understand what it is trying to tell me:

+ git rebase --onto 7f2e78cad9ecd1a9d33dfd50e65316c733c88d90 45726dcc556d2528b6848d5545b8297a9aa041b2
Auto-merging .github/workflows/ci.yml
CONFLICT (content): Merge conflict in .github/workflows/ci.yml
error: could not apply 257ab726a... Run the flake-regressions test suite
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Could not apply 257ab726a... Run the flake-regressions test suite
+ echo -e '\e[33m\e[1mRestart this script after resolving the merge conflict as described above\e[0m'
Restart this script after resolving the merge conflict as described above

so I fixed the merge conflict, and wasn't sure if I should run the script immediately after fixing the edits, after running git rebase --continue, if I should continue the rebase to the ned and then re-run it, or what.

Mic92 · 2025-05-20T17:19:46Z

@grahamc you did replace nixfmt with clang-format in run.sh, right?

grahamc · 2025-05-20T17:32:03Z

Yeah, and actually that script doesn't call nixfmt, it uses the command specified in the .git-blame-ignore-revs.

On that PR, I think it actually makes more sense to just do a big format on the tips and merge from there. So in other words, I think it is actually working perfectly.

Mic92 · 2025-05-20T20:43:52Z

Yeah, and actually that script doesn't call nixfmt, it uses the command specified in the .git-blame-ignore-revs.

On that PR, I think it actually makes more sense to just do a big format on the tips and merge from there. So in other words, I think it is actually working perfectly.

So all we need to do after merging this is: nix develop -c ../../NixOS/nixpkgs/maintainers/scripts/auto-rebase/run.sh check-format for every pull request?

grahamc · 2025-05-20T22:52:38Z

I think so! Want to test it on a few?

Mic92 · 2025-05-21T05:52:54Z

For some reason it didn't work the first time I tried it, but the second time?
Anyway I was able to rebase lazytree v2 after that: Mic92#9

grahamc · 2025-05-21T21:20:14Z

For some reason it didn't work the first time I tried it, but the second time? Anyway I was able to rebase lazytree v2 after that: Mic92#9

Nice! I've just pushed another reformat. Exciting..!

fzakaria · 2025-05-22T13:59:04Z

Lets also run clang-tidy and remove unnecessary headers!
I like this.

It's should be trivial to re apply to a branch (apply before the merge). Why so much headache over existing PRs?

➕

grahamc · 2025-05-22T15:02:52Z

I ran clang-tidy and it did basically nothing, so I think there'll need to be a separate PR bringing that in 👍

fzakaria · 2025-05-22T15:06:05Z

I'm not sure if we have our .clang-tidy file set up right but it includes https://clang.llvm.org/extra/clang-tidy/checks/misc/include-cleaner.html

grahamc · 2025-05-22T15:57:16Z

I'm not sure if we have our .clang-tidy file set up right but it includes https://clang.llvm.org/extra/clang-tidy/checks/misc/include-cleaner.html

I tried adding that to this PR, but it caused compilation to fail, so I'm not sure.

fzakaria · 2025-05-22T16:34:40Z

@grahamc yea when I removed some headers it complained about -- some seemed to be necessary fro C++ templating.
I had to add them explicitly.

We can do it as a follow-up.
Can you write the file you did to trigger it though for whomever wants to pick it up please.

fzakaria · 2025-05-22T16:35:38Z

@grahamc also does this PR include a check to make sure it stays formatted?

Looks like meson https://mesonbuild.com/Code-formatting.html already includeds a format-check target that should be added to the GitHub actions.

grahamc · 2025-05-22T17:18:10Z

@fzakaria Yes, the tests jobs builds the pre-commit runs:

pre-commit-run> Running: $ pre-commit run --all-files
pre-commit-run> check-merge-conflicts....................................................Passed
nix-util> [5/58] Compiling C++ object libnixutil.so.p/current-process.cc.o
nix-util> [6/58] Compiling C++ object libnixutil.so.p/english.cc.o
nix-util> [7/58] Compiling C++ object libnixutil.so.p/config-global.cc.o
nix-util> [8/58] Compiling C++ object libnixutil.so.p/args.cc.o
pre-commit-run> check-merge-conflicts-2..................................................Passed
nix-util> [5/58] Compiling C++ object libnixutil.so.p/current-process.cc.o
nix-util> [6/58] Compiling C++ object libnixutil.so.p/config-global.cc.o
nix-util> [7/58] Compiling C++ object libnixutil.so.p/english.cc.o
pre-commit-run> clang-format.............................................................Passed
pre-commit-run> nixfmt-rfc-style.........................................................Passed
nix-util> [8/58] Compiling C++ object libnixutil.so.p/args.cc.o
nix-util> [9/58] Compiling C++ object libnixutil.so.p/environment-variables.cc.o
nix-util> [10/58] Compiling C++ object libnixutil.so.p/exit.cc.o
nix-util> [11/58] Compiling C++ object libnixutil.so.p/configuration.cc.o
nix-util> [9/58] Compiling C++ object libnixutil.so.p/environment-variables.cc.o

fzakaria · 2025-05-22T17:24:17Z

Thank you for the edification -- seems to not use the Meson configuration.
(Don't think it matters but I wasn't sure how it gets run)

* It is tough to contribute to a project that doesn't use a formatter, * It is extra hard to contribute to a project which has configured the formatter, but ignores it for some files * Code formatting makes it harder to hide obscure / weird bugs by accident or on purpose, Let's rip the bandaid off? Note that PRs currently in flight should be able to be merged relatively easily by applying `clang-format` to their tip prior to merge.

grahamc · 2025-05-22T19:25:01Z

Hey @Mic92 what's the process / next steps? :)

Mic92 · 2025-05-25T15:18:40Z

Hey @Mic92 what's the process / next steps? :)

Check your email inbox.

grahamc · 2025-05-25T17:04:08Z

I don't see anything :/

Mic92 · 2025-05-25T18:48:10Z

I don't see anything :/

Interesting. Please ask Eelco for more information.

grahamc requested review from Ericson2314, roberth and edolstra as code owners April 29, 2025 13:01

grahamc force-pushed the check-format branch 8 times, most recently from 8eca3a2 to fb60ae4 Compare May 20, 2025 16:48

grahamc force-pushed the check-format branch from fb60ae4 to f797993 Compare May 20, 2025 20:00

grahamc force-pushed the check-format branch from f797993 to ca6411f Compare May 21, 2025 21:18

grahamc force-pushed the check-format branch from ca6411f to 9a29618 Compare May 21, 2025 22:45

grahamc force-pushed the check-format branch 2 times, most recently from b1aafbd to 25b3999 Compare May 22, 2025 15:56

grahamc added 5 commits May 22, 2025 15:24

format.sh: support looping until it is happy

90db903

Add sed

f7390ea

Drop a ton of files that should just get formatted

aa296bb

Update .git-blame-ignore-revs to ignore the mass reformatting

849683a

grahamc force-pushed the check-format branch from 25b3999 to 849683a Compare May 22, 2025 19:24

Uh oh!

Rip off the bandaid: Format the codebase with clang-format #13108

Are you sure you want to change the base?

Rip off the bandaid: Format the codebase with clang-format #13108

Uh oh!

Conversation

grahamc commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Context

Uh oh!

Mic92 commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grahamc commented Apr 29, 2025

Uh oh!

Mic92 commented Apr 29, 2025

Uh oh!

grahamc commented Apr 29, 2025

Uh oh!

Mic92 commented May 12, 2025

Uh oh!

grahamc commented May 20, 2025

Uh oh!

Mic92 commented May 20, 2025

Uh oh!

grahamc commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mic92 commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grahamc commented May 20, 2025

Uh oh!

Mic92 commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grahamc commented May 21, 2025

Uh oh!

fzakaria commented May 22, 2025

Uh oh!

grahamc commented May 22, 2025

Uh oh!

fzakaria commented May 22, 2025

Uh oh!

grahamc commented May 22, 2025

Uh oh!

fzakaria commented May 22, 2025

Uh oh!

fzakaria commented May 22, 2025

Uh oh!

grahamc commented May 22, 2025

Uh oh!

fzakaria commented May 22, 2025

Uh oh!

grahamc commented May 22, 2025

Uh oh!

Mic92 commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grahamc commented May 25, 2025

Uh oh!

Mic92 commented May 25, 2025

Uh oh!

Uh oh!

grahamc commented Apr 29, 2025 •

edited

Loading

Mic92 commented Apr 29, 2025 •

edited

Loading

grahamc commented May 20, 2025 •

edited

Loading

Mic92 commented May 20, 2025 •

edited

Loading

Mic92 commented May 21, 2025 •

edited

Loading

Mic92 commented May 25, 2025 •

edited

Loading