treewide: migrate to lib.cmakeBool and lib.cmakeFeature#386865
Closed
pbsds wants to merge 1 commit intoNixOS:stagingfrom
Closed
treewide: migrate to lib.cmakeBool and lib.cmakeFeature#386865pbsds wants to merge 1 commit intoNixOS:stagingfrom
pbsds wants to merge 1 commit intoNixOS:stagingfrom
Conversation
github-actions
bot
added
6.topic: python
paparodeo
reviewed
Mar 4, 2025
Contributor
There was a problem hiding this comment.
not sure if this is an unpopular opinion but I don't see any benefit of the lib.cmakeXXX outside of when using a flag to toggle a boolean value.
"-DADLplug_USE_SYSTEM_FMT=ON"
is pretty self explanatory and really nothing is gained other than character count when calling the lib function.
(lib.cmakeBool "ADLplug_USE_SYSTEM_FMT" true)
the downside is possible mistakes in the script and making it harder to track changes using git blame.
I did this using bash not because should've, but because I could.
Candidates were made with
```shell
set -euo pipefail
N_WORKERS=4
NIX_BUILD_ARGS=( -j1 ) # remote builders to brrr
export GIT_FLOCK="$(mktemp)"
git-wait() {
flock "$GIT_FLOCK" --command "git $(printf " %q" "$@")"
}
available_platforms=(
x86_64-linux
# aarch64-linux
# x86_64-darwin
# aarch64-darwin
)
get_cmake_config_drv_path() {
local system="$1"
local attrpath="$2"
nix eval --impure --system "$system" -f default.nix "$attrpath" --apply "$(cat <<"EOF"
x: (x.overrideAttrs (old: {
phases = [ "unpackPhase" "patchPhase" "configurePhase" "installPhase" ];
doBuild = false;
doCheck = false;
doInstallCheck = false;
NIX_OUTPATH_USED_AS_RANDOM_SEED = "/nix/store/nj1na0qwqhpd128vr71p70hz9jyhnz5x";
installPhase = ''
shopt -s nullglob
# remove external cached variables, essentially those from cmakeFlags, found between:
# ########################
# # EXTERNAL cache entries
# ########################
# ...
# ########################
# # INTERNAL cache entries
# ########################
test -s ./CMakeCache.txt && \
sed -i '/^# EXTERNAL cache entries$/,/^# INTERNAL cache entries$/d' CMakeCache.txt
# Cmake does not order makefile targets deterministically
for fname in **/Makefile **/Makefile2; do
data="$(cat "$fname")"
sort <<<"$data" >$fname
done
# cat all configure files to $out, retain filenames
find . -type f -print0 | sort --zero-terminated | xargs -0 --max-args=100 tail -n+1 \
| sed -E 's@/nix/store/[^ /]+@@g' \
>$out
# Remove randomness from output.
# I love how "deterministic builds" only care about the output
# artifacts and not the intermediate build configuration artifacts.
# The alternative to this find-replace madness would be to make mkdtemp(3) deterministic
# https://github.com/Kitware/CMake/blob/3bdf63e84db42e63e4ce1b364d6274ef401009d4/Source/cmCoreTryCompile.cxx#L503-L505
# https://github.com/Kitware/CMake/blob/3bdf63e84db42e63e4ce1b364d6274ef401009d4/Source/cmSystemTools.cxx#L1449
sed_args=$(printf ' -e s@%q@xxxxx@g' $({
strings $out | grep -oE '/build/[A-Za-z0-9]+\.(s|res)' | cut -d/ -f3 | cut -d. -f1 | grep -vE '(CMAKE|CMake|Linking)' | grep -E '^.{6}.*$'
strings $out | grep -oE '/TryCompile-[A-Za-z0-9]+' | cut -d- -f2
strings $out | grep -oE 'CMakeFiles/cmTC_[A-Za-z0-9]+\.dir' | cut -d/ -f2 | cut -d. -f1
} | sort -u))
sed -i $out $sed_args
# echo "sed_args=$sed_args" >>$out
'';
})).drvPath
EOF
)" --raw
}
rewrite_cmake_flags() {
for fname in "$@"; do
sd '"-D([A-Za-z0-9_-]+)(?::STRING)?=((?:\$\{[^}]*\}|\\.|[^\\"$])*)"' '(lib.cmakeFeature "$1" "$2")' "$fname" # "-Dfoo=bar" -> (cmakeFeature "foo" "bar")
sd '"-D([A-Za-z0-9_-]+):(?:BOOL|bool)=(?:ON|On|on|TRUE|True|true)"' '(lib.cmakeBool "$1" true)' "$fname" # "-Dfoo:bool=on" -> (cmakeBool "foo" true)
sd '"-D([A-Za-z0-9_-]+):(?:BOOL|bool)="(?:OFF|Off|off|FALSE|False|false)"\)"' '(lib.cmakeBool "$1" true)' "$fname" # "-Dfoo:bool=off" -> (cmakeBool "foo" false)
sd '\(lib\.cmakeFeature "([A-Za-z0-9_-]+)" "\\"((?:\$\{[^}]*\}|\\.|[^ \\"$])*)\\""\)' '(lib.cmakeFeature "$1" "$2")' "$fname" # "\"foo\"" -> "foo"
sd '\(lib\.cmakeFeature "([A-Za-z0-9_-]+)" "(?:ON|On|on|TRUE|True|true)"\)' '(lib.cmakeBool "$1" true)' "$fname" # cmakeFeature -> cmakeBool
sd '\(lib\.cmakeFeature "([A-Za-z0-9_-]+)" "(?:OFF|Off|off|FALSE|False|false)"\)' '(lib.cmakeBool "$1" false)' "$fname" # cmakeFeature -> cmakeBool
sd '\(lib\.cmakeFeature "([A-Za-z0-9_-]+)" "\$\{([^}! ]*)\}"\)' '(lib.cmakeFeature "$1" $2)' "$fname" # "${foobar}" -> foobar
sd '\(lib\.cmakeFeature "([A-Za-z0-9_-]+)" "\$\{([^}]*)\}"\)' '(lib.cmakeFeature "$1" ($2))' "$fname" # "${foo bar}" -> (foo bar)
# (lib.cmakeFeature foo (if bar then "ON" else "OFF")) -> (lib.cmakeBool foo bar)
sd '\(lib\.cmakeFeature "([A-Za-z0-9_-]+)" \(if (.*) then "(?:ON|on|TRUE|True|true)" else "(?:OFF|Off|off|FALSE|False|false)"\)\)' '(lib.cmakeBool "$1" ($2))' "$fname"
sd '\(lib\.cmakeFeature "([A-Za-z0-9_-]+)" \(if (.*) then "(?:OFF|off|FALSE|False|false)" else "(?:ON|On|on|TRUE|True|true)"\)\)' '(lib.cmakeBool "$1" (!($2)))' "$fname"
# ensure first `lib.optional (lib.cmakeBool ...)` is prefixed with `++`
sd '^( *)cmakeFlags *=[ \n]*lib\.optional ' '${1}cmakeFlags = [\n$1 ]\n$1 ++ lib.optional ' "$fname"
# lib.optional foo (lib.cmakeBool "asd" true) -> lib.cmakeBool "asd" foo
sd ' \+\+ lib\.optional ([a-zA-Z0-9.]+) \(lib.cmakeBool "([A-Za-z0-9_-]+)" true\)' ' ++ [ (lib.cmakeBool "$2" $1) ]' "$fname"
sd ' \+\+ lib\.optional ([a-zA-Z0-9.]+) \(lib.cmakeBool "([A-Za-z0-9_-]+)" false\)' ' ++ [ (lib.cmakeBool "$2" (!$1)) ]' "$fname"
sd ' \+\+ lib\.optional \(([^)]+)\) \(lib.cmakeBool "([A-Za-z0-9_-]+)" true\)' ' ++ [ (lib.cmakeBool "$2" ($1)) ]' "$fname"
sd ' \+\+ lib\.optional \(([^)]+)\) \(lib.cmakeBool "([A-Za-z0-9_-]+)" false\)' ' ++ [ (lib.cmakeBool "$2" (!($1))) ]' "$fname"
# joining logic
sd '^( *)\+\+ \[ \(lib.cmakeBool "([A-Za-z0-9_-]+)" (.*)\) \]' '$1++ [\n$1 (lib.cmakeBool "$2" (!($3)))\n$1]' "$fname"
# sd '^( *)([^ ].*)( \+\+ lib\.optional ([a-zA-Z0-9.]+|\(([^)]+)\)) \(lib.cmakeBool "[A-Za-z0-9_-]+" .*\))' '${1}${2}\n ${1} $3' "$fname"
# split `] ++` into two lines
sd '^( *)\] \+\+ ' '$1]\n$1++ ' "$fname"
# strip `]\n++ [`
sd '\n( *)\]\n *\+\+ \[$' '' "$fname"
# undo ++ prefixing for first `lib.optional (lib.cmakeBool ...)`
sd '^( *)cmakeFlags = \[\n *\]\n *\+\+ lib\.optional ' '${1}cmakeFlags =\n${1} lib.optional ' "$fname"
# simplify bool expressions
# sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(!!(.*)\)\)' '(lib.cmakeBool "$1" ($2))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(!\(\((.*)\)\)\)\)' '(lib.cmakeBool "$1" (!($2)))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(!\(!(.*)\)\)\)' '(lib.cmakeBool "$1" ($2))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(\((.*)\)\)' '(lib.cmakeBool "$1" ($2))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(!(.*) == (.*)\)\)' '(lib.cmakeBool "$1" ($2 != $3))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(!(.*) != (.*)\)\)' '(lib.cmakeBool "$1" ($2 == $3))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \((.*) == false\)\)' '(lib.cmakeBool "$1" (!($2)))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \((.*) != false\)\)' '(lib.cmakeBool "$1" ($2))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \((.*) == true\)\)' '(lib.cmakeBool "$1" ($2))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \((.*) != true\)\)' '(lib.cmakeBool "$1" (!($2)))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(!\(!(.*)\)\)\)' '(lib.cmakeBool "$1" ($2))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(\((.*)\)\)' '(lib.cmakeBool "$1" ($2))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(!\(([A-Za-z0-9._-]+)\)\)\)' '(lib.cmakeBool "$1" (!$2))' "$fname"
sd '\(lib.cmakeBool "([A-Za-z0-9_-]+)" \(([A-Za-z0-9._-]+)\)\)' '(lib.cmakeBool "$1" $2)' "$fname"
done
}
list_attrpath_fname_col() {
test -s packages.json || ( set -x;
time nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f ./. -qaP --json --meta --show-trace --no-allow-import-from-derivation > packages.json
)
jq <packages.json 'to_entries[] | select(.value.meta.position==null|not) | "\(.key)\t\(.value.meta.position)"' -r \
| sed -e "s#\t$(realpath .)/#\t#" \
| sed -e 's#:\([0-9]*\)$#\t\1#' \
| tr '0123456789' '9876543210' | sort | tr '0123456789' '9876543210' \
| grep . \
| grep -iv haskell \
| grep -iv /top-level/ \
| grep -iv chicken \
| grep -iv build \
| grep -E '/(package|default)\.nix'
}
git-wait restore .
rm -r results-configure || true
mkdir -p results-configure
flockdir=$(mktemp -d)
declare -i COUNT=0
while read attrpath fname col; do
grep -q cmakeFlags "$fname" || continue
grep -q -E '(lib\.cmakeFeature|"-D[A-Za-z0-9_-]+(:[A-Za-z]+)?=((\$\{[^}]*\}|[^"$])*)")' "$fname" || continue
grep -q mesonFlags "$fname" && continue || true
echo "$attrpath"
echo | (
# mutex on fname
flock --nonblock 200 || {
>&2 echo "failed to aquire lock for $fname"
exit 1
}
was_formatted=$(nixfmt --check "$fname" >&/dev/null && echo true || echo false)
config_pre_drv=()
for system in "${available_platforms[@]}"; do
config_pre_drv+=("$(get_cmake_config_drv_path "$system" "$attrpath")") || true
done
[[ -n "${config_pre_drv[*]}" ]] || exit
rewrite_cmake_flags "$fname"
if git-wait diff --exit-code --quiet "$fname"; then
exit
fi
abort() {
git-wait restore "$fname"
exit 1
}
trap 'git-wait restore "$fname"' ERR EXIT
if $was_formatted; then
nixfmt "$fname" || abort # syntax error
else
nixfmt --verify "$fname" || abort # syntax error
fi
config_post_drv=()
for system in "${available_platforms[@]}"; do
config_post_drv+=("$(get_cmake_config_drv_path "$system" "$attrpath")") || true
done
[[ -n "${config_post_drv[*]}" ]] || abort
if [[ "${config_pre_drv[*]}" = "${config_post_drv[*]}" ]]; then
# unchanged eval
git-wait add "$fname"
else
# config_pre="$( nix-build "${NIX_BUILD_ARGS[@]}" "${config_pre_drv[@]}" --no-out-link)"
# config_post="$(nix-build "${NIX_BUILD_ARGS[@]}" "${config_post_drv[@]}" --no-out-link)"
mkdir -p results-configure/"$attrpath"
config_pre="$( nix-build "${NIX_BUILD_ARGS[@]}" "${config_pre_drv[@]}" -o results-configure/"$attrpath"/pre )" || abort
config_post="$(nix-build "${NIX_BUILD_ARGS[@]}" "${config_post_drv[@]}" -o results-configure/"$attrpath"/post)" || abort
if diff -q <(cat $config_pre) <(cat $config_post) ; then
# unchanged configuration artifacts
git-wait add "$fname"
else
diff -u <(cat $config_pre) <(cat $config_post) | delta --paging never
git-wait restore "$fname"
fi
fi
) 200>"$flockdir"/"$(sha256sum - <<<"$fname" | cut -d' ' -f1)".lock &
while [[ $(jobs -p | wc -l) -ge $N_WORKERS ]]; do
wait -n < <(jobs -p) || true
done
done < <(list_attrpath_fname_col)
wait
```
And picked with `--patch`, filtering for changes that involve logic in lib.cmakeBool.
I then compared `packages.json` from `nix-env ... --meta` to check for eval errors: this is the diff: https://gist.github.com/pbsds/631d39494827f121a0b170e85636a356
pbsds
force-pushed
the
migrate-cmake-flags-1739919000
branch
from
496bcdf to
79ff6f4
Compare
github-actions
bot
removed
the
6.topic: python
Member
Author
|
Sure, i filtered for cases where cmakeBool are not true or false, the new diff went from 220 affected files to 24 |
github-actions
bot
added
10.rebuild-darwin: 1001-2500
Member
Author
|
13 tasks
pbsds
added a commit
to pbsds/nixpkgs
that referenced
this pull request
Mar 11, 2025
Inspired by NixOS#387725 (comment), script is based on NixOS#336172 using what i learned in NixOS#386865, part of NixOS#346453 Should be zero rebuilds. All candidates were made using: ```shell export NIXPKGS_ALLOW_UNFREE=1 export NIXPKGS_ALLOW_INSECURE=1 export NIXPKGS_ALLOW_BROKEN=1 git-wait restore . test -s packages.json || ( set -x; time nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f ./. -qaP --json --meta --drv-path --out-path --show-trace --no-allow-import-from-derivation --arg config '{ allowAliases = false; }' > packages.json ) list_attrpath_fname_col() { jq <packages.json 'to_entries[] | select(.value.meta.position==null|not) | "\(.key)\t\(.value.meta.position)"' -r | sed -e "s#\t$(realpath .)/#\t#" | sed -e 's#:\([0-9]*\)$#\t\1#' | grep . | grep -iv haskell | grep -iv /top-level/ | grep -iv chicken | grep pkgs/by-name/ | grep -iv build | grep -E '/(package|default)\.nix' } FLOCKDIR="$(mktemp -d)" N_WORKERS=4 while read attrpath fname col; do grep -qE 'repo *= *("\$\{pname\}"|pname);' "$fname" || continue echo | ( # mutex on fname flock --nonblock 200 || { >&2 echo "failed to aquire lock for $fname" exit 1 } echo "$attrpath" data="$(nix eval --impure --expr 'with import ./. {}; { inherit ('"$attrpath"') pname drvPath passthru meta; drvPath2='"$attrpath"'.src.drvPath; }' --json)" || exit test -n "$data" || exit pname="$(jq <<<"$data" .pname -r)" test -n "$pname" || exit (set -x sd -F '${pname}' "$pname" "$fname" sd -F ' = pname;' " = \"$pname\";" "$fname" ) data2="$(nix eval --impure --expr 'with import ./. {}; { inherit ('"$attrpath"') pname drvPath passthru meta; drvPath2='"$attrpath"'.src.drvPath; }' --json)" if [[ "$data" = "$data2" ]]; then (set -x; git-wait add "$fname") else (set -x; git-wait restore "$fname") exit fi (set -x sd -F ' rec {' ' {' "$fname" ) data3="$(nix eval --impure --expr 'with import ./. {}; { inherit ('"$attrpath"') pname drvPath passthru meta; drvPath2='"$attrpath"'.src.drvPath; }' --json 2>/dev/nul)" if [[ "$data" = "$data3" ]]; then (set -x; git-wait add "$fname") else (set -x; git-wait restore "$fname") fi ) 200>"$FLOCKDIR"/"$(sha256sum - <<<"$fname" | cut -d' ' -f1)".lock & while [[ $(jobs -p | wc -l) -ge $N_WORKERS ]]; do wait -n < <(jobs -p) || true done done < <(list_attrpath_fname_col) wait git restore . time nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f ./. -qaP --json --meta --drv-path --out-path --show-trace --no-allow-import-from-derivation --arg config '{ allowAliases = false; }' > packages2.json ``` `diff packages{,2}.json` is empty, indicating that no package nor src derivation has changed. I checked and cherry-picked the changes using `GIT_DIFF_OPTS='-u15' git -c interactive.singleKey=true add --patch`
sandptel
pushed a commit
to sandptel/nixpkgs
that referenced
this pull request
Mar 13, 2025
Inspired by NixOS#387725 (comment), script is based on NixOS#336172 using what i learned in NixOS#386865, part of NixOS#346453 Should be zero rebuilds. All candidates were made using: ```shell export NIXPKGS_ALLOW_UNFREE=1 export NIXPKGS_ALLOW_INSECURE=1 export NIXPKGS_ALLOW_BROKEN=1 git-wait restore . test -s packages.json || ( set -x; time nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f ./. -qaP --json --meta --drv-path --out-path --show-trace --no-allow-import-from-derivation --arg config '{ allowAliases = false; }' > packages.json ) list_attrpath_fname_col() { jq <packages.json 'to_entries[] | select(.value.meta.position==null|not) | "\(.key)\t\(.value.meta.position)"' -r | sed -e "s#\t$(realpath .)/#\t#" | sed -e 's#:\([0-9]*\)$#\t\1#' | grep . | grep -iv haskell | grep -iv /top-level/ | grep -iv chicken | grep pkgs/by-name/ | grep -iv build | grep -E '/(package|default)\.nix' } FLOCKDIR="$(mktemp -d)" N_WORKERS=4 while read attrpath fname col; do grep -qE 'repo *= *("\$\{pname\}"|pname);' "$fname" || continue echo | ( # mutex on fname flock --nonblock 200 || { >&2 echo "failed to aquire lock for $fname" exit 1 } echo "$attrpath" data="$(nix eval --impure --expr 'with import ./. {}; { inherit ('"$attrpath"') pname drvPath passthru meta; drvPath2='"$attrpath"'.src.drvPath; }' --json)" || exit test -n "$data" || exit pname="$(jq <<<"$data" .pname -r)" test -n "$pname" || exit (set -x sd -F '${pname}' "$pname" "$fname" sd -F ' = pname;' " = \"$pname\";" "$fname" ) data2="$(nix eval --impure --expr 'with import ./. {}; { inherit ('"$attrpath"') pname drvPath passthru meta; drvPath2='"$attrpath"'.src.drvPath; }' --json)" if [[ "$data" = "$data2" ]]; then (set -x; git-wait add "$fname") else (set -x; git-wait restore "$fname") exit fi (set -x sd -F ' rec {' ' {' "$fname" ) data3="$(nix eval --impure --expr 'with import ./. {}; { inherit ('"$attrpath"') pname drvPath passthru meta; drvPath2='"$attrpath"'.src.drvPath; }' --json 2>/dev/nul)" if [[ "$data" = "$data3" ]]; then (set -x; git-wait add "$fname") else (set -x; git-wait restore "$fname") fi ) 200>"$FLOCKDIR"/"$(sha256sum - <<<"$fname" | cut -d' ' -f1)".lock & while [[ $(jobs -p | wc -l) -ge $N_WORKERS ]]; do wait -n < <(jobs -p) || true done done < <(list_attrpath_fname_col) wait git restore . time nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f ./. -qaP --json --meta --drv-path --out-path --show-trace --no-allow-import-from-derivation --arg config '{ allowAliases = false; }' > packages2.json ``` `diff packages{,2}.json` is empty, indicating that no package nor src derivation has changed. I checked and cherry-picked the changes using `GIT_DIFF_OPTS='-u15' git -c interactive.singleKey=true add --patch`
wegank
added
the
2.status: merge conflict
Contributor
|
I'm back on nixpkgs now Peder 🙉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I did this using bash not because I should've, but because I could. Next time however i think i'll reach for tree-sitter. Somewhere between the multiprocessing concurrency limiting, all the mutex logic, boolean expression rewriting using regex and making cmake configurations deterministic i started regretting my choices.
Candidates were made with
And picked with
--patch.I then compared
packages.jsonfromnix-env ... --metato check for eval errors: this is the diff: https://gist.github.com/pbsds/631d39494827f121a0b170e85636a356Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.