-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Git LFS and husky prepush hook? #108
Comments
Hi @orlin, Good question. Would it be possible to move the logic out of the {
"scripts": {
"prepush": "node ./scripts/git-lfs-exists.js"
}
} |
@typicode I'm having the same issue. Willing to try it out but I'm not sure how to convert the git-lfs code to JS |
@typicode No. package.json
I suppose LFS requires parameters: https://git-scm.com/book/gr/v2/Customizing-Git-Git-Hooks
|
What would probably solve the issue: ability to add hooks code instead of replacing it.
|
+1 |
1 similar comment
+1 |
i have the same problem and to solve it i migrate from husky to git-hooks. both library is pretty similiar but git-lfs hooks work pretty well with git-hooks :) |
Try to add $GIT_PARAMS into your prepush script |
+1 |
Any real solutions to this outside of manually editing the git hook...? |
@GregTheGreek I switched to |
Amazing thanks!
…On Mon, Sep 24, 2018, 09:04 Filip Kinský ***@***.***> wrote:
@GregTheGreek <https://github.com/GregTheGreek> I switched to
git-hooks-plus because of this problem and it works just well.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#108 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AQJSTSPHU2nr_dScxn-eRNRenP2XL2E1ks5ueNhkgaJpZM4MW_4A>
.
|
Have the same issue, i'd rather not switch again to a new tool. |
I've managed to get this working with the latest version of husky (1.1.3): package.json "husky": {
"hooks": {
"pre-push": "hooks/pre-push-lfs $HUSKY_GIT_PARAMS <<< $HUSKY_GIT_STDIN"
}
} The trick was figuring out how to set STDIN for the command, using The script hooks/pre-push-lfs #!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting .git/hooks/pre-push.\n"; exit 2; }
git lfs pre-push "$@" Be sure to make it executable: chmod +x hooks/pre-push-lfs This got pre-push working for me. I think the other hooks probably need the same setup too (though haven't tested this yet): package.json "husky": {
"hooks": {
"post-checkout": "hooks/post-checkout-lfs $HUSKY_GIT_PARAMS <<< $HUSKY_GIT_STDIN",
"post-commit": "hooks/post-commit-lfs $HUSKY_GIT_PARAMS <<< $HUSKY_GIT_STDIN",
"post-merge": "hooks/post-merge-lfs $HUSKY_GIT_PARAMS <<< $HUSKY_GIT_STDIN",
"pre-push": "hooks/pre-push-lfs $HUSKY_GIT_PARAMS <<< $HUSKY_GIT_STDIN"
}
} Hope that helps. |
This is not really a solution for me? This is just hacking in and renaming shipped hooks and this way the default shipped LFS hooks will probably die over time. The change needs to happen in Husky i think to append the husky hook implementation with the already existing one (or atleast give it as an option). Ofcourse this will make husky not maintaining the LFS hook but in my case i also do not want it. |
@jmchambers thank you for the trick. It worked well for me on arch linux, but a coworker using Ubuntu gets the following error:
replacing bin/sh with bin/bash or bin/dash in the script didn't help. |
After reading a bit about here-strings, I modified the hook calls as follow
|
Sorry for the delayed reply @Fandekasp, glad you got it sorted. I'm on mac, I guess here-strings are a platform-dependent thing. I prefer your syntax. @pimjansen, I think the philosophy behind Husky is that all hooks should be outside the |
Just realized that somehow the |
To summarize suggestions by @jmchambers and @Fandekasp, the following works for me and keeps the hooks under version control as desired:
Yes, this does mean that if those hooks for LFS ever change that your hook files would need to be updated to stay in sync, but if you look at those hooks files they are pretty inert - they just call the git lfs commands that must possess the actual business logic for the hooks. So I would estimate that is a minor issue - and would be easily solved after a future breaking change by repeating the here-entailed process of saving the hooks from |
@mattrabe thanks for summary. However there is an issue with that solution. After clone
And husky won't overwrite that hooks on |
@mbelsky I believe your statement to be correct regarding files in .git/hooks - however, if I am reading correctly it does not affect my summary above, because the files in my summary are in |
@mattrabe I've created a simple repo with husky and lfs. To reproduce the issue that I described, please do following steps:
Expected result:On Why?This happens because after cloning |
@mbelsky I think if we just add a step 3.5 in there where we manually go to the |
@aviaryan absolutely right. However this step break lfs support for a repo. So in addition it requires 4.5 step where a user manually merge lfs hooks in husky hooks. |
@mbelsky Sorry but I don't see how we need that step. If we go by @mattrabe's post, LFS hooks should be already placed by husky (at PS - It could be that I don't get how |
@aviaryan oh I've missed that we placed lfs hooks under |
Just tried @mattrabe 's suggestion but I always get push failed: > running pre-push hook: echo $HUSKY_GIT_STDIN | hooks/pre-push-lfs $HUSKY_GIT_PARAMS
This should be run through Git's pre-push hook. Run `git lfs update` to install it.
pre-push hook failed (add --no-verify to bypass) I'm running yorkie (fork of husky with no issue queue of its own) rather than husky because Vue CLI 3 but I'm led to believe that they're similar enough. Any pointers? |
@danbohea Did you run |
@mattrabe No I didn't, I thought that this would attempt to write to the .git/hooks/pre-push file that yorkie has already written to. Shall I run it anyway? |
@mattrabe I ran it: $ git lfs update
Hook already exists: pre-push
#!/bin/sh
#yorkie 2.0.0
command_exists () {
command -v "$1" >/dev/null 2>&1
}
has_hook_script () {
[ -f package.json ] && cat package.json | grep -q "\"$1\"[[:space:]]*:"
}
# OS X and Linux only
load_nvm () {
# If nvm is not loaded, load it
command_exists nvm || {
export NVM_DIR="$1"
[ -s "$1/nvm.sh" ] && . "$1/nvm.sh"
}
}
# OS X and Linux only
run_nvm () {
# If nvm has been loaded correctly, use project .nvmrc
command_exists nvm && [ -f .nvmrc ] && nvm use
}
cd "."
# Check if pre-push is defined, skip if not
has_hook_script pre-push || exit 0
# Add common path where Node can be found
# Brew standard installation path /usr/local/bin
# Node standard installation path /usr/local
export PATH="$PATH:/usr/local/bin:/usr/local"
# Try to load nvm using path of standard installation
load_nvm /home/dan/.nvm
run_nvm
# Export Git hook params
export GIT_PARAMS="$*"
# Run hook
node "./node_modules/yorkie/src/runner.js" pre-push || {
echo
echo "pre-push hook failed (add --no-verify to by
To resolve this, either:
1: run `git lfs update --manual` for instructions on how to merge hooks.
2: run `git lfs update --force` to overwrite your hook. |
@danbohea Hm, I wish I could help out - I have not experienced that before. |
Hey @danbohea |
Thanks @mbelsky I now have something working with husky (eventually I need to get this playing nice with yorkie). Your guide really helped me understand what each tool is doing and when, especially when installing/initialising. |
Here are commands that work on Windows (in Git Bash), as well as *nix: "husky": {
"hooks": {
"pre-commit": "lint-staged",
"post-checkout": "cross-env-shell echo $HUSKY_GIT_STDIN | cross-env-shell sh lfs-hooks/post-checkout $HUSKY_GIT_PARAMS",
"post-commit": "cross-env-shell echo $HUSKY_GIT_STDIN | cross-env-shell sh lfs-hooks/post-commit $HUSKY_GIT_PARAMS",
"post-merge": "cross-env-shell echo $HUSKY_GIT_STDIN | cross-env-shell sh lfs-hooks/post-merge $HUSKY_GIT_PARAMS",
"pre-push": "cross-env-shell echo $HUSKY_GIT_STDIN | cross-env-shell sh lfs-hooks/pre-push $HUSKY_GIT_PARAMS"
}
}, The
|
Ignore above. Command chaining simply does the job. No need for anything fancier.
Might not look pretty but for multiple commands but those can either be wrapped in npm script command or run as bash script that exits with non 0 for errors. |
Reading through this issue it seems like there are a lot of potential solutions, but tbh. I'd like feedback from the lib author as to what is a canonical solution to this problem from husky's perspective. The original question in the top comment was:
@typicode, when you have time, it'd be nice to have your feedback on what you'd prefer as the solution. Ideally we'd then also add a test case to ensure that the canonical solution won't break. Until then I don't feel safe implementing this in a project. |
Not sure how to test it, but I think this code might work. I've opened a pr. Can someone take a look: #729? Update: I have tested it on one of my repositories and it worked |
Closing as it should be easier now to use Git LFS with husky's new approach. |
Git Large File Storage[0] is used for versioning large files. We believe it's necessary for storing the Structured Interventions pdf in our repo. It's tricky to set up in conjunction with our Husky linting git hook, so I've followed the instructions on a [GitHub issue](typicode/husky#108 (comment)) [0]: https://git-lfs.github.com/
Git Large File Storage[0] is used for versioning large files. We believe it's necessary for storing the Structured Interventions pdf in our repo. It's tricky to set up in conjunction with our Husky linting git hook, so I've followed the instructions on a [GitHub issue](typicode/husky#108 (comment)) [0]: https://git-lfs.github.com/
Git Large File Storage[0] is used for versioning large files. We believe it's necessary for storing the Structured Interventions pdf in our repo. It's tricky to set up in conjunction with our Husky linting git hook, so I've followed the instructions on a [GitHub issue](typicode/husky#108 (comment)) [0]: https://git-lfs.github.com/
Git Large File Storage[0] is used for versioning large files. We believe it's necessary for storing the Structured Interventions pdf in our repo. It's tricky to set up in conjunction with our Husky linting git hook, so I've followed the instructions on a [GitHub issue](typicode/husky#108 (comment)) [0]: https://git-lfs.github.com/
Git Large File Storage[0] is used for versioning large files. We believe it's necessary for storing the Structured Interventions pdf in our repo. It's tricky to set up in conjunction with our Husky linting git hook, so I've followed the instructions on a [GitHub issue](typicode/husky#108 (comment)) [0]: https://git-lfs.github.com/
@typicode Can you share a link to a resource (or explain here) that explains why or how husky@V5 solves this issue? |
husky v4 installs husky on default git hooks folder As we can see in the below example, after migrating to
My migration from v4 to v8 (adding git-lfs too)IT IS JUST AN EXAMPLE FOR v4package.json: "husky": {
"hooks": {
"pre-push": "yarn lint"
}
} v8 (with git-lfs):package.json: "scripts": {
"prepare": "husky install"
}, .husky/pre-push: #!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"
# my custom hook
yarn lint
# git-lfs hook
command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting '.git/hooks/pre-push'.\n"; exit 2; }
git lfs pre-push "$@" As we can see, .husky/pre-push is more flexible (shell script), is under git version control and uses the original I tried to be short, but it's not easy to explain with few words. 😅 |
I just tried to
husky
aprepush
script and nothing happened. Looking at.git/hooks/pre-push
I see that I already have a hook there:Running
node node_modules/husky/bin/install.js
doesn't help:I guess that's good, as I don't want Git LFS to break and get some large files in the repo without noticing...
Is there a husky solution for having a prepush while also using git-lfs?
The text was updated successfully, but these errors were encountered: