treewide: add developer shell and pre-commit hooks and simplify GitHub workflows

[trueNAHO]

This patchset closes issue https://github.com/danth/stylix/issues/236 ("treewide: add linters and formatters") after integrating pre-commit hooks into the developer shell and simplifying GitHub workflows using nix flake check.

This patchset is stacked on top of the pending patchset https://github.com/danth/stylix/pull/515 ("treewide: re-add flake-utils dependency and interface flake systems") and should only be merged after it.

[!NOTE]

These commits should be merged individually, not squashed, to ensure a clear and easily reversible changelog. In case of change requests, review the commits separately.

[trueNAHO]

Subject: [PATCH 04/17] stylix: add deadnix and statix pre-commit hooks

[...]

diff --git a/flake.nix b/flake.nix
index a407bb6..e4596c8 100644
--- a/flake.nix
+++ b/flake.nix
@@ -59,6 +59,16 @@
 
     nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable";
 
+    pre-commit-hooks = {
+      inputs = {
+        flake-compat.follows = "flake-compat";
+        nixpkgs-stable.follows = "pre-commit-hooks/nixpkgs";
+        nixpkgs.follows = "nixpkgs";
+      };
+
+      url = "github:cachix/pre-commit-hooks.nix";
+    };
+
     # Interface flake systems.
     systems.url = "github:nix-systems/default";
   };
@@ -70,6 +80,15 @@
         inherit (nixpkgs) lib;
         pkgs = nixpkgs.legacyPackages.${system};
       in {
+        checks.pre-commit-hooks = inputs.pre-commit-hooks.lib.${system}.run {
+          hooks = {
+            deadnix.enable = true;
+            statix.enable = true;
+          };
+
+          src = ./.;
+        };
+
         devShells.default = pkgs.mkShell {
           packages = [ inputs.home-manager.packages.${system}.default ];
         };

All pre-commit-hooks instances should be renamed to git-hooks, following https://github.com/cachix/git-hooks.nix/pull/488.

[trueNAHO]

I will address https://github.com/danth/stylix/pull/519#issuecomment-2307609958 together with any potential change requests.

For reference:

I prefer squashing as the default since it keeps the history cleaner on the master branch

[...] squashing https://github.com/danth/stylix/pull/519 into a single commit would result in a practically incomprehensible and revertible history.

-- https://github.com/danth/stylix/pull/514#issuecomment-2310460991

[trueNAHO]

Subject: [PATCH 07/17] ci: simplify Docs GitHub workflow

Link: https://github.com/danth/stylix/pull/519
Link: https://github.com/trueNAHO/dotfiles/blob/390d6261e7cf6d19a119643da216258c772db881/.github/workflows/docs.yml
---
 .github/workflows/docs.yml | 61 +++++++++++++-------------------------
 1 file changed, 20 insertions(+), 41 deletions(-)

diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
index 3c758152..4d1515e9 100644
--- a/.github/workflows/docs.yml
+++ b/.github/workflows/docs.yml
@@ -1,3 +1,4 @@
+---
 name: Docs

 on:
@@ -5,53 +6,31 @@ on:
     branches:
       - master

-jobs:
-  build:
-    name: Build
+permissions:
+  contents: read
+  id-token: write
+  pages: write

-    permissions:
-      contents: read
+concurrency:
+  cancel-in-progress: true
+  group: pages

+jobs:
+  docs:
     runs-on: ubuntu-22.04

Restrict the workflow permissions to the docs job:

jobs:
  docs:
    runs-on: ubuntu-22.04

    permissions:
      contents: read
      id-token: write
      pages: write

[trueNAHO]

I will address https://github.com/danth/stylix/pull/519#issuecomment-2307609958 together with any potential change requests.

For reference:

I prefer squashing as the default since it keeps the history cleaner on the master branch

[...] squashing https://github.com/danth/stylix/pull/519 into a single commit would result in a practically incomprehensible and revertible history.

-- https://github.com/danth/stylix/pull/514#issuecomment-2310460991

@danth, I don't think it would significantly impact your review, but should I rebase this PR on top of master and resolve the https://github.com/danth/stylix/pull/519#issuecomment-2307609958 and https://github.com/danth/stylix/pull/519#issuecomment-2329091378 nitpicks before you review this patchset?

[trueNAHO]

-      - name: Prepare docs for upload
-        run: cp -r --dereference --no-preserve=mode,ownership result/ public/

Unless the situation has changed since the original workflow was written, I think this copy was necessary due to an error with the files being part of the Nix store. (It was either because they were owned by root, or because there were symlinks involved, I can't remember exactly)

Good catch! Admittedly, I adapted this GitHub Action from my dotfiles setup without testing it. The only difference with my dotfiles setup is:

-path: result/usr/share/doc
+path: result

I assume the GitHub Action does not consider path pointing to a read-only symlink directory. In my dotfiles setup, I do not run into such problems because path points to a regular (not a read-only symlink directory) directory inside result. I use /usr/share/doc because it is a more conventional FHS location for documentation.

Should we apply your cp workaround or move the documentation output to result/usr/share/doc? NOTE: This is unaddressed in v1.

+To automatically enter the developer shell upon entering the project directory
+with [`direnv`](https://direnv.net), run:

For clarity we should mention that direnv needs to be installed beforehand

Alternatively, if you have [`direnv`](https://direnv.net) installed, you may
automatically enter the developer shell upon entering the project directory.
To enable this, run:

I assumed the direnv link made this obvious, but I would not mind adding the note. NOTE: I have not included this in v1.

+          packages = [
+            inputs.home-manager.packages.${system}.default
+            inputs.self.checks.${system}.pre-commit-hooks.enabledPackages
+            pkgs.ghc

I don't think GHC is necessary since the palette generator should be built using nix build .#palette-generator

The developer shell should include all necessary development packages. Since GHC is almost never used, I created a separate developer shell for it.

Personally I would prefer nixfmt over Alejandra, since it's being developed into the official standard formatter. At a glance it seems pre-commit-hooks supports it.

Despite its very early development, using the official formatter sound sreasonable due to Stylix's popularity.

Currently, harmonizing nixfmt (with the required treefmt-nix input) across nix fmt and pre-commit hooks is a nightmare and not worth implementing. The following patch demonstrates a "working" proof of concept without guaranteeing harmonization with nix fmt:

From 8c9597ea94795ee3061d6efb2d4d808ef9471ca5 Mon Sep 17 00:00:00 2001
From: NAHO <[email protected]>
Date: Tue, 10 Sep 2024 00:33:18 +0200
Subject: [PATCH] stylix: add nixfmt formatter

---
 flake.lock | 23 ++++++++++++++++++++++-
 flake.nix  | 13 +++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/flake.lock b/flake.lock
index b05b9fd..ddfaa91 100644
--- a/flake.lock
+++ b/flake.lock
@@ -283,7 +283,8 @@
         "gnome-shell": "gnome-shell",
         "home-manager": "home-manager",
         "nixpkgs": "nixpkgs",
-        "systems": "systems"
+        "systems": "systems",
+        "treefmt-nix": "treefmt-nix"
       }
     },
     "systems": {
@@ -300,6 +301,26 @@
         "repo": "default",
         "type": "github"
       }
+    },
+    "treefmt-nix": {
+      "inputs": {
+        "nixpkgs": [
+          "nixpkgs"
+        ]
+      },
+      "locked": {
+        "lastModified": 1725271838,
+        "narHash": "sha256-VcqxWT0O/gMaeWTTjf1r4MOyG49NaNxW4GHTO3xuThE=",
+        "owner": "numtide",
+        "repo": "treefmt-nix",
+        "rev": "9fb342d14b69aefdf46187f6bb80a4a0d97007cd",
+        "type": "github"
+      },
+      "original": {
+        "owner": "numtide",
+        "repo": "treefmt-nix",
+        "type": "github"
+      }
     }
   },
   "root": "root",
diff --git a/flake.nix b/flake.nix
index d6c7192..e1dc976 100644
--- a/flake.nix
+++ b/flake.nix
@@ -71,6 +71,11 @@

     # Interface flake systems.
     systems.url = "github:nix-systems/default";
+
+    treefmt-nix = {
+      inputs.nixpkgs.follows = "nixpkgs";
+      url = "github:numtide/treefmt-nix";
+    };
   };

   outputs =
@@ -109,6 +114,14 @@
           ];
         };

+        formatter = (
+          inputs.treefmt-nix.lib.evalModule pkgs {
+            programs.nixfmt.enable = true;
+            projectRootFile = "flake.nix";
+          }
+        )
+        .config.build.wrapper;
+
         packages = let
             universalPackages = {
               docs = import ./docs { inherit pkgs inputs lib; };
--

For reference, Alejandra adds 69% less new lines than nixfmt:

• Alejandra

  • 87 files changed, 1355 insertions(+), 1148 deletions(-)
    

• nixfmt

  • 85 files changed, 2532 insertions(+), 1874 deletions(-)
    

[trueNAHO]

Changelog

v1: https://github.com/danth/stylix/commit/cb8c9880874e209416198dbb68288051045cef20

• Replace Alejandra with nixfmt-rfc-style
• Add ghc developer shell
• Restrict .github/workflows/docs.yml workflow permission to the docs job
• Rename pre-commit-hooks instances to git-hooks
• Rebase on top of commit ef81ad9e85e6 ("gnome: move gnome-shell overlay out of gnome scope (#541)")

v0: https://github.com/danth/stylix/pull/519/commits/ad87e25438cc1aefd749fa5950759a9c6f6281d3

[trueNAHO]

Changelog

v2: https://github.com/danth/stylix/commit/e7d69955b38c011841e91500c217f67303d15225

• Bump magic-nix-cache-action GitHub Action: v7 -> v8
• Bump nix-installer-action GitHub Action: v13 -> v14

v1: https://github.com/danth/stylix/commit/cb8c9880874e209416198dbb68288051045cef20

• Replace Alejandra with nixfmt-rfc-style
• Add ghc developer shell
• Restrict .github/workflows/docs.yml workflow permission to the docs job
• Rename pre-commit-hooks instances to git-hooks
• Rebase on top of commit ef81ad9e85e6 ("gnome: move gnome-shell overlay out of gnome scope (#541)")

v0: https://github.com/danth/stylix/pull/519/commits/ad87e25438cc1aefd749fa5950759a9c6f6281d3

[trueNAHO]

@danth, could you approve this PR again by looking at its changelog and addressing the following unanswered question:

-      - name: Prepare docs for upload
-        run: cp -r --dereference --no-preserve=mode,ownership result/ public/

Unless the situation has changed since the original workflow was written, I think this copy was necessary due to an error with the files being part of the Nix store. (It was either because they were owned by root, or because there were symlinks involved, I can't remember exactly)

Good catch! Admittedly, I adapted this GitHub Action from my dotfiles setup without testing it. The only difference with my dotfiles setup is:

-path: result/usr/share/doc
+path: result

I assume the GitHub Action does not consider path pointing to a read-only symlink directory. In my dotfiles setup, I do not run into such problems because path points to a regular (not a read-only symlink directory) directory inside result. I use /usr/share/doc because it is a more conventional FHS location for documentation.

Should we apply your cp workaround or move the documentation output to result/usr/share/doc? NOTE: This is unaddressed in v1.

-- https://github.com/danth/stylix/pull/519#issuecomment-2341201294

I will rebase this PR on top of master and resolve merge conflicts when I resolve the GitHub Action issue based on your feedback.

[!NOTE]

Heavily consider reviewing the commits individually (https://patch-diff.githubusercontent.com/raw/danth/stylix/pull/519.patch), as over 90% of the diff stems from the final "treewide: add and apply nixfmt pre-commit hook" commit.

[trueNAHO]

Changelog

v3: https://github.com/danth/stylix/commit/ffd7436c23d17ede055d6be99711f9a037ed4c7e

• Rebase on top of commit 04afcfc0684d ("gnome: fix GDM theme not applying when Gnome is disabled (#598)")
• Bump nix-installer-action GitHub action: v14 -> v15
• Remove Link: https://github.com/trueNAHO/dotfiles* tags from commit messages

v2: https://github.com/danth/stylix/commit/e7d69955b38c011841e91500c217f67303d15225

• Bump magic-nix-cache-action GitHub Action: v7 -> v8
• Bump nix-installer-action GitHub Action: v13 -> v14

v1: https://github.com/danth/stylix/commit/cb8c9880874e209416198dbb68288051045cef20

• Replace Alejandra with nixfmt-rfc-style
• Add ghc developer shell
• Restrict .github/workflows/docs.yml workflow permission to the docs job
• Rename pre-commit-hooks instances to git-hooks
• Rebase on top of commit ef81ad9e85e6 ("gnome: move gnome-shell overlay out of gnome scope (#541)")

v0: https://github.com/danth/stylix/pull/519/commits/ad87e25438cc1aefd749fa5950759a9c6f6281d3

[trueNAHO]

Cc: @danth

path: result

-      - name: Prepare docs for upload
-        run: cp -r --dereference --no-preserve=mode,ownership result/ public/

Unless the situation has changed since the original workflow was written, I think this copy was necessary due to an error with the files being part of the Nix store. (It was either because they were owned by root, or because there were symlinks involved, I can't remember exactly)

Good catch! Admittedly, I adapted this GitHub Action from my dotfiles setup without testing it. The only difference with my dotfiles setup is:

-path: result/usr/share/doc
+path: result

I assume the GitHub Action does not consider path pointing to a read-only symlink directory. In my dotfiles setup, I do not run into such problems because path points to a regular (not a read-only symlink directory) directory inside result. I use /usr/share/doc because it is a more conventional FHS location for documentation.

Should we apply your cp workaround or move the documentation output to result/usr/share/doc? NOTE: This is unaddressed in v1.

-- https://github.com/danth/stylix/pull/519#issuecomment-2341201294

To continue making progress on the Roadmap by getting this PR merged, I have verified that the current path: result solution works.

Setup

To ensure that the GitHub Actions work properly, ensure that the GitHub repository has the following permissions before merging this PR.

https://github.com/danth/stylix/settings/actions

https://github.com/danth/stylix/settings/pages

Demo

The temporary https://github.com/trueNAHO/stylix-519 GitHub repository demonstrates the working GitHub Actions. Additionally, this repository displays the GitHub Pages website in its description:

PR Status

AFAIK, this PR is merge-ready with https://github.com/danth/stylix/pull/608 being merged.

[trueNAHO]

Changelog

v4: https://github.com/danth/stylix/commit/8880c87e15a580bd50a2468e892dfec4a2fe3df6

• Rebase on top of commit 5ab1207b2fde ("hyprland: adapt breaking changes (#610)")
• Update git-hooks input
• Bump nix-installer-action GitHub action: v15 -> v16

v3: https://github.com/danth/stylix/commit/ffd7436c23d17ede055d6be99711f9a037ed4c7e

• Rebase on top of commit 04afcfc0684d ("gnome: fix GDM theme not applying when Gnome is disabled (#598)")
• Bump nix-installer-action GitHub action: v14 -> v15
• Remove Link: https://github.com/trueNAHO/dotfiles* tags from commit messages

v2: https://github.com/danth/stylix/commit/e7d69955b38c011841e91500c217f67303d15225

• Bump magic-nix-cache-action GitHub Action: v7 -> v8
• Bump nix-installer-action GitHub Action: v13 -> v14

v1: https://github.com/danth/stylix/commit/cb8c9880874e209416198dbb68288051045cef20

• Replace Alejandra with nixfmt-rfc-style
• Add ghc developer shell
• Restrict .github/workflows/docs.yml workflow permission to the docs job
• Rename pre-commit-hooks instances to git-hooks
• Rebase on top of commit ef81ad9e85e6 ("gnome: move gnome-shell overlay out of gnome scope (#541)")

v0: https://github.com/danth/stylix/pull/519/commits/ad87e25438cc1aefd749fa5950759a9c6f6281d3

[jaredmontoya]

@trueNAHO @danth isn't it better to just configure formatters and linters using treefmt-nix so that nix fmt executes all formatters and linters and nix flake check checks if formatting is applied. Then just execute nix flake check on github actions as a PR check?

pre-commit hooks are useless in my opinion if the only thing they are used for is formatting and linting, they are easily bypassed. If someone is serious about enforcing code styling they should use treefmt-nix with a nix flake check github action that runs on pull requests.

treefmt-nix also has no dependencies other than nixpkgs so consumers of the nix flake don't have to download git hooks flake and all it's dependencies.

And treefmt-nix is such a simple enhancement with no downsides for nix fmt and nix flake check that everyone should be using it in their system configuration flake anyway. It's much better than default way of using nix fmt where user just assigns

formatter = pkgs.nixfmt-rfc-style;

and can't use anything other than that one formatter + doesn't get style checks on nix flake check

[trueNAHO]

isn't it better to just configure formatters and linters using treefmt-nix so that nix fmt executes all formatters and linters and nix flake check checks if formatting is applied. Then just execute nix flake check on github actions as a PR check?

pre-commit hooks are useless in my opinion if the only thing they are used for is formatting and linting, they are easily bypassed. If someone is serious about enforcing code styling they should use treefmt-nix with a nix flake check github action that runs on pull requests.

Yes, pre-commit hooks are an optional Git feature. Their compliance is, as you proposed, already checked with nix flake check in a GitHub action. Considering that Stylix naturally attracts casual one-off contributors, it seems good to ensure that their commits meet minimum quality standards:

 treewide: integrate linters and formatters into nix develop

What is the reasoning for this to be integrated into nix develop instead of just using nix fmt?

nix develop and nix flake simplify CI integration:

• I acknowledge pre-commit hooks being controversial. Nonetheless, they integrate nicely with nix flake check and nix develop, simplifying GitHub Actions and arguably improving the developer feedback loop. -- https://github.com/danth/stylix/pull/426#issuecomment-2168168100

• Interesting, hooks might make it more convenient for regular contributors. Having it run on CI too ensures nothing is missed. -- https://github.com/danth/stylix/pull/426#issuecomment-2168792953

-- https://github.com/danth/stylix/issues/236#issuecomment-2241615285

treefmt-nix also has no dependencies other than nixpkgs so consumers of the nix flake don't have to download git hooks flake and all it's dependencies.

And treefmt-nix is such a simple enhancement with no downsides for nix fmt and nix flake check that everyone should be using it in their system configuration flake anyway. It's much better than default way of using nix fmt where user just assigns

formatter = pkgs.nixfmt-rfc-style;

and can't use anything other than that one formatter + doesn't get style checks on nix flake check

The current git-hooks.nix input introduces only the repository itself and its gitignore dependency as overhead. Note that end-users can disable the developer git-hooks input with:

inputs.stylix.inputs.git-hooks.follows = "";

Initially, nix fmt was supposed to be supported, but it didn't seem possible when I last tried. At least without weird workarounds:

Currently, harmonizing nixfmt (with the required treefmt-nix input) across nix fmt and pre-commit hooks is a nightmare and not worth implementing. The following patch demonstrates a "working" proof of concept without guaranteeing harmonization with nix fmt:

From 8c9597ea94795ee3061d6efb2d4d808ef9471ca5 Mon Sep 17 00:00:00 2001
From: NAHO <[email protected]>
Date: Tue, 10 Sep 2024 00:33:18 +0200
Subject: [PATCH] stylix: add nixfmt formatter

---
 flake.lock | 23 ++++++++++++++++++++++-
 flake.nix  | 13 +++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/flake.lock b/flake.lock
index b05b9fd..ddfaa91 100644
--- a/flake.lock
+++ b/flake.lock
@@ -283,7 +283,8 @@
         "gnome-shell": "gnome-shell",
         "home-manager": "home-manager",
         "nixpkgs": "nixpkgs",
-        "systems": "systems"
+        "systems": "systems",
+        "treefmt-nix": "treefmt-nix"
       }
     },
     "systems": {
@@ -300,6 +301,26 @@
         "repo": "default",
         "type": "github"
       }
+    },
+    "treefmt-nix": {
+      "inputs": {
+        "nixpkgs": [
+          "nixpkgs"
+        ]
+      },
+      "locked": {
+        "lastModified": 1725271838,
+        "narHash": "sha256-VcqxWT0O/gMaeWTTjf1r4MOyG49NaNxW4GHTO3xuThE=",
+        "owner": "numtide",
+        "repo": "treefmt-nix",
+        "rev": "9fb342d14b69aefdf46187f6bb80a4a0d97007cd",
+        "type": "github"
+      },
+      "original": {
+        "owner": "numtide",
+        "repo": "treefmt-nix",
+        "type": "github"
+      }
     }
   },
   "root": "root",
diff --git a/flake.nix b/flake.nix
index d6c7192..e1dc976 100644
--- a/flake.nix
+++ b/flake.nix
@@ -71,6 +71,11 @@

     # Interface flake systems.
     systems.url = "github:nix-systems/default";
+
+    treefmt-nix = {
+      inputs.nixpkgs.follows = "nixpkgs";
+      url = "github:numtide/treefmt-nix";
+    };
   };

   outputs =
@@ -109,6 +114,14 @@
           ];
         };

+        formatter = (
+          inputs.treefmt-nix.lib.evalModule pkgs {
+            programs.nixfmt.enable = true;
+            projectRootFile = "flake.nix";
+          }
+        )
+        .config.build.wrapper;
+
         packages = let
             universalPackages = {
               docs = import ./docs { inherit pkgs inputs lib; };
--

-- https://github.com/danth/stylix/pull/519#issuecomment-2341201294

[trueNAHO]

Changelog

v5: https://github.com/danth/stylix/commit/69785a7428237961122fb90712911d30271b5d6e

• Rebase on top of commit 963e77a3a4fc ("gtk: add support for theming Flatpak applications (#693)")
• Add commit ffef4de9abd8 ("ci: update Ubuntu runner to ubuntu-24.04")
• Extend commit a9295b8153bc ("ci: simplify Docs GitHub workflow") from v4 to also simplify the Backport workflow in commit 92ae2de02dd6 ("ci: simplify GitHub workflows")

v4: https://github.com/danth/stylix/commit/8880c87e15a580bd50a2468e892dfec4a2fe3df6

• Rebase on top of commit 5ab1207b2fde ("hyprland: adapt breaking changes (#610)")
• Update git-hooks input
• Bump nix-installer-action GitHub action: v15 -> v16

v3: https://github.com/danth/stylix/commit/ffd7436c23d17ede055d6be99711f9a037ed4c7e

• Rebase on top of commit 04afcfc0684d ("gnome: fix GDM theme not applying when Gnome is disabled (#598)")
• Bump nix-installer-action GitHub action: v14 -> v15
• Remove Link: https://github.com/trueNAHO/dotfiles* tags from commit messages

v2: https://github.com/danth/stylix/commit/e7d69955b38c011841e91500c217f67303d15225

• Bump magic-nix-cache-action GitHub Action: v7 -> v8
• Bump nix-installer-action GitHub Action: v13 -> v14

v1: https://github.com/danth/stylix/commit/cb8c9880874e209416198dbb68288051045cef20

• Replace Alejandra with nixfmt-rfc-style
• Add ghc developer shell
• Restrict .github/workflows/docs.yml workflow permission to the docs job
• Rename pre-commit-hooks instances to git-hooks
• Rebase on top of commit ef81ad9e85e6 ("gnome: move gnome-shell overlay out of gnome scope (#541)")

v0: https://github.com/danth/stylix/pull/519/commits/ad87e25438cc1aefd749fa5950759a9c6f6281d3

[trueNAHO]

Changelog

v6: https://github.com/danth/stylix/commit/c16493e269ce22cf7393bcff10ce423c75ea139f

• Rebase on top of commit a4ed4168fb83 ("foot: update desktop file name in testbed (#707)")
• Add commit "stylix: add packages to checks output" for nix flake check to work as expected
• Add commit "stylix: add nix-flake-check package" to parallelize the native nix flake check command
• Replace commit "ci: merge Build and Lint GitHub workflows using 'nix flake check'" with commits "ci: consolidate Build and Lint workflows into single Check workflow" and "ci: prevent the Check workflow from running duplicated checks outputs" because GitHub workflows scale better by spawning more instances rather than parallelizing a single instance

v5: https://github.com/danth/stylix/commit/69785a7428237961122fb90712911d30271b5d6e

• Rebase on top of commit 963e77a3a4fc ("gtk: add support for theming Flatpak applications (#693)")
• Add commit ffef4de9abd8 ("ci: update Ubuntu runner to ubuntu-24.04")
• Extend commit a9295b8153bc ("ci: simplify Docs GitHub workflow") from v4 to also simplify the Backport workflow in commit 92ae2de02dd6 ("ci: simplify GitHub workflows")

v4: https://github.com/danth/stylix/commit/8880c87e15a580bd50a2468e892dfec4a2fe3df6

• Rebase on top of commit 5ab1207b2fde ("hyprland: adapt breaking changes (#610)")
• Update git-hooks input
• Bump nix-installer-action GitHub action: v15 -> v16

v3: https://github.com/danth/stylix/commit/ffd7436c23d17ede055d6be99711f9a037ed4c7e

• Rebase on top of commit 04afcfc0684d ("gnome: fix GDM theme not applying when Gnome is disabled (#598)")
• Bump nix-installer-action GitHub action: v14 -> v15
• Remove Link: https://github.com/trueNAHO/dotfiles* tags from commit messages

v2: https://github.com/danth/stylix/commit/e7d69955b38c011841e91500c217f67303d15225

• Bump magic-nix-cache-action GitHub Action: v7 -> v8
• Bump nix-installer-action GitHub Action: v13 -> v14

v1: https://github.com/danth/stylix/commit/cb8c9880874e209416198dbb68288051045cef20

• Replace Alejandra with nixfmt-rfc-style
• Add ghc developer shell
• Restrict .github/workflows/docs.yml workflow permission to the docs job
• Rename pre-commit-hooks instances to git-hooks
• Rebase on top of commit ef81ad9e85e6 ("gnome: move gnome-shell overlay out of gnome scope (#541)")

v0: https://github.com/danth/stylix/pull/519/commits/ad87e25438cc1aefd749fa5950759a9c6f6281d3

[trueNAHO]

After this change, I think the nix_flake_check workflow will only evaluate testbeds and documentation, rather than actually building them. (The docs workflow builds the documentation, but not until after merging.) Previously, the derivations for each testbed were built fully, which is able to catch more errors.

Good catch! I always forget that nix flake check does not yet build all packages.${system}.<PACKAGE> packages. IIRC, adding this functionality to Nix is part of stabilizing flakes.

This could be helped while still only having to run nix flake check by moving or copying everything from packages into checks. Moving has the consequence of breaking old documentation which says "try this with nix flake run .#testbeds-gnome-dark" and such; copying has the consequence of making the output from nix flake show less readable and potentially evaluating each package twice.

The following safe pattern ensures that all packages.${system}.<PACKAGE> packages are part of the checks output, without evaluating packages twice or sacrificing readability:

checks =
  lib.attrsets.unionOfDisjoint
  {
    git-hooks = inputs.git-hooks.lib.${system}.run {
      hooks = {/* Add hooks here */};
      src = ./.;
    };
  }
  self.packages.${system};

Potential future name conflicts can easily be resolved by adding a prefix:

checks =
  lib.attrsets.unionOfDisjoint
  {
    git-hooks = inputs.git-hooks.lib.${system}.run {
      hooks = {/* Add hooks here */};
      src = ./.;
    };
  }
  (
    lib.attrsets.concatMapAttrs
    (name: value: {"package-${name}" = value;})
    self.packages.${system};
  );

For simplicity, the prefix is not added in the new commit "stylix: add packages to checks output".

Unless it was tested and found not to be an issue, it appears that this comment has been forgotten somewhere along the development of this PR.

This has already been addressed and successfully tested:

path: result

-      - name: Prepare docs for upload
-        run: cp -r --dereference --no-preserve=mode,ownership result/ public/

Unless the situation has changed since the original workflow was written, I think this copy was necessary due to an error with the files being part of the Nix store. (It was either because they were owned by root, or because there were symlinks involved, I can't remember exactly)

Good catch! Admittedly, I adapted this GitHub Action from my dotfiles setup without testing it. The only difference with my dotfiles setup is:

-path: result/usr/share/doc
+path: result

I assume the GitHub Action does not consider path pointing to a read-only symlink directory. In my dotfiles setup, I do not run into such problems because path points to a regular (not a read-only symlink directory) directory inside result. I use /usr/share/doc because it is a more conventional FHS location for documentation.

Should we apply your cp workaround or move the documentation output to result/usr/share/doc? NOTE: This is unaddressed in v1.

-- https://github.com/danth/stylix/pull/519#issuecomment-2341201294

To continue making progress on the Roadmap by getting this PR merged, I have verified that the current path: result solution works.

Setup

To ensure that the GitHub Actions work properly, ensure that the GitHub repository has the following permissions before merging this PR.

https://github.com/danth/stylix/settings/actions

https://github.com/danth/stylix/settings/pages

Demo

The temporary https://github.com/trueNAHO/stylix-519 GitHub repository demonstrates the working GitHub Actions. Additionally, this repository displays the GitHub Pages website in its description:

PR Status

AFAIK, this PR is merge-ready with https://github.com/danth/stylix/pull/608 being merged.

-- NAHO, https://github.com/danth/stylix/pull/519#issuecomment-2463022590

This is quite a large PR which has been open for some time. Some parts are already ready to merge, but are waiting due to other parts which are not.

AFAIK, everything is merge-ready. Only the last commit "treewide: add and apply nixfmt pre-commit hook" frequently causes merge conflicts for obvious reasons, which is the primary reason for all rebases. Note that this patchset should only be merged if it is rebased on top of master to prevent the CI from failing.

I think breaking it down would ease reviewing and lead to a faster merge overall.

This patchset should be considered as a series of atomic commits and should be merged as such:

• [!NOTE]

  These commits should be merged individually, not squashed, to ensure a clear and easily reversible changelog. In case of change requests, review the commits separately.

  -- NAHO, https://github.com/danth/stylix/pull/519#issue-2481722202

• [!NOTE]

  Heavily consider reviewing the commits individually (https://patch-diff.githubusercontent.com/raw/danth/stylix/pull/519.patch), as over 90% of the diff stems from the final "treewide: add and apply nixfmt pre-commit hook" commit.

  -- NAHO, https://github.com/danth/stylix/pull/519#issuecomment-2397552781

Would you mind splitting it into something like the following?

• Simplify workflows with nix flake check + packages moved or copied into checks

This is already covered by the following commits:

Commit ID	Commit Header
3	treewide: ignore unused arguments detected by deadnix
4	stylix: add deadnix and statix pre-commit hooks
5	ci: consolidate Build and Lint workflows into single Check workflow
9	ci: lock workflow dependencies to increase reproducibility
10	ci: update Ubuntu runner to ubuntu-24.04
11	ci: simplify workflows
14	ci: prevent the Check workflow from running duplicated checks outputs

Note that the commit "ci: prevent the Check workflow from running duplicated checks outputs" comes after the later commit "treewide: add and apply yamllint pre-commit hook" to modify the jq query in its properly formatter version.

• Add developer shell + direnv + related documentation

This is already covered by the following commits:

Commit ID	Commit Header
1	treewide: add developer shell
2	treewide: leverage direnv to automatically enter developer shell
6	stylix: integrate pre-commit hooks into developer shell
7	stylix: add packages to checks output
8	stylix: add nix-flake-check package
17	stylix: add ghc developer shell

Note that the commit "stylix: add ghc developer shell" comes after the later commit "stylix: add hlint pre-commit hook" to group Haskell related commits together.

• Add formatter hooks and checks.«system».git-hooks
• Apply the chosen formatters treewide

This is already covered by the following commits:

Commit ID	Commit Header
12	treewide: add and apply typos pre-commit hook
13	treewide: add and apply yamllint pre-commit hook
15	treewide: add and apply stylish-haskell
16	stylix: add hlint pre-commit hook
18	treewide: add and apply nixfmt pre-commit hook

As far as I can tell, these are independent of each other, apart from the hooks which may require the developer shell to be merged first.

Actually, they depend on each other to satisfy the CI and prevent upstream merge conflicts. For example, the commits adding pre-commit hooks come after the CI migration commits to make the CI migration purely a refactor change without any functional changes. The commits migrating the CI come after the developer shell setup because the developer shell is used in the CI. Inside each of these three groups, the commits are ordered to reduce internal merge conflicts and make individual commits as easy as possible to understand.

I have taken great care to make the individual commits easy to understand, even through the several rebases this patchset has gone through.

The only patch from this patchset that is truly independent has already been extracted into https://github.com/danth/stylix/pull/520.

However, I agree that we should get this PR merged as quickly as possible and resolve nitpicks in follow-up PRs.

I'm suggesting applying the formatters separately from adding them just due to the large number of changed files, which makes it hard to find the important parts in the diff, as well as being prone to merge conflicts.

Adding and applying the formatters temporarily makes the commit non-atomic and therefore harder to revert, breaks the CI, makes future bug hunting harder, and is discouraged:

When dividing your change into a series of patches, take special care to ensure that the kernel builds and runs properly after each patch in the series. Developers using git bisect to track down a problem can end up splitting your patch series at any point; they will not thank you if you introduce bugs in the middle.

-- The Linux Kernel documentation, "Submitting patches: the essential guide to getting your code into the kernel"

[stylix-automation[bot]]

Backport failed for release-24.11, 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.11
git worktree add -d .worktree/backport-519-to-release-24.11 origin/release-24.11
cd .worktree/backport-519-to-release-24.11
git switch --create backport-519-to-release-24.11
git cherry-pick -x 703f49aaca9030b066bb60310d19d7cadfb4b376 6ef37ca6aaf82e59394d0a93acdc932af1739d58 3c54cb3384116d430e2a83c5d3d3f0dd59eb9ff2 6085560893ae3620acf8d799dda138b207e6f5ed 0d0af5f4426104ed82b17d41f8763fcf84b2c0ea b3230ef39814b1c3809ffbad20f6361c8b737731 e56d332ca367bd021d3c584ac9130064916cd0c7 a0838923e45f63b2b46a132eaa02c45e03e8818a 1aa931f6f1eb85b4f8358e7ed3706ed82d3048ca fe72c2306f517a865c23e9fc92fc72cc408c1ab7 2b85a562355c80e1ef2f9070a3a44befdd7c9764 5ab7d0345a902f5f4c30f9c31ccf42c563ad6463 d3bdbf0c5b4656955be0019e821de6fa65a6bb78 4ceede75042e362d3270b52d4870702a99915bb0 439c6cf24e89730d3271baf10e5706df1cdecedf 708b2147c095a60994f56714968f00de531f8deb 807c81894e9af60b105476c3dab679e9dc86686f ad64260a75f0331d4d9b0db70f60634283e0aa5d

[stylix-automation[bot]]

Backport failed for release-24.11, 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.11
git worktree add -d .worktree/backport-519-to-release-24.11 origin/release-24.11
cd .worktree/backport-519-to-release-24.11
git switch --create backport-519-to-release-24.11
git cherry-pick -x 703f49aaca9030b066bb60310d19d7cadfb4b376 6ef37ca6aaf82e59394d0a93acdc932af1739d58 3c54cb3384116d430e2a83c5d3d3f0dd59eb9ff2 6085560893ae3620acf8d799dda138b207e6f5ed 0d0af5f4426104ed82b17d41f8763fcf84b2c0ea b3230ef39814b1c3809ffbad20f6361c8b737731 e56d332ca367bd021d3c584ac9130064916cd0c7 a0838923e45f63b2b46a132eaa02c45e03e8818a 1aa931f6f1eb85b4f8358e7ed3706ed82d3048ca fe72c2306f517a865c23e9fc92fc72cc408c1ab7 2b85a562355c80e1ef2f9070a3a44befdd7c9764 5ab7d0345a902f5f4c30f9c31ccf42c563ad6463 d3bdbf0c5b4656955be0019e821de6fa65a6bb78 4ceede75042e362d3270b52d4870702a99915bb0 439c6cf24e89730d3271baf10e5706df1cdecedf 708b2147c095a60994f56714968f00de531f8deb 807c81894e9af60b105476c3dab679e9dc86686f ad64260a75f0331d4d9b0db70f60634283e0aa5d