treewide: CUDA reformat with nixfmt-rfc-style

[ConnorBaker]

Description of changes

Reformats the following paths with nixfmt-rfc-style:

• pkgs/development/cuda-modules
• pkgs/test/cuda
• pkgs/top-level/cuda-packages.nix

Additionally, adds the commit in which the reformatting happens to .git-blame-ignore-revs.

Ideally this will not cause any rebuilds, but it may if whitespace in raw text blocks change.

This PR also introduces a workflow to ensure that the changed files continue to stay formatted.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[ConnorBaker]

PR="299578"; \
SYSTEM="x86_64-linux"; \
CUDA_SUPPORT="true"; \
CUDA_CAPABILITIES='[ "7.5" ]'; \
nixpkgs-review pr "$PR" \
  --system "$SYSTEM" \
  --no-shell \
  --checkout commit \
  --allow aliases \
  --build-args "--max-jobs 1" \
  --skip-package-regex 'python3(\d){1,2}Packages\.(pytorch-pfn-extras|ffcv)' \
  --skip-package-regex 'python3(\d){1,2}Packages\.(shap|mlflow|optuna)' \
  --extra-nixpkgs-config "{
    allowUnfree = true;
    allowBroken = false;
    cudaSupport = ${CUDA_SUPPORT:-false};
    cudaCapabilities = ${CUDA_CAPABILITIES:-[]};
  }"

Result of nixpkgs-review pr 299578 --extra-nixpkgs-config '{ allowUnfree = true; allowBroken = false; cudaSupport = true; cudaCapabilities = [ "7.5" ]; }' run on x86_64-linux 1

3 packages built:

• cudaPackages.saxpy
• cudaPackagesGoogle.saxpy
• cudaPackages_10.saxpy

[ConnorBaker]

PR="299578"; \
SYSTEM="aarch64-darwin"; \
CUDA_SUPPORT="false"; \
CUDA_CAPABILITIES='[ ]'; \
nixpkgs-review pr "$PR" \
  --system "$SYSTEM" \
  --no-shell \
  --checkout commit \
  --allow aliases \
  --build-args "--max-jobs 1" \
  --extra-nixpkgs-config "{
    allowUnfree = true;
    allowBroken = false;
    cudaSupport = ${CUDA_SUPPORT:-false};
    cudaCapabilities = ${CUDA_CAPABILITIES:-[]};
  }"

Result of nixpkgs-review pr 299578 --extra-nixpkgs-config '{ allowUnfree = true; allowBroken = false; cudaSupport = false; cudaCapabilities = [ ]; }' run on aarch64-darwin 1

[ConnorBaker]

PR="299578"; \
SYSTEM="x86_64-darwin"; \
CUDA_SUPPORT="false"; \
CUDA_CAPABILITIES='[ ]'; \
nixpkgs-review pr "$PR" \
  --system "$SYSTEM" \
  --no-shell \
  --checkout commit \
  --allow aliases \
  --build-args "--max-jobs 1" \
  --extra-nixpkgs-config "{
    allowUnfree = true;
    allowBroken = false;
    cudaSupport = ${CUDA_SUPPORT:-false};
    cudaCapabilities = ${CUDA_CAPABILITIES:-[]};
  }"

Result of nixpkgs-review pr 299578 --extra-nixpkgs-config '{ allowUnfree = true; allowBroken = false; cudaSupport = false; cudaCapabilities = [ ]; }' run on x86_64-darwin 1

[ConnorBaker]

PR="299578"; \
SYSTEM="aarch64-linux"; \
CUDA_SUPPORT="true"; \
CUDA_CAPABILITIES='[ "7.2" ]'; \
nixpkgs-review pr "$PR" \
  --system "$SYSTEM" \
  --no-shell \
  --checkout commit \
  --allow aliases \
  --build-args "--max-jobs 4" \
  --extra-nixpkgs-config "{
    allowUnfree = true;
    allowBroken = false;
    cudaSupport = ${CUDA_SUPPORT:-false};
    cudaCapabilities = ${CUDA_CAPABILITIES:-[]};
  }"

Result of nixpkgs-review pr 299578 --extra-nixpkgs-config '{ allowUnfree = true; allowBroken = false; cudaSupport = true; cudaCapabilities = [ "7.2" ]; }' run on aarch64-linux 1

[ConnorBaker]

PR="299578"; \
SYSTEM="aarch64-linux"; \
CUDA_SUPPORT="true"; \
CUDA_CAPABILITIES='[ "7.5" ]'; \
nixpkgs-review pr "$PR" \
  --system "$SYSTEM" \
  --no-shell \
  --checkout commit \
  --allow aliases \
  --build-args "--max-jobs 4" \
  --extra-nixpkgs-config "{
    allowUnfree = true;
    allowBroken = false;
    cudaSupport = ${CUDA_SUPPORT:-false};
    cudaCapabilities = ${CUDA_CAPABILITIES:-[]};
  }"

Result of nixpkgs-review pr 299578 --extra-nixpkgs-config '{ allowUnfree = true; allowBroken = false; cudaSupport = true; cudaCapabilities = [ "7.5" ]; }' run on aarch64-linux 1

2 packages built:

• cudaPackages.saxpy
• cudaPackagesGoogle.saxpy

[samuela]

Thanks for doing this @ConnorBaker ! I'm a fan of all things automated formatting.

How will we enforce this formatting standard in future PRs? Once things are all pretty and pristine after this PR, what's to stop someone from unknowingly breaking formatting in a subsequent PR?

[ConnorBaker]

How will we enforce this formatting standard in future PRs? Once things are all pretty and pristine after this PR, what's to stop someone from unknowingly breaking formatting in a subsequent PR?

I don't know :(

As far as I can tell, we don't have any sort of pre-commit hook setup with Nixpkgs. I suppose I could add a new workflow which checks to make sure that the paths formatted by this PR are unchanged when the formatter is run again, but I'm not sure if that's something which is allowed.

Any ideas?

[samuela]

I suppose I could add a new workflow which checks to make sure that the paths formatted by this PR are unchanged when the formatter is run again, but I'm not sure if that's something which is allowed.

A GitHub Actions workflow? Yeah I think that makes sense. IIUC there's a --verify option that we can make use of.

Perhaps the NixOS/rfcs#166 authors (cc @piegamesde @infinisil) might have input on how to enforce the new formatting standard in CI?

[ConnorBaker]

@samuela see b6024fa -- I use the --check argument so the command should fail with an error if a file isn't formatted.

[samuela]

@samuela see b6024fa -- I use the --check argument so the command should fail with an error if a file isn't formatted.

Woohoo! Looks great!

[infinisil]

This is a bit early, because nixfmt-rfc-style is not yet stable! We still need to release the first official version, and we planned it to only apply to Nixpkgs after that, see NixOS/nixfmt#153.

That said, I don't see any major problems with code owners manually starting to enforce even the unstable format.

[piegamesde]

Am I seeing correctly that the reformat is causing a rebuild? If so, would you please investigate this and open a bug report? Nixfmt should never change the code's semantics, and thus never cause any rebuilds.

[infinisil]

Oh and with a more generic check, the @NixOS/nix-formatting team should be added as the workflows code owner

[gabyx]

I suppose I could add a new workflow which checks to make sure that the paths formatted by this PR are unchanged when the formatter is run again, but I'm not sure if that's something which is allowed.

A GitHub Actions workflow? Yeah I think that makes sense. IIUC there's a --verify option that we can make use of.

Perhaps the NixOS/rfcs#166 authors (cc @piegamesde @infinisil) might have input on how to enforce the new formatting standard in CI?

I would make a GIthub action workflow to just run the changed files in the PR through the formatter and if any diff arises after the run -> report to the PR and block it from merging (basically happens directly because this workflow would be unsuccessful).

[samuela]

if any diff arises after the run -> report to the PR and block it from merging

Is this not the behavior of nixfmt --check?

[ConnorBaker]

The only thing is that I wouldn't want people to add additional GitHub workflows for each part that starts using the unstable format, so instead I'd say let's not imply that the GitHub action is specific to CUDA, and instead name it something generic like check-nix-formatting.yml

Also please add an explanatory comment in this file to say that it's not stable yet with a link to NixOS/rfcs#166

@infinisil I've renamed the file and added a comment to make it clear its opt-in.

Do we want to run the check for all PRs at this point? Maybe we maintain (in the workflow definition) a list of subsystems that explicitly opt-in into enforcing the experimental style

@SomeoneSerge @piegamesde I've relaxed the run condition so it will trigger on all PRs, but have restricted it to just the relevant CUDA files.

I would make a GIthub action workflow to just run the changed files in the PR through the formatter and if any diff arises after the run -> report to the PR and block it from merging (basically happens directly because this workflow would be unsuccessful).

@gabyx @samuela yes, that is what the --check flag is for. Though, it won't print the diff itself.

Am I seeing correctly that the reformat is causing a rebuild? If so, would you please investigate this and open a bug report? Nixfmt should never change the code's semantics, and thus never cause any rebuilds.

@piegamesde @infinisil I've often wondered about those counts, especially for unfree redistributables. If they're calculated naively (e.g., does Hydra have a copy of the package cached?) then it would make sense there are always "rebuilds"; Hydra doesn't build unfree packages (like anything involving CUDA). That the Darwin count is non-zero makes me chuckle, because that's definitely one platform NVIDIA doesn't support, and one which should never be impacted by our changes (unless we, you know, break evaluation).

Is there an easy way for me to check exactly what would need to be rebuilt, or to further investigate this?

---

I see the current PR as the minimal amount of work necessary to format the portion of the code the @NixOS/cuda-maintainers team works on, and to keep it formatted.

I think it would be a good idea to open an issue on the nixfmt repo to create a list of requirements for a GitHub Action which runs nixfmt on a repository. I think that's a good way to gain further adoption; I know if there were a GitHub action specifically for nixfmt that I'd use it -- especially if it provided additional functionality like producing a diff of the changed files, like @gabyx suggested.

What are y'all's thoughts on something like that? I think the changes I've made to the PR (finding paths from environment variables) make it flexible enough for a small group of maintainers to opt-in prior to tree-wide formatting, but I wouldn't want to extend it further.

Concerns have been addressed; pending another review

[infinisil]

Looking good to me! I opened NixOS/nixfmt#180 to track a generic GitHub workflow file.

We'll discuss this PR in today's formatting team meeting and will most likely merge it then :)

[infinisil]

(discussed in the formatting team meeting)

Can confirm that the workflow works (tweag#88), let's merge!

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2024-04-02/42658/1

[samuela]

Woohoo!! thanks everyone -- excited to finally get some auto-formatting CI in nixpkgs!

[philiptaron]

Thank you all for being the first-out-of-the-gate team to opt in to RFC 166!