diff --git a/.config/filegen-manifest.json b/.config/filegen-manifest.json index 10a5e42..9b52bbd 100644 --- a/.config/filegen-manifest.json +++ b/.config/filegen-manifest.json @@ -6,7 +6,25 @@ "deactivate": null, "ignore-modification": null, "permissions": "600", - "source": "/nix/store/kzmn0sh10g17q526a6zczgl9pi8da21d-gitattributes", + "source": "/nix/store/9qakq8nv15bh26zlwvq7xj1nxj6ppqb4-pre-commit-config.yaml", + "target": "./.pre-commit-config.yaml", + "type": "copy" + }, + { + "clobber": null, + "deactivate": null, + "ignore-modification": null, + "permissions": "600", + "source": "/nix/store/yw3vsh26id80w6bivi9v4x3b835b4r5j-check-pre-commit-hooks.yml", + "target": "./.github/workflows/check-pre-commit-hooks.yml", + "type": "copy" + }, + { + "clobber": null, + "deactivate": null, + "ignore-modification": null, + "permissions": "600", + "source": "/nix/store/mv9zlzz043051izx92fxk6ibz6fn21yd-gitattributes", "target": ".gitattributes", "type": "copy" } diff --git a/.gitattributes b/.gitattributes index 3057906..5daa663 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,4 @@ +.pre-commit-config.yaml linguist-generated +.github/workflows/check-pre-commit-hooks.yml linguist-generated .gitattributes linguist-generated .config/filegen-manifest.json linguist-generated diff --git a/.github/workflows/check-pre-commit-hooks.yml b/.github/workflows/check-pre-commit-hooks.yml new file mode 100644 index 0000000..f5477fd --- /dev/null +++ b/.github/workflows/check-pre-commit-hooks.yml @@ -0,0 +1,24 @@ +# This file is automatically generated from Nix configuration. Do not edit directly. + +concurrency: + cancel-in-progress: true + group: ${{ github.workflow }}-${{ github.ref }} +jobs: + prek: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 + - uses: cachix/install-nix-action@8aa03977d8d733052d78f4e008a241fd1dbf36b3 + - name: Run pre-commit hooks + run: prek --all-files --show-diff-on-failure + shell: nix develop .#standards --command bash {0} +name: Make sure all pre-commit hooks pass +"on": + pull_request: + branches: + - '**' + types: + - opened + - reopened + - synchronize + - ready_for_review diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..c1d78b4 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,41 @@ +repos: +- hooks: + - id: trailing-whitespace + - id: check-added-large-files + - id: check-case-conflict + - id: check-illegal-windows-names + - id: end-of-file-fixer + - id: fix-byte-order-marker + - id: check-json + - id: check-json5 + - id: check-toml + - id: check-vcs-permalinks + - id: check-xml + - args: + - --fix=lf + id: mixed-line-ending + - id: check-symlinks + - id: destroyed-symlinks + - id: check-merge-conflict + - id: detect-private-key + - id: check-shebang-scripts-are-executable + - id: check-executables-have-shebangs + repo: builtin +- hooks: + - args: + - --write-changes + - --force-exclude + description: Check the repository for spelling mistakes + entry: typos + id: typos + language: system + name: typos + types: + - text + - description: Ensure that files set up with the filegen module are up-to-date + entry: filegen-apply-script + id: filegen + language: system + name: filegen + pass_filenames: false + repo: local diff --git a/README.md b/README.md index f6c19cc..4d9e945 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ To use the standards in a new project, create the following # Specify a default devshell for the project; other options are # documented in the devshells section below. # - # devShells.default = inputs'.famedly-engineering-standards.devShells.rust; + # devShells.default = inputs'.famedly-engineering-standards.devShells.standards; famedly.standards = { # Read module documentation for further details, but most @@ -87,6 +87,7 @@ The following basic devshells are available: | Name | Purpose | | --------- | ------- | +| standards | Contains basic tools and configuration used by all famedly projects. | | rust | Contains the Famedly Rust toolchain, and everything required to build Rust projects. | | k8s | Contains miscellaneous k8s-related utilities, especially useful on MacOS. | @@ -154,3 +155,35 @@ to ensure that most developers can interact with this repository. If you struggle creating a completely new file, ask someone with more nix experience to help you hook this into the filegen-activate command. + +### Adding pre-commit hooks + +Pre-commit hooks are intended to be *very* fast to execute. These +should be ergonomic to run every time you create a git commit - when +doing rebase operations, that can mean dozens in a few 100ms. + +As such, these should never contain expensive operations - compiling +code is absolutely not acceptable, and even complex parsing (for +formatting) may be pushing it. + +For more expensive operations, add a workflow instead, and let CI +perform the check. + +--- + +To actually add a pre-commit hook, in addition to defining the hook, +any dependencies required **must** be added to the `prek` +wrapper. Assuming the dependency is packaged in nixpkgs, this is quite +easy: + +```nix +{ + config.perSystem = { pkgs, ... }: { + prek-pre-commit.package.runtimePkgs = [ pkgs.foobar ]; + }; +} +``` + +More complex modifications to the `prek` wrapper are possible too, +refer to our [wrapper module provider](https://birdeehub.github.io/nix-wrapper-modules/modules/default.html) +for details. diff --git a/flake.lock b/flake.lock index 799f267..f4b849e 100644 --- a/flake.lock +++ b/flake.lock @@ -95,7 +95,8 @@ "flake-parts": "flake-parts", "github-actions-nix": "github-actions-nix", "nixpkgs": "nixpkgs", - "rust-overlay": "rust-overlay" + "rust-overlay": "rust-overlay", + "wrappers": "wrappers" } }, "rust-overlay": { @@ -117,6 +118,26 @@ "repo": "rust-overlay", "type": "github" } + }, + "wrappers": { + "inputs": { + "nixpkgs": [ + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1780661205, + "narHash": "sha256-3F5DixT3Gk91lBI9E+TGMm0ko5HrRbDiL23di16TJGA=", + "owner": "BirdeeHub", + "repo": "nix-wrapper-modules", + "rev": "8dd304c3582ddd339217e1cc5fb53f50acb63c2d", + "type": "github" + }, + "original": { + "owner": "BirdeeHub", + "repo": "nix-wrapper-modules", + "type": "github" + } } }, "root": "root", diff --git a/flake.nix b/flake.nix index 0a50512..e538921 100644 --- a/flake.nix +++ b/flake.nix @@ -20,6 +20,11 @@ url = "github:oxalica/rust-overlay"; inputs.nixpkgs.follows = "nixpkgs"; }; + + wrappers = { + url = "github:BirdeeHub/nix-wrapper-modules"; + inputs.nixpkgs.follows = "nixpkgs"; + }; }; outputs = @@ -37,7 +42,10 @@ flakeModules = rec { filegen = ./nix/modules/filegen.nix; - prek-pre-commit = importApply ./nix/modules/prek-pre-commit.nix { inherit filegen; }; + prek-pre-commit = importApply ./nix/modules/prek-pre-commit.nix { + inherit filegen; + inherit (inputs) wrappers; + }; }; default = importApply ./nix (args // { inherit importApply flakeModules; }); diff --git a/nix/default.nix b/nix/default.nix index 1bbdfe5..2f1df7a 100644 --- a/nix/default.nix +++ b/nix/default.nix @@ -25,7 +25,20 @@ importingFlake: { ]; # TODO: Break this out into a proper ecosystem-oriented module. - perSystem = { pkgs, lib, ... }: { - devshells.k8s.packages = lib.attrValues { inherit (pkgs) kubectl kubelogin-oidc kubetui; }; - }; + perSystem = + { + config, + pkgs, + lib, + ... + }: + { + devshells.k8s = { + packages = lib.attrValues { inherit (pkgs) kubectl kubelogin-oidc kubetui; }; + + # TODO: Find a better way to inherit devshell + # configurations. + commands = lib.filter (command: command.name != "menu") config.devshells.standards.commands; + }; + }; } diff --git a/nix/general/default.nix b/nix/general/default.nix index e71d397..c2adcb2 100644 --- a/nix/general/default.nix +++ b/nix/general/default.nix @@ -3,9 +3,15 @@ lib, importApply, ... -}: +}@args: importingFlake: { - imports = [ ./action-versions.nix ]; + imports = [ + ./action-versions.nix + (importApply ./devshell.nix args) + (importApply ./pre-commit-hooks.nix args) + + ./workflows/check-pre-commit-hooks.nix + ]; config.perSystem = { config, ... }: diff --git a/nix/general/devshell.nix b/nix/general/devshell.nix new file mode 100644 index 0000000..d890ae0 --- /dev/null +++ b/nix/general/devshell.nix @@ -0,0 +1,29 @@ +{ + flakeModules, + inputs, + lib, + ... +}: +importingFlake: { + config.perSystem = { config, ... }: { + devshells.standards = { + name = lib.mkDefault "engineering-standards"; + + commands = [ + { + name = "prek"; + help = "Run pre-commit hooks on currently staged changes"; + category = "[[lints and checks]]"; + package = config.prek-pre-commit.package.wrapper; + } + + { + name = "prek -s main -o HEAD"; + help = "Run pre-commit hooks on all commits in the current branch"; + category = "[[lints and checks]]"; + package = config.prek-pre-commit.package.wrapper; + } + ]; + }; + }; +} diff --git a/nix/general/pre-commit-hooks.nix b/nix/general/pre-commit-hooks.nix new file mode 100644 index 0000000..e203204 --- /dev/null +++ b/nix/general/pre-commit-hooks.nix @@ -0,0 +1,117 @@ +{ + flakeModules, + inputs, + lib, + ... +}: +importingFlake: { + config.perSystem = { pkgs, self', ... }: { + prek-pre-commit = { + package.runtimePkgs = lib.attrValues { + inherit (pkgs) editorconfig-checker typos; + filegen-activate = self'.apps.filegen-activate.meta.package; + }; + + workspaces.".".repos = [ + { + repo = "builtin"; + + # Regularly check this list for useful new lints, these + # are generally pretty cheap since they're implemented in + # rust and run in the same process: + # + # https://prek.j178.dev/builtin/#supported-hooks_1 + hooks = [ + { id = "trailing-whitespace"; } + { id = "check-added-large-files"; } + { id = "check-case-conflict"; } + { id = "check-illegal-windows-names"; } + { id = "end-of-file-fixer"; } + + # This needs very specific per-repo configuration, so we + # don't globally enable it. + # + # {id = "file-contents-sorter";} + + { id = "fix-byte-order-marker"; } + { id = "check-json"; } + { id = "check-json5"; } + + # We should use treefmt-nix for this instead. + # + # { id = "pretty-format-json"; } + + { id = "check-toml"; } + { id = "check-vcs-permalinks"; } + + # TODO: We would like to use this, but we need to tweak it + # a little for helm charts. + # + # { id = "check-yaml"; } + + { id = "check-xml"; } + { + id = "mixed-line-ending"; + args = [ "--fix=lf" ]; + } + { id = "check-symlinks"; } + { id = "destroyed-symlinks"; } + { id = "check-merge-conflict"; } + { id = "detect-private-key"; } + + # Branch protection rules should be set on the git forge + # instead. + # + # { id = "no-commit-to-branch"; } + + { id = "check-shebang-scripts-are-executable"; } + { id = "check-executables-have-shebangs"; } + ]; + } + + { + repo = "local"; + hooks = [ + { + id = "typos"; + name = "typos"; + description = "Check the repository for spelling mistakes"; + + entry = "typos"; + args = [ + "--write-changes" + "--force-exclude" + ]; + + language = "system"; + types = [ "text" ]; + } + + # TODO: Currently too invasive for some downstreams, we + # need to either tweak the configuration or make it + # somewhat overridable. + # + # { + # id = "editorconfig"; + # name = "editorconfig"; + # description = "Ensure all files in the project match editorconfig rules"; + + # entry = "editorconfig-checker"; + # language = "system"; + # } + + { + id = "filegen"; + name = "filegen"; + description = "Ensure that files set up with the filegen module are up-to-date"; + pass_filenames = false; + + entry = "filegen-apply-script"; + language = "system"; + } + ]; + } + ]; + }; + }; +} diff --git a/nix/general/workflows/check-pre-commit-hooks.nix b/nix/general/workflows/check-pre-commit-hooks.nix new file mode 100644 index 0000000..0cec5e4 --- /dev/null +++ b/nix/general/workflows/check-pre-commit-hooks.nix @@ -0,0 +1,45 @@ +{ config, ... }: +let + allowed-actions = config.famedly.standards.allowed-action-versions; +in +{ + perSystem.githubActions.workflows.check-pre-commit-hooks = { + name = "Make sure all pre-commit hooks pass"; + + # We don't run these on `push`, since the start and end of the + # commit series isn't clear in that case. + # + # This does mean that you need to have an open PR for workflows + # against your branch to run, but that's probably reasonable for + # cost saving purposes anyway. + on.pullRequest = { + branches = [ "**" ]; + types = [ + "opened" + "reopened" + "synchronize" + "ready_for_review" + ]; + }; + + concurrency = { + group = "\${{ github.workflow }}-\${{ github.ref }}"; + cancelInProgress = true; + }; + + jobs.prek = { + runsOn = "ubuntu-latest"; + + steps = [ + { uses = allowed-actions."actions/checkout".uses; } + { uses = allowed-actions."cachix/install-nix-action".uses; } + + { + name = "Run pre-commit hooks"; + shell = "nix develop .#standards --command bash {0}"; + run = "prek --all-files --show-diff-on-failure"; + } + ]; + }; + }; +} diff --git a/nix/modules/filegen.nix b/nix/modules/filegen.nix index 5c2626d..0e43339 100644 --- a/nix/modules/filegen.nix +++ b/nix/modules/filegen.nix @@ -195,11 +195,10 @@ in ]; apps = let - activate = pkgs.writers.writeNu "filegen-apply-script" '' + activate = pkgs.writers.writeNuBin "filegen-apply-script" '' cd (git rev-parse --show-toplevel) (${lib.getExe cfg.smfhPackage} - --verbose --impure diff --fallback @@ -208,22 +207,29 @@ in mkdir .config cp --preserve [] ${new-manifest} .config/filegen-manifest.json + chmod 600 .config/filegen-manifest.json ''; - deactivate = pkgs.writers.writeNu "filegen-deactivate-script" '' + deactivate = pkgs.writers.writeNuBin "filegen-deactivate-script" '' cd (git rev-parse --show-toplevel) ${lib.getExe cfg.smfhPackage} deactivate .config/filegen-manifest.json ''; in { filegen-activate = { - program = activate.outPath; - meta.description = "Install files defined by the `filegen` options of this flake"; + program = lib.getExe activate; + meta = { + description = "Install files defined by the `filegen` options of this flake"; + package = activate; + }; }; filegen-deactivate = { - program = deactivate.outPath; - meta.description = "Uninstall all files previously installed using `filegen-activate`"; + program = lib.getExe deactivate; + meta = { + description = "Uninstall all files previously installed using `filegen-activate`"; + package = deactivate; + }; }; }; }; diff --git a/nix/modules/prek-pre-commit.nix b/nix/modules/prek-pre-commit.nix index 0c5aba3..3f34f5e 100644 --- a/nix/modules/prek-pre-commit.nix +++ b/nix/modules/prek-pre-commit.nix @@ -1,4 +1,4 @@ -{ filegen, ... }: +{ filegen, wrappers, ... }: { flake-parts-lib, lib, ... }: let inherit (lib) types; @@ -15,14 +15,22 @@ in options.prek-pre-commit = { package = lib.mkOption { description = "The `prek` package to use to execute pre-commit hooks"; - type = types.package; - default = pkgs.prek; + type = wrappers.lib.types.subWrapperModule { + imports = [ wrappers.lib.modules.default ]; + + pkgs = lib.mkDefault pkgs; + package = lib.mkDefault pkgs.prek; + }; }; workspaces = lib.mkOption { description = '' `prek` configuration for each workspace. + Note that any hooks that depend on system packages being installed + should have their packages installed via the + `prek-pre-commit.package.runtimePkgs` option. + See the [`prek` documentation](https://prek.j178.dev/workspace/) for details. ''; default = { }; diff --git a/nix/rust/devshell.nix b/nix/rust/devshell.nix index 3ec01b0..2682c3d 100644 --- a/nix/rust/devshell.nix +++ b/nix/rust/devshell.nix @@ -48,6 +48,10 @@ value = "${self'.packages.famedly-rust-toolchain}/lib/rustlib/src/rust/library"; } ]; + + # TODO: Find a better way to inherit devshell + # configurations. + commands = lib.filter (command: command.name != "menu") config.devshells.standards.commands; }; }