Skip to content

Make usable without package distribution #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
1 of 4 tasks
roberth opened this issue Sep 4, 2020 · 10 comments
Open
1 of 4 tasks

Make usable without package distribution #61

roberth opened this issue Sep 4, 2020 · 10 comments

Comments

@roberth
Copy link
Contributor

roberth commented Sep 4, 2020

pre-commit-hooks.nix in its current form is a package distribution by default; using pinned packages.

Some users (me, #55?) prefer to provide their own packages only, in order to have consistent versions everywhere in a project.

I'd be happy to provide a function to do that, which will work at the module level.

TODO

  • write Simplify options #60 to make the module system interface a similar as possible
  • use module system defaults for the default tools
  • set the default tools with a module that can be enabled (and enable it in run so its behavior remains unchanged)
  • expose the function in default.nix

In order to improve performance I'd like to move tools into its own attrset, so we can evaluate the spine of default.nix without having to load the pinned nixpkgs. It's even possible to unbreak the move and provide deprecation warnings.

@blaggacao
Copy link
Contributor

blaggacao commented Sep 4, 2020

If — as a collateral — the code base could be made a little bit easier to grasp, that would be great. I was in sheer despair more than once when working on #52 . — not directly related, though.

@domenkozar
Copy link
Member

Couldn't we just expose pkgs attribute that could be overriden?

@roberth
Copy link
Contributor Author

roberth commented Oct 6, 2020

Couldn't we just expose pkgs attribute that could be overriden?

In the past we've needed a solution for packages that are packaged with pre-commit-hooks.nix itself. Those did not come from pkgs so to support that, we'd need to expose a way to override tools (or something similar).

So either we forgo packaging hooks or the ability to override those, or we allow tools overrides. The latter can already be done if you wire up the modules yourself, so I think we should make run do nothing more than invoke the module system. Not rearranging arguments into a configuration module is a small breaking change iirc; not too bad.

@domenkozar
Copy link
Member

Overriding tools sounds good to me (I forgot we didn't just apply an overlay to pkgs).

@blaggacao
Copy link
Contributor

blaggacao commented Oct 15, 2020

I like the way how devshell presents itself as an overlay, this makes it easier to chain things together. Maybe not 100% on topic, but I figured to best comment here nevertheless: https://github.com/numtide/devshell/blob/master/default.nix

What I had in mind:

{
  nixpkgs = import ./nixpkgs-src.nix;
  overlays = [ 
    (import ./overlays/overlay.nix)
    (import ./overlays/pre-commit.nix)
  ];
  pkgs = (import ./devshell-src.nix) { inherit nixpkgs overlays; };
}

(strictly pinning to a single nixpkgs version maybe also helps reduce the closure of the devshell?)

@u-quark
Copy link
Contributor

u-quark commented Dec 31, 2020

I think people, in general, want to pin the version of tools they use in the pre-commit hooks. That means they need to provide the tools option. This works, even though it is a bit explicit. A problem arises when pre-commit-hooks.nix needs to wrap an external tool with some script (like we do with terraform-fmt). Then the user would need to apply the same wrapper before they pass it into tools. This is not very neat. Maybe we can provide a nixpkgs parameter that the tools will be derived from?

@domenkozar
Copy link
Member

That sounds good to me.

@domenkozar
Copy link
Member

I think people, in general, want to pin the version of tools they use in the pre-commit hooks. That means they need to provide the tools option. This works, even though it is a bit explicit. A problem arises when pre-commit-hooks.nix needs to wrap an external tool with some script (like we do with terraform-fmt). Then the user would need to apply the same wrapper before they pass it into tools. This is not very neat. Maybe we can provide a nixpkgs parameter that the tools will be derived from?

Note that this is done. What we need are docs to override pkgs and also to override a specific tool.

@sir4ur0n
Copy link

Note that this is done. What we need are docs to override pkgs and also to override a specific tool.

Can you please explain how this can be done? 🙏
The function run still does not take any pkgs argument, only tools, and terraform-fmt does not use terraform coming from tools

@domenkozar
Copy link
Member

That one isn't possible to override right now - see #196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants