Skip to content
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

Issue with custom pre-push hook with git lfs install #2865

Closed
abhishekph opened this issue Feb 14, 2018 · 13 comments
Closed

Issue with custom pre-push hook with git lfs install #2865

abhishekph opened this issue Feb 14, 2018 · 13 comments

Comments

@abhishekph
Copy link

Currently, we are using .git/hooks/pre-push hook for verification of commit before the final push.

We have some users who work with git-lfs and some users who yet not using git-lfs.

Whenever we initialize our git repository we copy .git/hooks/pre-push from our hooks directory. This happens for all users. But whenever the user wants to use git-lfs, they have to run the script which does following:
$ rm -rf .git/hooks/pre-push
$ git lfs install --force

If there is alternative to this situation? Is there a way to initialize git-lfs even when there is a "custom" pre-push hook already exists? Can we modify pre-push hook installed by git-lfs to call some alternative script e.g. custom-pre-push?

@larsxschneider
Copy link
Member

Great question @abhishekph ! The core problem right now is that Git only supports one script per hook. I am already thinking about possible ways to improve Git in that regard.

Here is what you can do today:

A Git LFS pre-push hook looks like this:

#!/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 "$@"

You can take this snippet and combine it with you custom pre-push hook. Afterwards Git LFS will be initialized whenever your users setup your custom verification hook. E.g.:

#!/bin/sh

# Custom verification
check-custom-verification

# 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 "$@"

Would that work for you?

@abhishekph
Copy link
Author

Thank you for the workaround. I will test adjusting our custom pre-push hook. The only issue see is if someone tries to do "git lfs install" it would still fail. So I will have to test if custom pre-push will still be working as expected without successful "git lfs install".

@abhishekph
Copy link
Author

Update. Workaround worked as I was able to push tracked objects. Please resolve this issue. Thank you again for the solution.

@larsxschneider
Copy link
Member

Great to hear! Your welcome 😊 !

@jokram
Copy link

jokram commented Mar 2, 2018

We recently have had the same issue. And I see here a danger that you could loose data.
I've made some tests using git-lfs/2.3.4, git version 2.15.0.windows.1 and here is my conclusion:

  • Git tries to install the necessary local Git LFS hooks on every git command if the repository contains tracked Git LFS files (e.g. git status).
  • If a local pre-push hook already exists then this hook will not be overwritten and a git push will NOT upload the changed LFS tracked files!
  • This might lead to the error "Object does not exist on the server" for other users when cloning / fetching.
  • In such a situation the user does not get any information that Git LFS objects are not uploaded properly.
  • Even more problematic: The user might delete such a branch not knowing that these LFS objects don't exist on the server.
  • And the git garbage collection will delete the local LFS objects which then cannot be reverted anymore.
  • Fixing is only possible if the LFS objects still exist locally with git lfs push --all origin <branch>

In a big company environment it's hard to ensure that all local pre-push hooks contain the LFS pre-push part. Is there any solution planned for this?

@jokram
Copy link

jokram commented Mar 2, 2018

And then a user could call git push --no-verify => bypass pre-push hook.
(I mean that's nearly criminal, but it would lead to inconsistent data on the server.)

@ttaylorr
Copy link
Contributor

ttaylorr commented Mar 3, 2018

Hi @jokram, I think that you make an excellent point. Currently, Git LFS will warn the user when it is unable to install a pre-push hook in the case that one already exists. Unfortunately, I'm not sure that the Git LFS client alone is able to do any better than that -- besides the initial warning, without a pre-push hook, Git LFS won't have any triggers from Git in order to determine that the pre-push hook isn't installed, and thus the problems that you describe above are possible.

Do you have any ideas on how we might indicate this more clearly to a user?

@jokram
Copy link

jokram commented Mar 5, 2018

Hi @ttaylorr, as workaround we are thinking of solving this problem via a server side pre-push hook which shall block pushes containing binary files which should have been tracked with LFS (because they are mentioned in the .gitattributes). But our first tests show that for big push operations this takes too long (more than 5 seconds). And in the case of GitHub a pre-receive hook will be aborted in such situations and the corresponding push would be blocked even if the push was OK.
And this solution depends on the Git server, so it's not a generic solution for Git LFS (if we would get it running).

To solve this in the context of Git LFS I think we would need an extension on Git side. E.g. to have multiple pre-push hooks, so that it can handle a chain of pre-push hooks. By this the custom hook could live in parallel to the Git LFS pre-push hook. This would still not solve git push --no-verify, but from my point of view this would be acceptable.

Or we would "overwrite" the normal Git push command by wrapping it with Git LFS specifics. So when a user calls git push it would call the git-push from Git LFS which itself calls the core git-push. (I'm not sure if this is possible and it's also not so nice, because it changes standard behavior).

@larsxschneider
Copy link
Member

larsxschneider commented Mar 5, 2018 via email

@erikng
Copy link

erikng commented Mar 21, 2019

Has there been any progress on multiple hooks with git/git-lfs?

@bk2204
Copy link
Member

bk2204 commented Mar 21, 2019

Git LFS will honor custom hooks and not overwrite them if you have them defined (unless you run git lfs update), so it's possible to use custom hooks that support Git LFS if they're available.

However, multiple hooks are a Git feature, and it's one that's best discussed upstream. There's been discussion in the past on the list, but nobody has implemented it upstream.

@artsmvch
Copy link

artsmvch commented Oct 2, 2023

@bk2204 as I understand, I can overwrite the git lfs pre-push hook with my own and it won't break anything?

Looking at the git lfs pre-push hook:
what exactly does the following command git lfs pre-push "$@"?

@sadamczyk
Copy link

I wanted to add my own custom pre-push hook as a git-template to all my projects, but ran into this issue with git-lfs putting it's own hook in some of the repos.
My solution to this problem is to add the git-lfs hook to all my projects and just check if the repo actually uses git-lfs by checking the .gitattributes. Credit to this SO answer for that: https://stackoverflow.com/a/76197696

#!/bin/sh

# Custom verification
check-custom-verification

# Only run the git-lfs pre-push when git-lfs is actually used in this repo
if git grep -q filter=lfs -- .gitattributes '**/.gitattributes' # https://stackoverflow.com/a/76197696
then
    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.\n"; exit 2; }
    git lfs pre-push "$@"
fi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants