From a6cf185bcdaecea78c0ba00892b61328e762a387 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 26 Jun 2024 02:04:25 +0200 Subject: [PATCH 1/3] workflows/check-nix-format: Enforce nixfmt on new/changed files This makes the Nix format workflow check new/changed files instead of just an allowlist. This enforces that all PRs updated after this is merged are required to have fully standard formatted Nix files! --- .github/workflows/check-nix-format.yml | 56 +++++++++----------------- 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/.github/workflows/check-nix-format.yml b/.github/workflows/check-nix-format.yml index fc8711a7cd1bb..bce0d9c9388ac 100644 --- a/.github/workflows/check-nix-format.yml +++ b/.github/workflows/check-nix-format.yml @@ -13,8 +13,16 @@ permissions: jobs: nixos: runs-on: ubuntu-latest - if: github.repository_owner == 'NixOS' + if: "github.repository_owner == 'NixOS' && !contains(github.event.pull_request.title, '[skip treewide]')" steps: + - name: Get list of changed files from PR + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh api \ + repos/NixOS/nixpkgs/pulls/${{github.event.number}}/files --paginate \ + | jq --raw-output '.[] | select(.status != "removed" and (.filename | endswith(".nix"))) | .filename' \ + > "$HOME/changed_files" - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 with: # pull_request_target checks out the base branch by default @@ -34,43 +42,19 @@ jobs: - name: Install nixfmt run: "nix-env -f '' -iAP nixfmt-rfc-style" - name: Check that Nix files are formatted according to the RFC style - # Each environment variable beginning with NIX_FMT_PATHS_ is a list of - # paths to check with nixfmt. - env: - NIX_FMT_PATHS_BSD: pkgs/os-specific/bsd - NIX_FMT_PATHS_MPVSCRIPTS: pkgs/applications/video/mpv/scripts - # Format paths related to the Nixpkgs CUDA ecosystem. - NIX_FMT_PATHS_CUDA: |- - pkgs/development/cuda-modules - pkgs/test/cuda - pkgs/top-level/cuda-packages.nix - NIX_FMT_PATHS_MAINTAINERS: |- - maintainers/maintainer-list.nix - maintainers/team-list.nix - NIX_FMT_PATHS_K3S: |- - nixos/modules/services/cluster/k3s - nixos/tests/k3s - pkgs/applications/networking/cluster/k3s - NIX_FMT_PATHS_VSCODE_EXTS: pkgs/applications/editors/vscode/extensions - NIX_FMT_PATHS_PHP_PACKAGES: pkgs/development/php-packages - NIX_FMT_PATHS_BUILD_SUPPORT_PHP: pkgs/build-support/php - # Iterate over all environment variables beginning with NIX_FMT_PATHS_. run: | - unformattedPaths=() - for env_var in "${!NIX_FMT_PATHS_@}"; do - readarray -t paths <<< "${!env_var}" - if [[ "${paths[*]}" == "" ]]; then - echo "Error: $env_var is empty." - exit 1 + unformattedFiles=() + while IFS= read -r file; do + # TODO: Make this more parallel, perhaps using treefmt, + # but need to make sure that only changed files are checked! + if ! nixfmt --check "$file"; then + unformattedFiles+=("$file") fi - echo "Checking paths: ${paths[@]}" - if ! nixfmt --check "${paths[@]}"; then - unformattedPaths+=("${paths[@]}") - fi - done - if (( "${#unformattedPaths[@]}" > 0 )); then - echo "Some required Nix files are not properly formatted" + done < "$HOME/changed_files" + + if (( "${#unformattedFiles[@]}" > 0 )); then + echo "Some new/changed Nix files are not properly formatted" echo "Please run the following in \`nix-shell\`:" - echo "nixfmt ${unformattedPaths[*]@Q}" + echo "nixfmt ${unformattedFiles[*]@Q}" exit 1 fi From 8f4eca422e21dc29ccb807eb66ba5647f5e57b88 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 17 Jul 2024 07:05:40 +0200 Subject: [PATCH 2/3] workflows/check-nix-format: determine changed files via base commit The next commit will use this to have a simpler change --- .github/workflows/check-nix-format.yml | 48 +++++++++++++++++++------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/.github/workflows/check-nix-format.yml b/.github/workflows/check-nix-format.yml index bce0d9c9388ac..77fcc93819378 100644 --- a/.github/workflows/check-nix-format.yml +++ b/.github/workflows/check-nix-format.yml @@ -7,6 +7,8 @@ name: Check that Nix files are formatted on: pull_request_target: + # See the comment at the same location in ./check-by-name.yml + types: [opened, synchronize, reopened, edited] permissions: contents: read @@ -15,18 +17,19 @@ jobs: runs-on: ubuntu-latest if: "github.repository_owner == 'NixOS' && !contains(github.event.pull_request.title, '[skip treewide]')" steps: - - name: Get list of changed files from PR - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - gh api \ - repos/NixOS/nixpkgs/pulls/${{github.event.number}}/files --paginate \ - | jq --raw-output '.[] | select(.status != "removed" and (.filename | endswith(".nix"))) | .filename' \ - > "$HOME/changed_files" - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 with: # pull_request_target checks out the base branch by default ref: refs/pull/${{ github.event.pull_request.number }}/merge + # Fetches the merge commit and its parents + fetch-depth: 2 + - name: Checking out base branch + run: | + base=$(mktemp -d) + baseRev=$(git rev-parse HEAD^1) + git worktree add "$base" "$baseRev" + echo "baseRev=$baseRev" >> "$GITHUB_ENV" + echo "base=$base" >> "$GITHUB_ENV" - name: Get Nixpkgs revision for nixfmt run: | # pin to a commit from nixpkgs-unstable to avoid e.g. building nixfmt @@ -44,13 +47,32 @@ jobs: - name: Check that Nix files are formatted according to the RFC style run: | unformattedFiles=() - while IFS= read -r file; do - # TODO: Make this more parallel, perhaps using treefmt, - # but need to make sure that only changed files are checked! - if ! nixfmt --check "$file"; then + + # TODO: Make this more parallel + + # Loop through all Nix files touched by the PR + while readarray -d '' -n 2 entry && (( ${#entry[@]} != 0 )); do + type=${entry[0]} + file=${entry[1]} + case $type in + A*) + dest=$file + ;; + M*) + dest=$file + ;; + C*|R*) + read -r -d '' dest + ;; + *) + echo "Ignoring file $file with type $type" + continue + esac + + if ! nixfmt --check "$dest"; then unformattedFiles+=("$file") fi - done < "$HOME/changed_files" + done < <(git diff -z --name-status ${{ env.baseRev }} -- '*.nix') if (( "${#unformattedFiles[@]}" > 0 )); then echo "Some new/changed Nix files are not properly formatted" From 8bbfb0bc8b793fac96dfaa1be93047765d5f12b3 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 17 Jul 2024 07:10:02 +0200 Subject: [PATCH 3/3] workflows/check-nix-format: Only ensure for already formatted files This prevents situations where contributors need to suddenly format a huge file even if they only changed a small part of it (e.g. all-packages.nix) --- .github/workflows/check-nix-format.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/check-nix-format.yml b/.github/workflows/check-nix-format.yml index 77fcc93819378..5395134de09fe 100644 --- a/.github/workflows/check-nix-format.yml +++ b/.github/workflows/check-nix-format.yml @@ -56,12 +56,15 @@ jobs: file=${entry[1]} case $type in A*) + source="" dest=$file ;; M*) + source=$file dest=$file ;; C*|R*) + source=$file read -r -d '' dest ;; *) @@ -69,7 +72,10 @@ jobs: continue esac - if ! nixfmt --check "$dest"; then + # Ignore files that weren't already formatted + if [[ -n "$source" ]] && ! nixfmt --check ${{ env.base }}/"$source" 2>/dev/null; then + echo "Ignoring file $file because it's not formatted in the base commit" + elif ! nixfmt --check "$dest"; then unformattedFiles+=("$file") fi done < <(git diff -z --name-status ${{ env.baseRev }} -- '*.nix')