Giter Site home page Giter Site logo

Comments (12)

calebcartwright avatar calebcartwright commented on June 2, 2024 2

I think the fundamental issue here is that you're trying to project too many responsibilities on rusty-hook, and I still feel like you're not reading my comments or perhaps I'm just being unclear. As such I'll try to rephrase a few things but going to go ahead and close this as I feel like we've arrived at a natural conclusion.

rusty-hook does nothing more, and nothing less, than wiring up a git repository to be able to execute a user-defined command or script as simply as possible, namely, by allowing users to specify their desired entry point script/command to run whenever a certain git event occurs. This is where rusty-hook hands off execution to the user/team/project to take whatever steps they want on that git event. Full stop.

In case a visual helps, the flow is:

User --> git --> git hook file --> rusty-hook --> respective-user-configured-command-or-script (user's responsibility)

rusty-hook is functionally done once it invokes your configured command/script, and rusty-hook is only intended to be invoked by the rusty-hook managed git hook files. The whole purpose of rusty-hook is to provide an "easy button" mechanism to have local git hooks, so that users do not have to worry about, write, nor even see the underlying hook files git invokes. rusty-hook periodically updates these hook files and would override any manual changes made to the files.

rusty-hook users should never modify the rusty-hook managed git hook files, and I've opened #162 as an action item for us to document this/make it explicit. As noted a few times, there's lots of ways of managing local git hooks. With rusty-hook you never work with the underlying git hook scripts, however, you seem to have a strong preference for trying to manage things at this level. If that's indeed the case, I'd encourage you to take a look at some alternative tools (we list some at the bottom of our own readme) which do require/allow the user to define and manipulate the underlying hook scripts.

Running rusty-hook init from one of the sub-projects generated configuration that doesn't work because it's looking for Cargo.toml in the wrong location

I'm not sure why you're zoomed into the Cargo.toml file, rusty-hook does not look for, nor depend on a Cargo.toml file, ever. rusty-hook is developed in Rust, and certainly geared towards Rust-based projects, but there's technically nothing that prevents someone from using it elsewhere. For example (albeit a rather contrived one), one could use rusty-hook with a JavaScript or TypeScript project that utilizes npm, so long you've got the rusty-hook bin on your path:

cd my-js-project
rusty-hook init
# .rusty-hook.toml
[hooks]
pre-commit = "npm test && npm run lint"

No project/workspace Cargo.toml required. Again, rusty-hook handles the process of wiring up the underlying git pieces and providing an easy entry point for custom command/script invocation when the hooks are triggered, that's all.

What those actual commands/scripts are, and the handling of any project/directory context is your responsibility as the user. You have 1 git repository and 1 set of git hooks, so if you want or need to impose any conditionality based on files/directories/modified content etc. then it's your responsibility to handle that within whatever script/command you've configured rusty-hook to trigger. If you need a script/command to be executed from a different directory, then it's your responsibility to change/set the directory context (e.g. with cargo invocations either changing into a directory containing the Cargo.toml file that cargo depends on, or utilizing cargo's --manifest-path flag path to tell cargo where to look)

I did find having to specify the --manifest-path argument in the .rusty-hook.toml file instead to be less intuitive than changing the working directory and thought that might be helpful usability feedback.

As mentioned above, rusty-hook is really only intended to be invoked by the rusty-hook git hook files. rusty-hook is not intended to be a general purpose config-file-drive command runner, and such a general purpose usage pattern is completely outside our scope/not something we intend to support.

As stated a few times on this thread and others, rusty-hook executes whatever corresponding user-defined-command-or-script from the git repository root, and this is very much by design because the usage context is strictly for when it is triggered by git hooks. Please remember that within the hook script file that git invokes, the "working directory" is not the user's working directory; it's not the directory where the user invoked git commit ... or whatever command, it's git managed context but which does have some variance and can be overridden in certain events which in turn necessitates standardization.

As such, if in any project, regardless of whether it's a monorepo or something else, if the commands/scripts you'd like to run need to be invoked from a specific directory, it is your responsibility to set the directory context accordingly with rusty-hook providing a guaranteed consistent starting directory (the git repo root) in 100% of cases.

Given this typo of repository layout, how do the sub-projects using Rust add pre-commit hooks independent of other sub-projects?

I feel like this has already been asked and answered, including with one specific example given in #157 (comment)

To repeat, you have 1 git repository and 1 set of git hook triggers. If you are in a monorepo scenario and want to do multiple things in multiple projects with different tooling/language/etc. constraints, then you need to structure your desired command/script to that effect. If you only want to run checks in projects based on certain conditions (e.g. if there were modified files), then that is your responsibility to define within whatever main hook-entry point you define.

e.g.

# rusty-hook.toml
pre-commit = "sh diff_based_pre-commit.sh"
# diff_based_pre_commit.sh

# inspect the diff
# if changes to cli dir
    # cargo fmt --manifest-path cli/Cargo.toml ....
    # or, change into the directory yourself
    # cd cli && cargo clippy ....
# fi

# if changes to webapp dir
   # or perhaps consider a pattern where each project/subdirectory defines it's own steps
   # e.g.
   # ./webapp/hook/pre-commit.sh
# fi

# if changes to other projects
# etc.

Again, I understand what you're saying that it'd be useful if rusty-hook could also work as a general purpose command runner so that you could utilize it again with a call flow that looks more like:

User --> git --> git hook file --> rusty-hook --> respective-user-configured-command-or-script --> rusty-hook --some-other-config-file --> another-user-configured-command-or-script --> ....

But that's not an in scope goal and not something we're going to support because that general purpose usage pattern adds undo complexity and challenges on the goals that are in scope.

You may want to check out something like cargo make for that more powerful/general purpose command runner function, which you could then utilize as the thing you have rusty-hook invoke for you when the git hooks are triggered, e.g.

User --> git --> git hook file --> rusty-hook --> cargo make ...


Please feel free to follow up if you have any final questions, but as noted in my opening I am going to close this issue as there's no action to be taken and I feel the topics have been addressed pretty thoroughly. I'm sorry rusty-hook hasn't really melded into the full scope of your desired usage model, but hope the above provides a clearer picture on what is in scope for us, and what isn't. If nothing else, I appreciate you reaching out as the dialog has underscored some known cases where we need some documentation improvements, and also identified some new cases where we need to document things explicitly.

from rusty-hook.

calebcartwright avatar calebcartwright commented on June 2, 2024

Can you share the structure of your project, and the contents of your rusty-hook config file?

from rusty-hook.

nirvdrum avatar nirvdrum commented on June 2, 2024

Sure. This particular project is a set of solutions to a text book I've been working through. So, I've isolated each chapter into its own Cargo project:

> ls
chap01/  chap02-03/  chap04/  chap05/  chap06/  chap07/  chap08/  chap09/  chap10/  tiger/

Each such folder is a simple Rust binary. I can go into any one of them and run cargo build, cargo test, cargo clippy and so on. For now, I'm just using the default generated .rusty-hook.toml file. I've updated to supply the --manifest-path as noted above:

[hooks]
pre-commit = "cargo test --manifest-path chap10/Cargo.toml"

[logging]
verbose = true

This works, but is naturally limited to that one directory. That's fine most of the time because I don't frequently revisit older chapters in the book. I just hit a lot of bumps getting that far. That said, I have worked on other mono-repo projects where that would be a problem. E.g., a GraphQL server and client as separate projects within the same repo.

My initial intention had been to store a .rusty-hook.toml per-project and then use one of the tricks that other mono-repo projects use to only run the check for that Rust project if the commit consists of files changed from that project. In that case, I'd need to to be able to do something like rusty-hook --config-file ${extracted_project_dir}/.rusty-hook.toml. I suppose even if I just unconditionally the checks for each project, it'd be helpful if they could have their own configuration.

from rusty-hook.

nirvdrum avatar nirvdrum commented on June 2, 2024

I've updated the config file to contain all the checks I'd like to run. Repeating the --manifest-path part gets a bit verbose:

[hooks]
pre-commit = "cargo fmt --manifest-path chap10/Cargo.toml --all -- --check && cargo clippy --manifest-path chap10/Cargo.toml --tests --all-features -- -D warnings && cargo test --manifest-path chap10/Cargo.toml"

[logging]
verbose = true

from rusty-hook.

calebcartwright avatar calebcartwright commented on June 2, 2024

Thanks for the extra info. There's a lot of content in the issue description and follow up comments, so I'm going to break up my responses into separate comments as well to hopefully make them a bit more readable.

This particular project is a set of solutions to a text book I've been working through. So, I've isolated each chapter into its own Cargo project

I'm probably sharing things you already know, but worth noting that most of the time in these situations you'd want to setup a Cargo workspace, most likely as a virtual workspace in this particular scenario. While it's not strictly necessary it's an extremely common pattern that you can find in projects like the compiler itself, tokio, etc.

If you were to define a workspace and add those projects as members then your cargo subcommands are all available when running from the root directory of your project, whereas that'll fail today. I realize you're a bit zoomed in on a particular project, but I do want to underline this because when you want to run anything (fmt, clippy, etc.) today manually/locally you'd have to do the exact same thing in including the --manifest-path opt, or changing into the directory.

I think it'd be helpful if the pre-commit hook and its interaction with the rusty-hook executable honored the working directory, with the default being the repository's root
I haven't looked at the source, but rusty-hook appears to ignore the working directory

There's a small chance I am remembering this incorrectly, but I'm pretty sure git itself manages the working directory when invoking the hook scripts, and that it always changes the directory to the tree root/git_dir. Additionally, the set of params git provides to hooks naturally varies on the hook, but to my knowledge those params don't include the directory where the hook-triggering command was executed, nor am I aware of a means for hooks to figure that out themselves in a deterministic way.

tl;dr:

  • git invokes the corresponding hook(s) from the root, info about the user's working directory when running git ... is unknown/lost to the hook/things the hook subsequently invokes
  • rusty-hook expects the config file to exist in the project root (same level as the .git directory). We can make this more explicit in the docs, but the file has to be in a known place
  • rusty-hook currently executes the corresponding command(s) for the hook defined in the config file from the same project root directory

That last one is the source of your problem because you want to run cargo commands as part of your hook, but you haven't set up your project in the standard way to be able to run cargo commands from your project root.

from rusty-hook.

calebcartwright avatar calebcartwright commented on June 2, 2024

It may just be that I have a niche use case and what I'm looking for is out of scope for this project. If so, feel free to close out. But, I think there could be some improvements to the user experience, whether that be to correctly report that this configuration isn't supported or to add support for it.

In times like this I think it's important to try to separate the "what" and "why" from a specific "how". For example, it's unclear to me from the discussion above and on the other thread whether you're explicitly trying to only have client side hooks for one project in what is currently an implicit workspace of multiple projects, or if perhaps this just is something you've fallen back to due to issues (e.g. running cargo commands from your project root).

Based on what you've shared I do think it would be a good idea for us to extend the supported configuration so that the hooks can specify their own target working directory.

However, more generally, I'm struggling to imagine real use cases where someone would go through quality-enforcement effort of having client side git hooks for one part of a project/workspace but not care about the other part of the project/workspace at all. The vast majority of the time a project that opts into having client side git hooks is going to have something on the server side as well to enforce those same types of checks (e.g. CI/PR checks, etc.) so I'm not entirely sure what the client side complexity really buys.

It would be nonsensical, in my opinion, to start a net-new project and only want to have partial quality checks. Even with large, existing projects where some new check is being slowly introduced (e.g. rustfmt, clippy, etc.) that's typically done by leveraging those tools respective configs, attributes, etc. to ignore parts of the projects while the team gradually apples the fixes to more and more of the project until the entire project passes those checks, and afterwards those checks get enforced consistently across the project.

So yes, we can certainly make it possible to control the directory your configured hook commands are run, but I'd be really curious to understand the thinking/motivation behind why you'd want that.

My initial intention had been to store a .rusty-hook.toml per-project and then use one of the tricks that other mono-repo projects use to only run the check for that Rust project if the commit consists of files changed from that project.

As I mentioned on the other thread, conditional behavior based on the contents of a diff in a pre-commit type scenario is the user's responsibility.

A core goal of rusty-hook is simplicity, so the entire setup and execution flow is primarily targeted to the user that just wants to plug their command into a toml file and never have to look at nor think about any of the underlying git mechanisms. Folks can always get more complex by writing their own scripts which rusty-hook will happily invoke, but we don't have any special case handling for any hooks. We could try to be a bit fancier with certain hooks down the road, but I think this is something bet left to users for the foreseeable future.

In that case, I'd need to to be able to do something like rusty-hook --config-file ${extracted_project_dir}/.rusty-hook.toml

Let me know if I've completely understood, but I'm just not following the thinking behind this proposal given the above context. If you want to have diff-based conditional hook behavior then you'll need to write your own script, track that script in version control, and configure the target hook in your rusty-hook.toml file to invoke your script:

e.g.

# rusty-hook.toml
pre-commit = "sh diff_based_pre-commit.sh"
# diff_based_pre_commit.sh

# inspect the diff
# if changes to proj10
# cargo fmt --manifest-path proj10/Cargo.toml ....
# cargo clippy ....

# if changes to other projects
# etc.

Given that you'd need such a script like that one, I don't understand why you'd want to add your own invocation of rusty-hook to execute those cargo commands for that project given that you already know everything you'd need to execute those commands. Having your diff_based_pre_commit.sh instead invoke rusty-hook would just be extra work and indirection to achieve the same result.

from rusty-hook.

calebcartwright avatar calebcartwright commented on June 2, 2024

cargo fmt --manifest-path chap10/Cargo.toml --all -- --check

And a random aside with my rustfmt-maintainer hat on 😄 , you probably won't actually ever need to use the --all flag. I'd like to rename it one day as it's a common source of confusion, but in general you should avoid it unless you know definitively you're in one of those extremely rare edge cases where's it's truly needed. It comes with some constant drawbacks like a requirement for network connectivity, a hit on the registry index, dependency graph resolution, etc. that 99% of all cases using --all could avoid since they don't really need it.

from rusty-hook.

nirvdrum avatar nirvdrum commented on June 2, 2024

I'm probably sharing things you already know, but worth noting that most of the time in these situations you'd want to setup a Cargo workspace, most likely as a virtual workspace in this particular scenario. While it's not strictly necessary it's an extremely common pattern that you can find in projects like the compiler itself, tokio, etc.

Thanks for the suggestion. I can look at it again, but for this particular use case I don't want to be sharing dependencies between each of the projects. I haven't tried a virtual workspace before, but my understanding is it still shares the Cargo.lock file. But, I'll give it a deeper look.

I think it'd be helpful if the pre-commit hook and its interaction with the rusty-hook executable honored the working directory, with the default being the repository's root
I haven't looked at the source, but rusty-hook appears to ignore the working directory

There's a small chance I am remembering this incorrectly, but I'm pretty sure git itself manages the working directory when invoking the hook scripts, and that it always changes the directory to the tree root/git_dir. Additionally, the set of params git provides to hooks naturally varies on the hook, but to my knowledge those params don't include the directory where the hook-triggering command was executed, nor am I aware of a means for hooks to figure that out themselves in a deterministic way.

I'm sorry if I wasn't clear on this. The issue isn't where the working directory starts when the hook is invoked. I've actually modified the generated pre-commit hook file to change into the project directory, but as far as I can tell rusty-hook ignores the current working directory:

#!/bin/sh
# rusty-hook
# version 0.11.2

hookName=$(basename "$0")
gitParams="$*"

# shellcheck source=src/hook_files/cli.sh
. "$(dirname "$0")"/cli.sh

if ! command -v rusty-hook >/dev/null 2>&1; then
  if [ -z "${RUSTY_HOOK_SKIP_AUTO_INSTALL}" ]; then
    installRustyHookCli
  else
    echo "rusty-hook is not installed, and auto install is disabled"
    echo "skipping ${hookName} hook"
    echo "You can reinstall it using 'cargo install rusty-hook' or delete this hook"
    exit 0
  fi
else
  ensureMinimumRustyHookCliVersion || true
fi

cd chap10
ls
cargo test

rusty-hook run --hook "${hookName}" "${gitParams}"
handleRustyHookCliResult $? "${hookName}"

In this situation, my explicit invocation of cargo test will work because the cd command changes the working directory. rusty-hook's invocation of cargo via its pre-commit command fails with:

Cargo.lock  Cargo.toml	clippy.toml  README.md	src  target   # <-- Output from the `ls` command I added in the pre-commit file
Found configured hook: pre-commit
Running command: cargo test
error: could not find `Cargo.toml` in `/home/nirvdrum/dev/workspaces/modern_compiler_implementation_in_ml-rust` or any parent directory
Configured hook command failed
pre-commit hook rejected
* rusty-hook expects the config file to exist in the project root (same level as the `.git` directory). We can make this more explicit in the docs, but the file has to be in a known place

I guess I'm advocating that this known place could just be within the current working directory. By default, that'll be the repository root. But, if the pre-commit script is modified to change the directory, then it'd just be picked up from that location or error out.

* rusty-hook currently executes the corresponding command(s) for the hook defined in the config file from the same project root directory

That last one is the source of your problem because you want to run cargo commands as part of your hook, but you haven't set up your project in the standard way to be able to run cargo commands from your project root.

I can just continue using a custom pre-commit hook then. I've adopted rusty-hook on a project with a more conventional layout and liked it, so I thought I'd use it here as well. I haven't run into many mono-repo projects that share dependencies across all components. Typically one of the use cases of splitting them apart into micro-services or whatever is so you can upgrade one independently of the others.

from rusty-hook.

nirvdrum avatar nirvdrum commented on June 2, 2024

In times like this I think it's important to try to separate the "what" and "why" from a specific "how". For example, it's unclear to me from the discussion above and on the other thread whether you're explicitly trying to only have client side hooks for one project in what is currently an implicit workspace of multiple projects, or if perhaps this just is something you've fallen back to due to issues (e.g. running cargo commands from your project root).

I hope I don't look like I'm trying to dodge questions. I am trying not to get drawn into the particulars of this one use case. This exact situation may not be all that common, but it's close enough of a multi-service mono-repo that I was hoping the simpler setup would be helpful. Each project can be "owned" by a different team and that team may have different requirements on what the checks for that project ought to be. So, it basically boils down to being able to run a configurable set of hooks for each project and then ideally, not running all the pre-commit checks in projects that didn't have any files changed, just to save on execution time.

Based on what you've shared I do think it would be a good idea for us to extend the supported configuration so that the hooks can specify their own target working directory.

However, more generally, I'm struggling to imagine real use cases where someone would go through quality-enforcement effort of having client side git hooks for one part of a project/workspace but not care about the other part of the project/workspace at all. The vast majority of the time a project that opts into having client side git hooks is going to have something on the server side as well to enforce those same types of checks (e.g. CI/PR checks, etc.) so I'm not entirely sure what the client side complexity really buys.

This is certainly doable from within GitHub Actions or most any other CI provider. But, that's just the typical trade-off between getting feedback before you commit or pushing a commit, waiting for CI, and now having to clean up history. And then the reality is you often get commits like "Fix CI" and an ensuing discussion about rebasing and merge strategies.

It would be nonsensical, in my opinion, to start a net-new project and only want to have partial quality checks. Even with large, existing projects where some new check is being slowly introduced (e.g. rustfmt, clippy, etc.) that's typically done by leveraging those tools respective configs, attributes, etc. to ignore parts of the projects while the team gradually apples the fixes to more and more of the project until the entire project passes those checks, and afterwards those checks get enforced consistently across the project.

I guess we can agree to disagree here. To the extent that different teams may be responsible for different parts of a product, having one team add a new check that now starts causing pre-commit failures for a team handling another project is a sure-fire way to start a spirited debate. Or maybe one project has a check that really only makes sense for that one project (e.g., validate generated GraphQL schema). At some level here, it just turns into a discussion about the relative merits of a mono-repo projects vs multiple repositories. The level of configuration I'm looking for is no different than what you'd have if you used separate repositories, which are typically not going to be the exact same set of checks for every project/repo in an organization. Ideally they would be, but the reality is they typically aren't.

So yes, we can certainly make it possible to control the directory your configured hook commands are run, but I'd be really curious to understand the thinking/motivation behind why you'd want that.

I hope I've addressed this by now, but if not, I'll give an example of a completely different mono-repo project I worked with at my last job. There it was a GraphQL-based single-page application with multiple services supporting the back-end. The reason we the company went with a mono-repo was so universal compatibility could be assured and deployments coordinated across services in a continuous deployment (CD) environment. The company had used separate repositories before and some service orchestration framework, but it was just simply too much overhead to manage and made CD way harder than with a mono-repo. And even then, if one part finished deploying before another, they wouldn't be agreeing on GraphQL schema or other APIs. I suppose Kubernetes could have fixed this, but for a whole host of reasons, that wasn't a realistic scenario.

The mono-repo was able to do complete integration tests across all components. And you couldn't run into a situation where the back-end updated the GraphQL schema and didn't update the front-end, because the front-end now wouldn't build. But, generally speaking, each project was pretty independent from the others. We tried not to let their dependency versions drift too far apart as a matter of maintenance, but if one project needed a new or updated dependency that would create issues for another project, that was a non-issue and thus didn't turn into a blocker.

As for lint checks and such, each project did indeed have its own configuration for that tool. But, that naturally meant you needed to change into the project directory before running the linter/formatter/etc. The only thing in the root of the repository was a Docker Compose configuration and the GitHub Actions workflows.

My initial intention had been to store a .rusty-hook.toml per-project and then use one of the tricks that other mono-repo projects use to only run the check for that Rust project if the commit consists of files changed from that project.

As I mentioned on the other thread, conditional behavior based on the contents of a diff in a pre-commit type scenario is the user's responsibility.

I don't mind it being my responsibility and I expect that it would be. rusty-hook just doesn't allow me to do it. Just to clarify, and setting aside the details of git tracking content rather than files, I'm not looking to change what runs based on what struct is being used or what have you. It's just checking the set of files being committed and changing the directory accordingly. I've done this without rusty-hook and am comfortable with that part of it.

A core goal of rusty-hook is simplicity, so the entire setup and execution flow is primarily targeted to the user that just wants to plug their command into a toml file and never have to look at nor think about any of the underlying git mechanisms. Folks can always get more complex by writing their own scripts which rusty-hook will happily invoke, but we don't have any special case handling for any hooks. We could try to be a bit fancier with certain hooks down the road, but I think this is something bet left to users for the foreseeable future.

If the mono-repo use case is too niche, I think that's fine. I'm not looking to strong-arm anyone into compromising their design goals. I did want, at the very least, to note usability issues I ran into. The setup documentation led me to believe the rusty-hook setup would be bound to the Rust project directory so I was surprised when I learned that it's actually bound to the git repo root.

Let me know if I've completely understood, but I'm just not following the thinking behind this proposal given the above context. If you want to have diff-based conditional hook behavior then you'll need to write your own script, track that script in version control, and configure the target hook in your rusty-hook.toml file to invoke your script:

e.g.

# rusty-hook.toml
pre-commit = "sh diff_based_pre-commit.sh"
# diff_based_pre_commit.sh

# inspect the diff
# if changes to proj10
# cargo fmt --manifest-path proj10/Cargo.toml ....
# cargo clippy ....

# if changes to other projects
# etc.

Given that you'd need such a script like that one, I don't understand why you'd want to add your own invocation of rusty-hook to execute those cargo commands for that project given that you already know everything you'd need to execute those commands. Having your diff_based_pre_commit.sh instead invoke rusty-hook would just be extra work and indirection to achieve the same result.

I can certainly do all this with custom scripts. For what it's worth, I didn't set out to find some corner case that breaks rusty-hook. I had successfully used rusty-hook on another project and found it it was a good way share pre-commit hooks in a repo (since git doesn't do that itself) and play nicely with Rust projects. I started trying to apply this to a mono-repo, thinking I'd have a similar experience. In particular, I thought rusty-hook init was aware of the directory it was being invoked from and would establish the configuration such that it could work with the sub-directory.

I wanted each project (really, just a sub-directory with its own Cargo.{toml,lock}) to have its own .rusty-hook.toml so the pre-commit checks are isolated between each project, version-controlled with each project, and configured consistently across each project. The conditional invocation is a nice-to-have, but not a requirement. If the generated pre-commit hook file just cd'd to each directory with a .rusty-hook.toml file and ran whatever hooks are configured within that file from that file's directory, that'd be fine as well. Most of the time it's going to run checks on files in projects that haven't changed, but I don't want that optimization to detract from the larger goal of per-project hooks/checks in a mono-repo.

And a random aside with my rustfmt-maintainer hat on smile , you probably won't actually ever need to use the --all flag. I'd like to rename it one day as it's a common source of confusion, but in general you should avoid it unless you know definitively you're in one of those extremely rare edge cases where's it's truly needed. It comes with some constant drawbacks like a requirement for network connectivity, a hit on the registry index, dependency graph resolution, etc. that 99% of all cases using --all could avoid since they don't really need it.

Thanks for the tip. I had pulled that check from the ripgrep repository. I figured the people over there knew Rust & Cargo a lot better than I do :-)

from rusty-hook.

calebcartwright avatar calebcartwright commented on June 2, 2024

I hope I don't look like I'm trying to dodge questions. I am trying not to get drawn into the particulars of this one use case.

To be honest, I feel like we're talking past each other in a manner that's not been terribly productive from my perspective.

When it comes to feature requests we really want to understand the what and why of the use case (and have structured the issue template accordingly) so that we have a better understanding of what someone is trying to achieve and why, whether that's an esoteric scenario or something potentially common, whether there's viable alternatives, and what a feasible solution would be if there's ultimately a need to enhance our product to support that use case. It's really difficult to have that type of dialog with discussions that start from the premise of a specific solution.

It feels like you have a very specific mental model of a git hook implementation for a certain workflow and that you feel like there's an issue with rusty-hook because it's not operating in that precise mental model. I concur that we will have to agree to disagree on this.

There are several ways of achieving a particular result with git hooks and if you prefer/are more comfortable with one that's perfectly alright but the fact that a different approach is taken with rusty-hook doesn't make it wrong; just different than what you may have assumed or be used to. rusty-hook can absolutely be used in multi-project/monorepos, and any characterizations/implications that it can't, is broken, or otherwise, are underinformed.

What I'd really like to do is wind the conversation back to the core picture so that we can have a more cohesive discussion as opposed to trying to debate your mental model/why rusty-hook operates the way it does because that doesn't seem to have gotten us anywhere.

I'm more than happy to have that type of discussion if you'd like. I'm also happy to continue to try to explain how/why rusty-hook works the way it does, especially since I get the impression there's fundamental parts of rusty-hook you still don't understand/assume to be different based on your experience with other git hook approaches.

However, if you'd prefer to stick with a particular approach and/or aren't interested in understanding an approach within the rusty-hook context then I do think it would be best to go ahead and close this issue.

from rusty-hook.

nirvdrum avatar nirvdrum commented on June 2, 2024

I'm sorry, something came up and I ended up not being able to address this at the time.

It feels like you have a very specific mental model of a git hook implementation for a certain workflow and that you feel like there's an issue with rusty-hook because it's not operating in that precise mental model. I concur that we will have to agree to disagree on this.

There are several ways of achieving a particular result with git hooks and if you prefer/are more comfortable with one that's perfectly alright but the fact that a different approach is taken with rusty-hook doesn't make it wrong; just different than what you may have assumed or be used to. rusty-hook can absolutely be used in multi-project/monorepos, and any characterizations/implications that it can't, is broken, or otherwise, are underinformed.

I'm afraid I've been unable to clearly articulate the issue. I'll take one more final attempt. One common layout for mono-repo projects is something like:

root/
  .github/
  cli/
  graphql-server/
  integration-tests/
  metrics/
  terraform/
  web-app/
  web-site/
  README.md

They are really separate projects living within the same repository. Each project has its own set of lint and code formatting rules. Not every project is using the same language and those that are may not be running with the same version of that language's runtime. They do not share dependencies. They do have some form of cross-project integration testing so to the extent that dependencies such as API-compatibility exist, a change in on project can't break another.

For the most part, you have different teams working on the different projects. There may be some form of company-wide standard on code formatting or linting, but some projects are older than others and they're progressively adopting. Maybe some lint rules make more sense in one problem domain than another. So, each rust project may have its own clippy.toml or may not have one at all. Since Clippy's version is tied to the Rust version via rustup and new rules are added or changed in every release, a team on an older version of Rust doesn't want all their builds to start failing because a separate team working on a separate sub-project wanted to update to the latest Rust version.

A non-Rust example of this is the Selenium project. There the sub-projects are largely split up by language and different groups of people work on each project. They all need to adhere to the same version of the WebDriver wire protocol. It's not an ideal example because in this organization, all of the C++ projects share a common Visual Studio solution. But, that solution file is different from the .NET solution file and each language may have its own rules for how things like JSON, XML, or YAML need to be formatted.

Given this typo of repository layout, how do the sub-projects using Rust add pre-commit hooks independent of other sub-projects? Reorganizing the repository is, unfortunately, a non-starter. As is restructuring each of the sub-projects to use a Cargo workspace. I was unable to find a way to do so with rusty-hook and so I opened the issue.

Running rusty-hook init from one of the sub-projects generated configuration that doesn't work because it's looking for Cargo.toml in the wrong location. I posit that it could work if the generated pre-commit hook would cd to the sub-project directory, thus finding the corresponding Cargo.toml (bullets #1 & #2 from the original issue) or if rusty-hook took a filename argument. That would result in the precommit file looking something like:

find . -name .rusty-hook.toml -print0 | xargs -0 rusty-hook run --hook "${hookName}" "${gitParams}"

Getting fancier, the generated precommit hook could use something like git diff --dirstat to only run rusty-hook on projects that had changed files.

I was more than happy to do that manually and I tried a few variations, but unfortunately when I tried that it didn't work because the working directory isn't used to locate the Cargo.toml file (bullet #3 from the original issue). That caught me off-guard. In most setups like this that I've seen, the invoked executable either takes a filename or directory as an argument and if one isn't provided, it looks for the resource relative to the current working directory since it makes scripting easier. I tried modifying the Bash script to cd, it didn't work, I was confused by that, dug into it, saw what was going on, and shared my findings. I spent a good deal of time trying to exhaust my options before opening the issue. I'm not trying to say rusty-hook is wrong or broken, but I did find having to specify the --manifest-path argument in the .rusty-hook.toml file instead to be less intuitive than changing the working directory and thought that might be helpful usability feedback.

In any event, in my initial issue I was suggesting that if the rusty-hook executable was a bit less clever and looked for Cargo.toml from the directory in which it was invoked (or took it as an argument), then rusty-hook would work better with mono-repo projects with the layout I've encountered. Likewise, I think it would still work out of the box for projects using a "one project per repo" model.

To be clear, I'm not really suggesting that rusty-hook do the recursive directory search thing I showed as a code example. It would certainly nifty if it did. But, I would find it immensely useful if rusty-hook was an executable that could be invoked within in a project directory and have all file locations resolve to that directory rather than the git repo root.

If this is already supported in some way, I'm sorry for having wasted your time. I've spent several hours on this and couldn't find a solution that didn't involve changing the way the rusty-hook executable operates. I'd be happy (if not ashamed) to discover I've been underinformed.

from rusty-hook.

Qix- avatar Qix- commented on June 2, 2024

Sorry to gravedig, this was the first search result both on Github and Google.

If anyone is searching here for a solution to the root Cargo.toml being in another subdirectory and don't want to read the conversaton above. the solution is to run ln -s some/dir/.rusty-hook.toml .rusty-hook.toml from the root of your repository, where some/dir is the root of your Rust project. Do not use absolute paths; this should Just Work for any git clone + cargo build/test/... anyone working on the repository executes.

from rusty-hook.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.