cachix / git-hooks.nix

Seamless integration of https://pre-commit.com git hooks with Nix.
Apache License 2.0
537 stars 155 forks source link

`rustfmt` and `clippy` fail with "No such file or directory" #126

Open ambroisie opened 3 years ago

ambroisie commented 3 years ago

Steps to reproduce:

With the following flake.nix:

{
  description = "Testing pre-commits";

  inputs = {
    flake-utils = {
      type = "github";
      owner = "numtide";
      repo = "flake-utils";
      ref = "master";
    };

    naersk = {
      type = "github";
      owner = "nix-community";
      repo = "naersk";
      ref = "master";
      inputs = {
        nixpkgs.follows = "nixpkgs";
      };
    };

    nixpkgs = {
      type = "github";
      owner = "NixOS";
      repo = "nixpkgs";
      ref = "nixpkgs-unstable";
    };

    pre-commit-hooks = {
      type = "github";
      owner = "cachix";
      repo = "pre-commit-hooks.nix";
      ref = "master";
      inputs = {
        flake-utils.follows = "flake-utils";
        nixpkgs.follows = "nixpkgs";
      };
    };

    rust-overlay = {
      type = "github";
      owner = "oxalica";
      repo = "rust-overlay";
      ref = "master";
      inputs = {
        flake-utils.follows = "flake-utils";
        nixpkgs.follows = "nixpkgs";
      };
    };
  };

  outputs =
    { self
    , flake-utils
    , naersk
    , nixpkgs
    , pre-commit-hooks
    , rust-overlay
    }:
    {
      overlay = final: prev: {
        foo =
          let
            inherit (final) system;
            overlays = [
              (import rust-overlay)
            ];
            pkgs = import nixpkgs { inherit overlays system; };
            my-rust = pkgs.rust-bin.stable.latest;
            naersk-lib = naersk.lib."${system}".override {
              cargo = my-rust.default;
              rustc = my-rust.default;
            };
          in
          naersk-lib.buildPackage {
            pname = "foo";
            version = "0.0.0";

            src = ./.;

            passthru = {
              inherit my-rust;
            };
          };
      };
    } // (flake-utils.lib.eachDefaultSystem (system:
    let
      overlays = [ self.overlay ];
      pkgs = import nixpkgs { inherit overlays system; };
      inherit (pkgs) lib;
      inherit (pkgs.foo.passthru) my-rust;
    in
    rec {
      checks = {
        pre-commit = pre-commit-hooks.lib.${system}.run {
          src = ./.;

          # FIXME: clippy and rustfmt fail when inside `nix flake check`
          # but `pre-commit run --all` works properly...
          # Also a problem when not overriding the `entry`
          hooks = {
            clippy = {
              enable = true;
              entry = lib.mkForce "${my-rust.clippy}/bin/cargo-clippy clippy";
            };

            nixpkgs-fmt = {
              enable = true;
            };

            rustfmt = {
              enable = true;
              entry = lib.mkForce "${my-rust.rustfmt}/bin/cargo-fmt fmt -- --check --color always";
            };
          };
        };
      };

      defaultPackage = packages.foo;

      devShell = pkgs.mkShell {
        inputsFrom = [
          defaultPackage
        ];

        nativeBuildInputs = with pkgs; [
          rust-analyzer
        ];

        inherit (checks.pre-commit) shellHook;

        RUST_SRC_PATH = "${my-rust.rust-src}/lib/rustlib/src/rust/library";
      };

      packages = {
        inherit (pkgs) foo;
      };
    }));
}

And executing the following commands to create a simple rust project:

nix shell nixpkgs#cargo -c cargo init .
# Create a Cargo.lock for naersk
nix shell nixpkgs#cargo -c cargo update
nix flake check

You get the following errors in the log:

clippy...................................................................Failed
- hook id: clippy
- exit code: 101

thread 'main' panicked at 'could not run cargo: Os { code: 2, kind: NotFound, message: "No such file or directory" }', src/tools/clippy/src/main.rs:163:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

nixpkgs-fmt..............................................................Passed
rustfmt..................................................................Failed
- hook id: rustfmt
- exit code: 1

No such file or directory (os error 2)

Even without overriding the entry attribute of clippy and rustfmt, I still get the same error.

garbas commented 2 years ago

This solved it for me. Clearly is a hack but I haven't yet look how we can fix this in pre-commit-hooks.nix.

           pkgs.buildEnv {
              name = my-rust.name;
              inherit (my-rust) meta;
              buildInputs = [ pkgs.makeWrapper ];
              paths = [ my-rust ];
              pathsToLink = [ "/" "/bin" ];
              postBuild = ''
                for i in $out/bin/*; do
                  wrapProgram "$i" --prefix PATH : "$out/bin"
                done

              '';
            };

The problem was that cargo-fmt needs to see rustfmt binary. This was the hint that help me -> https://github.com/rust-lang/rustfmt/issues/1071#issuecomment-228013196

domenkozar commented 2 years ago

Could someone open a PR?

K900 commented 2 years ago

I think what needs to happen here is you need to actually run in an environment that has rustc, cargo, clippy and all the other stuff in the same environment, so it gets kinda tricky.

winterqt commented 2 years ago

Hey folks, I tried to fix this with #156. I've lightly tested it and it does seem to fix the issues described here. Let me know if there's anything I missed. Thanks!

m1-s commented 2 years ago

@winterqt This still is b0rken for me. Requesting to re-open this ticket.

I have created a minimal reproducible example at github.com/m1-s/pre-commit-hooks-nix-clippy.

When executing nix run "git+ssh://git@github.com/m1-s/pre-commit-hooks-nix-clippy?ref=main" -L both clippy and rustfmt fail for me.

winterqt commented 2 years ago

Ah, I see. Wonder why it's behaving differently in that context -- it's in PATH either way, so that's... weird.

I'll take a look at this soon, in the meantime I do agree that this ticket should be reopened.

winterqt commented 2 years ago

@m1-s Your clippy issue was caused by the fact that you have a dependency (and no Cargo.lock for a binary, but still the same issue), and the rustfmt hook is fixed with this patch:

diff --git a/modules/hooks.nix b/modules/hooks.nix
index b97badb..d55ceb5 100644
--- a/modules/hooks.nix
+++ b/modules/hooks.nix
@@ -313,7 +313,7 @@ in
             nativeBuildInputs = [ pkgs.makeWrapper ];
             postBuild = ''
               wrapProgram $out/bin/cargo-fmt \
-                --prefix PATH : ${lib.makeBinPath [ tools.cargo ]}
+                --prefix PATH : ${lib.makeBinPath [ tools.cargo tools.rustfmt ]}
             '';
           };
         in

Going back to the first issue, it seems like... well, nobody, has tested the check aspects of this with a project that has dependencies. We obviously need to have a way to download these dependencies into the build directory to be able to check them, which these hooks currently don't provide.

I have an idea that involves using importCargoLock to automatically do this for the cargo-check and clippy hooks, if @domenkozar is okay with the idea I can (try to) write + PR an implementation of it.

lafrenierejm commented 2 years ago

the rustfmt hook is fixed with [adding tools.rustfmt to makeBinPath's argument]

Thanks for the patch, @winterqt! I can confirm that it fixed the rustfmt hook for me.

m1-s commented 2 years ago

@winterqt I cant speak for @domenkozar but I would love to see a PR that fixes clippy.