swellaby / rusty-hook

git hook manager, geared toward Rust projects
MIT License
206 stars 11 forks source link

should download same version of CLI as dev-dependency #136

Closed ezpuzz closed 3 years ago

ezpuzz commented 3 years ago

Environment Details

Description

I wanted to use the multiple commands functionality of rusty-hook on a project (https://github.com/amethyst/amethyst) but hooks were broken for other contributors because the init script only installs the stable version of rusty-hook CLI even though the dev-depency specifies git master. Because of this I have to remove rusty-hook from our repo for now.

calebcartwright commented 3 years ago

Thanks for reaching out!

I wanted to use the multiple commands functionality of rusty-hook

The released versions of rusty-hook already support this. There's an unreleased change that allows for some slightly cleaner TOML syntax to define those commands, but the end result is exactly the same.

e.g., this is how folks run multiple commands today:

pre-push = "cargo check && cargo fmt -- --check"

while the new feature is aimed at letting folks achieve the same thing in a way some find easier to read, especially when you have many commands and/or long ones:

pre-push = [
  "cargo check",
  "cargo fmt -- --check"
]

Internally, rusty-hook literally converts the latter to the former so if you think rusty-hook would be helpful in your project, I wouldn't necessarily let the absence of the array approach discourage you! We'll update the docs to make that more clear than it is right now.

hooks were broken for other contributors because the init script only installs the stable version of rusty-hook CLI even though the dev-depency specifies git master

Understand the situation you're describing but this isn't quite accurate. Adding rusty-hook as a dev dependency on your project is really just shorthand for getting the rusty-hook cli installed and the git hooks initialized in the project for cases where you want to make the setup easier for contributors, but there's important context and nuance in how those are achieved so as to not break any other projects on your machine that are also using rusty-hook.

AFAIK, cargo still doesn't have the notion of project/directory specific binary dev deps, so regardless of how it ends up getting installed (manual vs dev dep) the rusty-hook cli gets placed under the same bin directory under CARGO_HOME (~/.cargo/bin) as any other cargo installed crate. Hypothetically, I suppose some elaborate/complex directory override solution similar to what rustup does for multiplexing could be possible, but I've never seen this with any crates and feel the drawbacks would weigh far too heavily for such an approach to be viable for this project.

The result of all that means that a given point in time there will only be one version of the rusty-hook cli on a given machine, and it's not possible to have different versions per project. Given that, and the fact that in a given env you could have multiple projects with different versions of the dependency, the approach that we take is to ensure that the cli/bin in every release of rusty-hook is fully backwards compatible with the underlying git scripts from every previous release.

This is done so that if you have Project A on your machine which you originally setup with v0.5 and then down the road you start Project B and use v0.11, that when you go back to Project A your git ... commands don't blow up and the hooks still work, all without you having to upgrade Project A to v0.11 too. It's not possible to also have future/forward guarantees though (the v0.5 cli would of course had no idea what the git hooks from v0.11 would be doing/need)

In order to be able to support this behavior which we consider critical, the auto-setup mode can only work with known, released versions, and that bootstrapping/installation process from the scripts will ensure the minimal needed version from crates.io is installed and available.

So while I understand the logical thought behind the suggestion, trying to do it would come with more tradeoffs than benefits even if it were possible (and there's various other reasons beyond those discussed above).

Accordingly, I'm going to go ahead and close this as there's no action to be taken, but I hope the above context was helpful and please let us know if you have any questions!

As an aside, one of the reasons why I haven't released the changes from master yet is because there's some that aren't fully compatible with older versions of the scripts and I don't want to cause problems for any of our existing users. Will hopefully get a chance to do that soon though and then publish a release so the array syntax for multiple commands will be available.