Skip to content
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

add treefmt.withConfig #169

Merged
merged 3 commits into from
Sep 21, 2022
Merged

add treefmt.withConfig #169

merged 3 commits into from
Sep 21, 2022

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented May 20, 2022

This is useful if you want to configure treefmt with nix, and precisely
pass all the commands from nixpkgs.

@srid
Copy link
Contributor

srid commented May 29, 2022

cf #165 (comment)

.envrc Outdated
@@ -0,0 +1,2 @@
export PRJ_ROOT=$PWD
Copy link
Contributor

@srid srid May 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting trick! Does direnv always use project root (parent dir of .direnv file) as PWD even if you open the terminal directly in one of its sub directories?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is necessary so that relative paths in the .envrc have the same result, even if the user jumps into a sub-folder directly.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DIRENV_DIR already exists and can be queried if you use direnv, there may be no need to set PRJ_ROOT then.

@srid
Copy link
Contributor

srid commented May 29, 2022

@zimbatm What do you think of exposing a flakeModule (a la cachix/git-hooks.nix#130) so that projects that already use flake-parts can use this directly, thus resolving #165?

@zimbatm
Copy link
Member Author

zimbatm commented May 30, 2022

Sounds good, the flake module probably can call treefmt.withConfig with its resulting schema at the end.

default.nix Outdated Show resolved Hide resolved
@srid
Copy link
Contributor

srid commented May 30, 2022

Sounds good, the flake module probably can call treefmt.withConfig with its resulting schema at the end.

Great; I'd be happy to test it out here:

https://github.com/srid/haskell-template/blob/6c9e2eb0870b108731df87cdcbb016c064e55041/flake.nix#L19-L25

@zimbatm zimbatm changed the title WIP: add treefmt.withConfig add treefmt.withConfig Jun 7, 2022
@zimbatm zimbatm marked this pull request as ready for review June 7, 2022 16:43
@zimbatm
Copy link
Member Author

zimbatm commented Jun 7, 2022

@srid ok this is ready if you want to give it a try

@srid
Copy link
Contributor

srid commented Jun 11, 2022

@zimbatm Trying it out on haskell-template,

diff --git a/flake.nix b/flake.nix
index 9918a57..684242e 100644
--- a/flake.nix
+++ b/flake.nix
@@ -5,22 +5,23 @@
     flake-parts.url = "github:hercules-ci/flake-parts";
     flake-parts.inputs.nixpkgs.follows = "nixpkgs";
     haskell-flake.url = "github:srid/haskell-flake";
+    treefmt.url = "github:numtide/treefmt/withConfig";
+    treefmt.flake = false;
   };
 
-  outputs = { self, nixpkgs, flake-parts, haskell-flake, ... }:
+  outputs = inputs@{ self, nixpkgs, flake-parts, haskell-flake, treefmt, ... }:
     flake-parts.lib.mkFlake { inherit self; } {
       systems = nixpkgs.lib.systems.flakeExposed;
       imports = [
         haskell-flake.flakeModule
       ];
-      perSystem = { self', pkgs, ... }: {
+      perSystem = { self', system, pkgs, ... }: {
         haskellProjects.default = {
           root = ./.;
           buildTools = hp: {
-            # TODO: Use https://github.com/numtide/treefmt/pull/169
             inherit (pkgs)
-              treefmt
               nixpkgs-fmt;
+            treefmt = (import treefmt { inherit system; nixpkgs = pkgs; }).treefmt.withConfig (pkgs.lib.importTOML ./treefmt.toml);
             inherit (hp)
               cabal-fmt
               fourmolu;

I see:

srid on now haskell-template on  master [!] via λ 9.0.2 via ❄️  impure (ghc-shell-for-haskell-template-0.1.0.0) 
❯ more $(which treefmt)
#!/nix/store/14lypyys4gfcl982rjddxa6jg7msqz9q-bash-5.1-p16/bin/bash
exec /nix/store/d88p6rcrpp83ir7i6lasljf871j34g25-treefmt/bin/treefmt --config-file /nix/store/vfblhmhr809df21cvcnh7mdwpqn0nv9w-treefmt.toml "$@"


srid on now haskell-template on  master [!] via λ 9.0.2 via ❄️  impure (ghc-shell-for-haskell-template-0.1.0.0) 
❯ treefmt
[ERR]: If --config-file is set, --tree-root must also be set

I wanted to see how importTOML behaved before switching to config defined entirely in Nix, but as you can see it looks like I need this PRJ_ROOT env var and pass it via --tree-root, which in turn would require direnv or devshell. Therefore, it seems to me that treefmt.withConfig as it is implemented currently wouldn't work with projects that use neither direnv nor devshell.

EDIT: I suppose we could add PRJ_ROOT to shellHook, but that would still add 'low-level' nix to flake.nix.

@zimbatm
Copy link
Member Author

zimbatm commented Jun 15, 2022

agreed. And since the absolute path to the project root would vary from machine to machine, I don't have a really good solution for it. Unless maybe the function also takes a command that points to the project root, like git rev-parse --show-toplevel

@zimbatm zimbatm force-pushed the withConfig branch 2 times, most recently from 01b66aa to 582c5aa Compare July 5, 2022 08:59
@srid
Copy link
Contributor

srid commented Aug 17, 2022

What if we had a wrapper script to determine the project root by traversing until a parent directory with a flake.nix in it? It does assume that the project has no sub-flakes in it.

@srid
Copy link
Contributor

srid commented Aug 17, 2022

What if we had a wrapper script to determine the project root by traversing until a parent directory with a flake.nix in it? It does assume that the project has no sub-flakes in it.

We can refine this idea to avoid the sub-flake assumption as follows:

formatter = pkgs.treefmt.withConfig (nixpkgs.lib.importTOML ./treefmt.toml) { projectRootFile = "flake.nix" };

Basically, this tells the wrapper script to cd ../. until the projectRootFile can be located. If the project has sub-flakes, the user can set it to an alternative file, like { projectRootFile = "cabal.project"; }.

@zimbatm If we can get this feature merged, it would be useful in Platonic-Systems/treefmt-flake#1. Ultimately, I imagine that treemfmt-flake can be upstreamed to the flake.nix of this repo.

@zimbatm
Copy link
Member Author

zimbatm commented Aug 18, 2022

yeah, that makes sense. Do you have the time to finish this PR so it takes pkgs.treefmt.withConfig { config = ...; projectRootFile = "..."; }, all of this gets passed to the module system, and it outputs the wrapper?

@srid
Copy link
Contributor

srid commented Aug 23, 2022

@zimbatm Let me know what you think: #177

@zimbatm
Copy link
Member Author

zimbatm commented Aug 23, 2022

ok, I think this is ready now. Any last words/feedback?

flake.nix Outdated
@@ -20,6 +20,14 @@

packages = pkgs;

# In Nix 2.8 you can run `nix fmt` to format this whole repo. Note that you need to have loaded the
# `nix develop` shell before so the various formatters are available in the PATH.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's true only if the dependencies are not specified in nix. if the config has references to packages then it should work even outside of nix develop

@srid
Copy link
Contributor

srid commented Aug 23, 2022

ok, I think this is ready now. Any last words/feedback?

I've tested it to work after applying #178

Looks good to merge. I'd only suggest making wrapper.projectRootFile = "flake.nix"; the default so the user doesn't have to specify it explicitly on every project using flakes (but without a sub-flake, as is usually the case).

@zimbatm
Copy link
Member Author

zimbatm commented Aug 24, 2022

Rebased. It looks like flake.parts doesn't support the formatter option yet. I will give it another shot next week.

@zimbatm zimbatm force-pushed the withConfig branch 2 times, most recently from 574b533 to 19a0b32 Compare August 24, 2022 10:41
default.nix Outdated Show resolved Hide resolved
@zimbatm
Copy link
Member Author

zimbatm commented Sep 19, 2022

Blocked on hercules-ci/flake-parts#56

bors bot added a commit to hercules-ci/flake-parts that referenced this pull request Sep 20, 2022
56: add formatter schema r=roberth a=zimbatm

This is needed for flakes that want to support `nix fmt`.

Right now it's missing some testing as I didn't find a test framework for it.

Needed by numtide/treefmt#169


Co-authored-by: zimbatm <[email protected]>
Co-authored-by: Jonas Chevalier <[email protected]>
bors bot added a commit to hercules-ci/flake-parts that referenced this pull request Sep 20, 2022
56: add formatter schema r=roberth a=zimbatm

This is needed for flakes that want to support `nix fmt`.

Right now it's missing some testing as I didn't find a test framework for it.

Needed by numtide/treefmt#169


Co-authored-by: zimbatm <[email protected]>
Co-authored-by: Jonas Chevalier <[email protected]>
@zimbatm zimbatm force-pushed the withConfig branch 3 times, most recently from 4d960bc to 1bac2bc Compare September 21, 2022 11:20
@zimbatm
Copy link
Member Author

zimbatm commented Sep 21, 2022

ok, I think this is ready

@zimbatm zimbatm force-pushed the withConfig branch 2 times, most recently from f44778c to f11a866 Compare September 21, 2022 15:10
.envrc Outdated Show resolved Hide resolved
Co-authored-by: Jörg Thalheim <[email protected]>
@zimbatm zimbatm force-pushed the withConfig branch 2 times, most recently from 905fe05 to de9885d Compare September 21, 2022 16:37
This is useful if you want to configure treefmt with nix, and precisely
pass all the commands from nixpkgs.

Co-authored-by: Sridhar Ratnakumar <[email protected]>
@zimbatm zimbatm merged commit 9615c40 into master Sep 21, 2022
@zimbatm zimbatm deleted the withConfig branch September 21, 2022 16:54
@srid
Copy link
Contributor

srid commented Sep 21, 2022

@zimbatm Will this (withConfig) be upstreamed to nixpkgs at some point?

@zimbatm
Copy link
Member Author

zimbatm commented Sep 21, 2022

I'm unsure about the policy regarding nix modules in the pkgs/ tree. Are there any precedents?

@srid
Copy link
Contributor

srid commented Sep 21, 2022

@zimbatm
Copy link
Member Author

zimbatm commented Sep 23, 2022

that's not quite the same, plus it's using IFD :-D

With @Mic92 we discussed moving this to another repo, so all the module systems could live separate from the code, and include pre-made samples for the various programs.

@zimbatm
Copy link
Member Author

zimbatm commented Sep 23, 2022

Now live at https://github.com/numtide/treefmt-nix . Looking for feedback!

@srid
Copy link
Contributor

srid commented Sep 23, 2022

Looking for feedback!

Sure: numtide/treefmt-nix#2

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

Successfully merging this pull request may close these issues.

4 participants