Add `flake-parts` module #14

srid · 2022-12-21T18:00:46Z

I'm opening a draft PR (instead of waiting for full implementation) just to get something working, as well as to get any early feedback. The commits in this PR can be squash merged (or I can rebase them if necessary) once ready.

This PR adds a flake-parts module for working with treefmt in the most ergonomic way. It also adds a flake check to test in CI that the source tree is formatted. You can see a demo here: srid/haskell-template#75

Resolves #1

Can the types.raw be replaced with appropriate types? See also: API feedback #2
Add documentation

roberth

Maybe I should read more of the treefmt implementation, but I'm confused by the supposed submodule-ish options in this PR. Doesn't treefmt already do its things in modules, that just need to be imported?

flake-module.nix

srid · 2022-12-21T18:21:24Z

Maybe I should read more of the treefmt implementation, but I'm confused by the supposed submodule-ish options in this PR. Doesn't treefmt already do its things in modules, that just need to be imported?

The module options are spread across several files: there is one module called module-options and several program specific ones in the programs directory. They are passed to evalModules here:

treefmt-nix/default.nix

Lines 23 to 25 in 4230552

    
             module-options 
        
           ] 
        
           ++ programs.modules

Is there a straightforward way to "merge" them to produce a single submodule for use in flake-parts options?

roberth · 2022-12-21T18:30:53Z

Is there a straightforward way to "merge" them to produce a single submodule for use in flake-parts options?

imports = [ ], or submoduleWith { modules = [ ... ]; } instead of submodule.

Another trick is you could use (<<treefmt>>.evalModule foo {}).type, which is a submodule type with all treefmt stuff in it. This doesn't show intent like it should, so perhaps it's better to refactor lib.nix a bit to extract the logic leading up to lib.evalModules into a function. Then you can do submoduleWith { modules = <<treefmt>>.prepareModule foo bar; }. It's the same thing, but less likely to be accidentally broken.

zimbatm

nice

flake-module.nix

Because treefmt 0.5 has the fix.

srid · 2022-12-21T21:00:12Z

error: The option `build.wrapper' is defined multiple times.

Actually, I found a solution to this: need to move the wrapper definition from "config" to under "options" (as default). Why is that? 🤷🏿 I wish Nix had Haskell-like type system ...

This still won't work; see follow-up commit.

This needed to make it work with `flake-parts`, otherwise we get: error: The option `build.wrapper' is defined multiple times.

flake-module.nix

zimbatm · 2022-12-21T23:54:26Z

flake-module.nix

+                  (import ./.).evalModule pkgs config.treefmt.config;
+              };
+              programs = mkOption {
+                type = types.attrsOf types.package;


Suggested change

type = types.attrsOf types.package;

type = types.listOf types.package;

A set of all the packages isn't it?

I used an attrset rather than a list - because, while the likes of buildInputs does take a list, in other places attrsets are used.

For example, haskell-flake's buildTools is an attrset of packages, because it allows the user to override the default build tools to disable some of them (by setting { foo = null; }.

buildTools = hp: { treefmt = config.treefmt.wrapper; } // config.treefmt.programs;

https://github.com/srid/haskell-template/blob/1706660e7b02ece881a67c99692b70db5ae5c8db/flake.nix#L28-L30

I also figured it would be nice to "tag" each program with the formatter it is associated with it, so the caller can do any special processing that way. If we make this use listOf instead, then haskell-template will have to re-implement the logic.

flake-module.nix

srid · 2022-12-22T01:28:06Z

This is ready for review!

@zimbatm Let me know what you decide about the type of the programs option.

README.md

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>

module-options.nix

zimbatm · 2022-12-23T13:18:40Z

flake-module.nix

+        };
+      });
+  };
+}


I like how smaller this module has become. Nice :)

Maybe a last question to @roberth mostly; given that flake-parts has a global namespace, do you think we should prefix treefmt into something like programs.treefmt for example?

I don't mind a flat namespace at all. And maybe treefmt is more than a program?
In Nixpkgs we're actually trying to get rid of the categorization hierarchy for various reasons. RFC coming soon, but preparations are happening here https://github.com/nixpkgs-architecture/simple-package-paths

roberth · 2022-12-24T16:46:51Z

Made a PR to add it to flake.parts if that's ok.
Not sure about the naming. The repo is treefmt-nix but the tool and option names are treefmt. What do you think?

Add treefmt-nix hercules-ci/flake.parts-website#65

I'll add a link to the rendered docs there.

Mic92 · 2022-12-27T11:20:19Z

How can I access treefmt itself for interactive usage?

Mic92 · 2022-12-27T11:21:52Z

perSystem { config }: = {
     packages.treefmt = config.treefmt.build.wrapper;
};

Would it make sense to add this output?

Mic92 · 2022-12-27T11:49:32Z

#18

srid · 2022-12-27T14:31:29Z

@Mic92 Does https://flake.parts/debug.html help?

I feel like unconditionally adding packages.treefmt (#18) is unnecessary in most cases. It can also feel intrusive; for example, I don't want packages.treefmt showing up CI jobs of every project of mine using this flake module. Perhaps it can be made conditional based on an enable option?

Mic92 · 2022-12-28T09:53:55Z

I think instead of a package it could also become module documentation (i.e. an example that adds the wrapped treefmt to a devshell) but just now it is not very visible without digging into the code how one can access treefmt otherwise.

Sridhar Ratnakumar added 2 commits December 21, 2022 12:52

Add flake-module

3672f5d

Fix

8280dec

roberth reviewed Dec 21, 2022

View reviewed changes

flake-module.nix Outdated Show resolved Hide resolved

Sridhar Ratnakumar added 2 commits December 21, 2022 13:16

Define config alongside the relevant option

974b5b3

flake-module.nix need not be a function

bfc8367

zimbatm reviewed Dec 21, 2022

View reviewed changes

flake-module.nix Outdated Show resolved Hide resolved

flake-module.nix Outdated Show resolved Hide resolved

No need to set $HOME

c9cc930

Because treefmt 0.5 has the fix.

This comment was marked as resolved.

Sign in to view

Sridhar Ratnakumar added 3 commits December 21, 2022 16:01

Factor out module definition in 'all-modules' var

d8901a3

Specify 'config' type using existing module options

d7d037b

This still won't work; see follow-up commit.

Move wrapper definition to option default

05c06a8

This needed to make it work with `flake-parts`, otherwise we get: error: The option `build.wrapper' is defined multiple times.

zimbatm reviewed Dec 21, 2022

View reviewed changes

Sridhar Ratnakumar added 4 commits December 21, 2022 20:03

Upstream 'check'

cdf0567

Trim check's buildInputs

10dfbea

Remove the top-level indirection configs

1671b29

Add documentation

5694629

srid changed the title ~~WIP: Add flake-parts module~~ Add flake-parts module Dec 22, 2022

srid marked this pull request as ready for review December 22, 2022 01:25

srid requested review from zimbatm and roberth and removed request for zimbatm December 22, 2022 01:27

srid requested review from zimbatm and removed request for roberth December 22, 2022 01:28

roberth reviewed Dec 22, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

f9d04b6

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>

roberth reviewed Dec 22, 2022

View reviewed changes

module-options.nix Show resolved Hide resolved

Add defaultText

2713e6d

zimbatm reviewed Dec 23, 2022

View reviewed changes

zimbatm merged commit 9798131 into numtide:master Dec 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `flake-parts` module #14

Add `flake-parts` module #14

srid commented Dec 21, 2022 •

edited

Loading

roberth left a comment

srid commented Dec 21, 2022 •

edited

Loading

roberth commented Dec 21, 2022

zimbatm left a comment

This comment was marked as resolved.

This comment was marked as resolved.

srid commented Dec 21, 2022

zimbatm Dec 21, 2022

srid Dec 22, 2022

srid commented Dec 22, 2022 •

edited

Loading

zimbatm Dec 23, 2022

roberth Dec 23, 2022

roberth commented Dec 24, 2022

Mic92 commented Dec 27, 2022

Mic92 commented Dec 27, 2022

Mic92 commented Dec 27, 2022

srid commented Dec 27, 2022

Mic92 commented Dec 28, 2022 •

edited

Loading

	type = types.attrsOf types.package;
	type = types.listOf types.package;

Add flake-parts module #14

Add flake-parts module #14

Conversation

srid commented Dec 21, 2022 • edited Loading

roberth left a comment

Choose a reason for hiding this comment

srid commented Dec 21, 2022 • edited Loading

roberth commented Dec 21, 2022

zimbatm left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

srid commented Dec 21, 2022

zimbatm Dec 21, 2022

Choose a reason for hiding this comment

srid Dec 22, 2022

Choose a reason for hiding this comment

srid commented Dec 22, 2022 • edited Loading

zimbatm Dec 23, 2022

Choose a reason for hiding this comment

roberth Dec 23, 2022

Choose a reason for hiding this comment

roberth commented Dec 24, 2022

Mic92 commented Dec 27, 2022

Mic92 commented Dec 27, 2022

Mic92 commented Dec 27, 2022

srid commented Dec 27, 2022

Mic92 commented Dec 28, 2022 • edited Loading

Add `flake-parts` module #14

Add `flake-parts` module #14

srid commented Dec 21, 2022 •

edited

Loading

srid commented Dec 21, 2022 •

edited

Loading

srid commented Dec 22, 2022 •

edited

Loading

Mic92 commented Dec 28, 2022 •

edited

Loading