Development shell with a pinned nixfmt #322512

infinisil · 2024-06-25T23:25:14Z

Description of changes

This is a decently-sized step towards having a fully nixfmt-formatted Nixpkgs (#322520) 🎉. I'd like others in the @NixOS/nix-formatting to approve this.

This PR makes it such that the existing workflow to check Nix formatting gives clear instructions on how to fix unformatted files.

It does so by introducing a shell.nix file for Nixpkgs that provides users with a pinned nixfmt version that matches CI.

Along with it comes a workflow to check that nix-build shell.nix works on x86_64-linux and aarch64-darwin (the two relevant platforms available in GitHub Actions), such that it doesn't break randomly

This PR introduces 2 new entries in the root of the directory tree (shell.nix and ci for some extra CI bits), exciting!

Things done

Successfully ran nix-shell and nixfmt pkgs/development/cuda-modules in it, a directory that's already enforced to be formatted
Wrote some docs
Tested formatting CI failure: https://github.com/tweag/nixpkgs/actions/runs/9671086737/job/26681017292?pr=95
- And successful: https://github.com/tweag/nixpkgs/actions/runs/9671095373/job/26681045327?pr=95
Tested nix-shell CI: https://github.com/tweag/nixpkgs/actions/runs/9671095369?pr=95
Added myself to codeowners of new files, and the security team for critical ones
Set myself as code owner of the formatter workflow file (can't do the formatting team because it doesn't have write access)

This work is sponsored by Antithesis ✨

Add a 👍 reaction to pull requests you find important.

.github/CODEOWNERS

nixos-discourse · 2024-06-26T01:43:21Z

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

https://discourse.nixos.org/t/formatting-team-meeting-2024-06-25/47661/1

drupol

Cool!

A few nits and it's good for me.

.github/CODEOWNERS

.github/workflows/check-nix-format.yml

.github/workflows/check-shell.yml

ci/README.md

shell.nix

This creates a new directory for all CI support files, will be populated in future commits.

This is needed such that in the next commit, we can re-use the same version from a shell.nix, allowing people to have a guaranteed matching nixfmt version.

This uses the reusable pinned Nixpkgs from the parent commit to create a shell.nix file for an environment with a pinned nixfmt version.

When some files are not formatted properly, this shows how people can fix the problem. This notably uses the shell.nix introduced in the parent commit to ensure that the nixfmt version matches what CI expects.

Otherwise it prints the command to run with an empty entry: nixfmt '<stuff>' '' Thanks to https://yaml-multiline.info/ for the great help with this

infinisil · 2024-06-26T14:24:28Z

Since I don't want this PR to focus on the direnv part, I split that into a separate PR that depends on this one: #322650 (also moved all comments about it there)

nixos-discourse · 2024-06-28T08:34:59Z

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

https://discourse.nixos.org/t/tweag-nix-dev-update-57/47823/1

.github/CODEOWNERS

0x4A6F · 2024-06-29T16:24:58Z

ci/update-pinned-nixpkgs.sh

+# https://stackoverflow.com/a/246128
+SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
+
+repo=https://github.com/nixos/nixpkgs


-repo=https://github.com/nixos/nixpkgs +repo=https://github.com/NixOS/nixpkgs

GitHub also works with the lowercase version, so I don't think this matters nor worth changing, even if only to set a precedent for less bikeshedding :P

infinisil · 2024-06-29T22:00:38Z

Since this has been approved by 3 other members of the formatting team, let's go ahead with merging this ;)

github-actions · 2024-06-29T22:01:18Z

Backport failed for release-24.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.05
git worktree add -d .worktree/backport-322512-to-release-24.05 origin/release-24.05
cd .worktree/backport-322512-to-release-24.05
git switch --create backport-322512-to-release-24.05
git cherry-pick -x 13599930cb314ca9335fbb9dcb31acc4342f748d b33ac05d043c03cf98397e5f026ee5a93b781199 a70ab58960c254c58c5289b18b769c9d2311dce7 53b517c6851ef52a86e44ea16220d93f88bf9849 d0bebb7d6b86a705f943dc926527400d7ebc144b 58f98e4b1bc487a519b0f57e9723eb2ce223c229

nixos-discourse · 2024-07-04T04:51:36Z

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

https://discourse.nixos.org/t/more-official-guidelines-in-regards-to-styling-choices/48407/1

infinisil · 2024-07-12T19:10:09Z

Manual backport of this is in #326630

alyssais · 2024-08-05T10:30:07Z

It's a shame this has broken nix-shell -A $pkg to get an interactive package building environment. Is there any way we could preserve that, without having to add "default.nix" to the end of the command?

infinisil · 2024-08-07T01:41:45Z

@alyssais Oh damn, yeah that's something that didn't occur to me in advance. Looks like nix-shell . -A $pkg works, which seems satisfactory to me. But we could also make nix-shell -A pkgs.$pkg work using passthru.pkgs = import ./. in shell.nix.

alyssais · 2024-08-07T06:18:59Z

Learning to add a . isn't too bad, so I think it would only be worth doing something if it didn't break people's workflows at all, but it's still a shame it broke.

infinisil added the backport release-24.05 Backport PR automatically label Jun 25, 2024

github-actions bot added the 6.topic: policy discussion label Jun 25, 2024

infinisil mentioned this pull request Jun 25, 2024

Formatting CI test tweag/nixpkgs#95

Closed

infinisil force-pushed the pinned-nixfmt branch 3 times, most recently from 9ca305f to 65bc415 Compare June 25, 2024 23:44

infinisil mentioned this pull request Jun 26, 2024

Reformat Nixpkgs #322520

Open

7 tasks

infinisil marked this pull request as ready for review June 25, 2024 23:46

infinisil requested review from Mic92, zowoq and a team as code owners June 25, 2024 23:46

infinisil force-pushed the pinned-nixfmt branch 3 times, most recently from db1efeb to 8f2e12e Compare June 25, 2024 23:54

mweinelt reviewed Jun 26, 2024

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

infinisil force-pushed the pinned-nixfmt branch from ac0fa99 to eb96b5b Compare June 26, 2024 00:20

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Jun 26, 2024

infinisil mentioned this pull request Jun 26, 2024

Treewide Nix reformat pass 1 [skip treewide] #322537

Draft

10 tasks

This comment was marked as outdated.

Sign in to view

drupol reviewed Jun 26, 2024

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

.github/workflows/check-nix-format.yml Outdated Show resolved Hide resolved

.github/workflows/check-shell.yml Outdated Show resolved Hide resolved

ci/README.md Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

Mic92 reviewed Jun 26, 2024

View reviewed changes

shell.nix Outdated Show resolved Hide resolved

infinisil added 4 commits June 26, 2024 16:04

ci: init directory

1359993

This creates a new directory for all CI support files, will be populated in future commits.

ci: reusable Nix format Nixpkgs pin

b33ac05

This is needed such that in the next commit, we can re-use the same version from a shell.nix, allowing people to have a guaranteed matching nixfmt version.

root: shell.nix for nixfmt

a70ab58

This uses the reusable pinned Nixpkgs from the parent commit to create a shell.nix file for an environment with a pinned nixfmt version.

workflows/check-nix-format: show command to fix

53b517c

When some files are not formatted properly, this shows how people can fix the problem. This notably uses the shell.nix introduced in the parent commit to ensure that the nixfmt version matches what CI expects.

infinisil added 2 commits June 26, 2024 16:16

workflows/check-nix-format: strip newline from file listings

d0bebb7

Otherwise it prints the command to run with an empty entry: nixfmt '<stuff>' '' Thanks to https://yaml-multiline.info/ for the great help with this

CODEOWNERS: set myself as code owner of workflows/check-nix-format

58f98e4

infinisil force-pushed the pinned-nixfmt branch 2 times, most recently from 0d74a6b to 58f98e4 Compare June 26, 2024 14:18

infinisil mentioned this pull request Jun 26, 2024

Direnv support #322650

Merged

1 task

tomberek approved these changes Jun 28, 2024

View reviewed changes

dasJ approved these changes Jun 29, 2024

View reviewed changes

Frontear reviewed Jun 29, 2024

View reviewed changes

.github/CODEOWNERS Show resolved Hide resolved

Frontear approved these changes Jun 29, 2024

View reviewed changes

0x4A6F approved these changes Jun 29, 2024

View reviewed changes

infinisil merged commit 81a7752 into NixOS:master Jun 29, 2024
29 checks passed

infinisil deleted the pinned-nixfmt branch June 29, 2024 22:00

infinisil mentioned this pull request Jul 8, 2024

flake: configure nix fmt #325575

Closed

13 tasks

toastal mentioned this pull request Jul 9, 2024

Remove .envrc #325793

Merged

13 tasks

This was referenced Jul 12, 2024

[24.05] maintainer-list: Fix formatting #326628

Merged

[Backport release-24.05] Development shell with a pinned nixfmt #326630

Merged

Scrumplex mentioned this pull request Jul 13, 2024

root: add nixpkgs-review to shell.nix #326832

Merged

13 tasks

MattSturgeon mentioned this pull request Jul 25, 2024

Enforce nixfmt on new files and changed files that were already formatted #326407

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Development shell with a pinned nixfmt #322512

Development shell with a pinned nixfmt #322512

infinisil commented Jun 25, 2024 •

edited

Loading

nixos-discourse commented Jun 26, 2024

This comment was marked as outdated.

drupol left a comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

infinisil commented Jun 26, 2024 •

edited

Loading

nixos-discourse commented Jun 28, 2024

0x4A6F Jun 29, 2024

infinisil Jun 29, 2024

infinisil commented Jun 29, 2024

github-actions bot commented Jun 29, 2024

nixos-discourse commented Jul 4, 2024

infinisil commented Jul 12, 2024

alyssais commented Aug 5, 2024 •

edited

Loading

infinisil commented Aug 7, 2024

alyssais commented Aug 7, 2024

Development shell with a pinned nixfmt #322512

Development shell with a pinned nixfmt #322512

Conversation

infinisil commented Jun 25, 2024 • edited Loading

Description of changes

Things done

nixos-discourse commented Jun 26, 2024

This comment was marked as outdated.

drupol left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

infinisil commented Jun 26, 2024 • edited Loading

nixos-discourse commented Jun 28, 2024

0x4A6F Jun 29, 2024

Choose a reason for hiding this comment

infinisil Jun 29, 2024

Choose a reason for hiding this comment

infinisil commented Jun 29, 2024

github-actions bot commented Jun 29, 2024

nixos-discourse commented Jul 4, 2024

infinisil commented Jul 12, 2024

alyssais commented Aug 5, 2024 • edited Loading

infinisil commented Aug 7, 2024

alyssais commented Aug 7, 2024

infinisil commented Jun 25, 2024 •

edited

Loading

infinisil commented Jun 26, 2024 •

edited

Loading

alyssais commented Aug 5, 2024 •

edited

Loading