Hook order ignored

[yajo]

Follow instructions from readme: https://github.com/cachix/pre-commit-hooks.nix/blob/61a3511668891c68ebd19d40122150b98dc2fe3b/README.md?plain=1#L31-L73

Now inspect .pre-commit-config.yaml:

# DO NOT MODIFY
# This file was generated by pre-commit-hooks.nix
{
  "default_stages": [
    "commit"
  ],
  "repos": [
    {
      "hooks": [
        {
          "entry": "/nix/store/yyc8pmfy43nl2hwfb3n16rcd866nqki8-elm-format-0.8.5/bin/elm-format --yes --elm-version=0.19",
          "exclude": "^$",
          "files": "\\.elm$",
          "id": "elm-format",
          "language": "system",
          "name": "elm-format",
          "pass_filenames": true,
          "stages": [
            "commit"
          ],
          "types": [
            "file"
          ],
          "types_or": []
        },
        {
          "entry": "/nix/store/gvp4jil2i6r4n7kvx4sh4d7vhp5v433j-ormolu-0.3.1.0-bin/bin/ormolu --mode inplace '--ghc-opt' '-Xlhs' '--ghc-opt' '-Xhs' ",
          "exclude": "^$",
          "files": "\\.l?hs(-boot)?$",
          "id": "ormolu",
          "language": "system",
          "name": "ormolu",
          "pass_filenames": true,
          "stages": [
            "commit"
          ],
          "types": [
            "file"
          ],
          "types_or": []
        },
        {
          "entry": "/nix/store/25i9mjmk2a0pq19jgiscn3wfc539ya3a-shellcheck-0.8.0-bin/bin/shellcheck",
          "exclude": "^$",
          "files": "",
          "id": "shellcheck",
          "language": "system",
          "name": "shellcheck",
          "pass_filenames": true,
          "stages": [
            "commit"
          ],
          "types": [
            "shell"
          ],
          "types_or": [
            "sh",
            "ash",
            "bash",
            "bats",
            "dash",
            "ksh"
          ]
        }
      ],
      "repo": "local"
    }
  ]
}

You'll see the order of hooks is:

• elm-format
• ormolu
• shellcheck

Now, change the order of hooks in default.nix. Set them like:

    hooks = {
      shellcheck.enable = true;
      ormolu.enable = true;
      elm-format.enable = true;
    };

Again, enter nix-shell and inspect .pre-commit-config.yaml. You'll see that the hooks are in the same order as the 1st time. The file is in fact the same.

This is not a big problem in this case, but in other cases, order matter.

For example, if you're writing python and want to enable autoflake and flake8, you will most likely want autoflake to run before flake8, so flake8 doesn't catch the errors that autoflake already fixed.

In specific cases like this one, we could still provide a sane default order. But there are custom hooks, and we can't predict in which order they should be executed.

So, this project needs IMHO a way to specify hook order, just like normal pre-commit does.

[roberth]

Attribute sets are always ordered lexicographically, as a way to simplify language semantics, but I can see that it's a little annoying here. pre-commit-hooks.nix can solve this by adding priority numbers to the hooks, or an option where you can specify a list of hook names to define the order you want, separately from the hooks definitions.

Alternatively, it might be possible to add a new type to the module system that makes it slightly more convenient than that, but this would have to be designed with care so that values can be merged from multiple modules without relying on an implicit ordering of those modules, which is a property that integrations such as devenv and flake-parts rely on.

you will most likely want autoflake to run before flake8

It'd be really nice if a default ordering could be specified declaratively to encode this knowledge so that it just does the right thing. We have a couple of instances in NixOS where we let users declare partial orderings using before or after attributes. The pre-commit.nix module could provide that functionality.

[yajo]

Or maybe this could accept both an attrset or a list of attrsets. Lists are ordered, so if we really care about ordering, we can use the latter form. Example:

hooks = [
  # Sorted
  {autoflake.enable = true;}
  {flake8.enable = true;}
  # Unsorted
  {
      shellcheck.enable = true;
      ormolu.enable = true;
      elm-format.enable = true;
  }
]

[srid]

We have a couple of instances in NixOS where we let users declare partial orderings using before or after attributes. The pre-commit.nix module could provide that functionality.

+1

If the user has both hpack and cabal2nix hooks enabled (cf. https://github.com/srid/haskell-flake/issues/144), they must be run in that order (otherwise the Nix won't be updated on first run); so cabal2nix.after = [ "hpack" ]; could satisfy that ordering via topsort.

[noamsto]

Did anyone managed to solve it? We have a python project and I want ruff to run before black

[yajo]

No solution yet AFAIK.

I guess it should be relatively easy to add support to a order key in the current schema, and sort hooks by order before producing the final .pre-commit-config.yaml file.

[sandydoo]

Perhaps we could eventually use something like taggedSubmodules: https://github.com/NixOS/nixpkgs/pull/254790

[yajo]

How would that fix this?

[roberth]

I suppose you could then have a listOf (taggedSubmodule m), but listOf doesn't let you merge configurations; only append.

This doesn't depend on taggedSubmodule though. I'd only recommend that type when consumers of the config must have knowledge of each tag anyway, which is not the case here. We have a bounded, generic interface in the form of basically the options in hook.nix. There's no need for a tag, and listOf (submodule ./hook.nix) would be sufficient if you then let users specify default configs with imports. (But listOf is still no good, so please don't. I recommend the before and/or after solution.)

[sandydoo]

I was just wondering what a list-based config for hooks would look like. That would solve the issue of hook order and provide an easy way to repeatedly specify hooks with different options.

taggedSubmodule caught my eye because it would let hooks retain their custom options on top of the generic submodule. And then in theory that could be rendered in the docs.

Using imports is an interesting idea though! I was thinking the tag would let us handle the import behind the scenes.

[sandydoo]

But listOf is still no good, so please don't. I recommend the before and/or after solution.

Now I have to ask! Why not?

[yajo]

Treefmt seems to have it better designed, with a priority field. Maybe we can imitate it.

https://treefmt.com/configure.html

El jue, 16 may 2024, 15:46, sander @.***> escribió:

But listOf is still no good, so please don't. I recommend the before and/or after solution.

Now I have to ask! Why not?

— Reply to this email directly, view it on GitHub https://github.com/cachix/git-hooks.nix/issues/250#issuecomment-2115451691, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHNXDPQYXNHLO3Q3A6VP53ZCTBF3AVCNFSM6AAAAAAVNGWBBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJVGQ2TCNRZGE . You are receiving this because you authored the thread.Message ID: @.***>