doublify / pre-commit-rust

Rust hooks for pre-commit
171 stars 78 forks source link

Link to my fork for future updates #31

Open FeryET opened 1 year ago

FeryET commented 1 year ago

Hi.

Since this repo has not been updated in a very long time, and since I wanted to add more pre-commit hooks, I decided to fork this repo. The fork is linked here: https://github.com/FeryET/pre-commit-rust

It would be awesome if you could link to my fork as an active continuation of your awesome tool.

Thanks a lot!

ogauthe commented 9 months ago

Hi!

I tried using pre-commit with @FeryET repo. I did not manage to run the hooks with rust as language: pre-commit could not locate a Cargo.toml in the commit hook directory (not in my own code directory). The error message is

An unexpected error has occurred: CalledProcessError: command: ('/mnt/sw/nix/store/2grrzm5kq84sxpxsfwmnzl60p53l1hn6-rust-1.70.0/bin/cargo', 'install', '--bins', '--root', '~/.cache/pre-commit/repolpsfu2ko/rustenv-system', '--path', '.')
return code: 101
stdout: (none)
stderr:
    error: `~/.cache/pre-commit/repolpsfu2ko` does not contain a Cargo.toml file. --path must point to a directory containing a Cargo.toml file.

This was caused by the way pre-commit deals with rust hooks, depending whether the language is system or rust. Commit e6f0aef52974e00b081986c1920a5fc1c1502715 changed the language to rust. For some reason I do not really understand, on my installation pre-commit seems unable to use rust as a language. With language system it executes the function install_rust_with_toolchain in file PATH_TO_PYTHON/site-packages/pre_commit/languages/rust.py and everything is fine.

Took me a bit of time to understand this, probably an issue between pre-commit and my rust setup based on environment module. If anyone has the same issue, the workaround for me was to revert to version 1.0.2, with language set to system.

FeryET commented 9 months ago

@ogauthe I figured out that this was a misunderstanding on my part that using language: rust will equal to the automatic installation of cargo for users. Contrary to my belief, it's for creating binary releases of pre-conmit hooks that are written in Rust.

Anyway, I have reverted the commit, and created a new minor release since I think this is stable.

Thank you for pointing the issue out!

ogauthe commented 9 months ago

I confirm everything is fine with the latest release. Thank you!

tyilo commented 6 months ago

@FeryET If you want your fork to be the new default repo, then you should enable issues.

FeryET commented 6 months ago

@FeryET If you want your fork to be the new default repo, then you should enable issues.

Oh my bad. Didn't know I had to manually enable the issues. It's enabled as of now.