Skip to content

Conversation

@bk2204
Copy link
Member

@bk2204 bk2204 commented Apr 11, 2022

Some people store their hooks inside their repository. This isn't a secure approach, but in some cases it can be acceptable if all users are trusted.

In such a case, if Git LFS always rewrites a hook even if it hasn't changed, it can result in the file being marked as modified by git status, since git status can invoke Git LFS, which in turn invokes Git LFS, which updates the hooks. Thus, the hooks will always appear modified.

Let's solve this by not writing the hooks if they haven't changed. We don't include tests here because the only way to determine if changes have occurred is the mtime, and that's difficult to effectively test in the testsuite in a portable way without introducing sleeps, which would make our tests slow.

Fixes #4865

Some people store their hooks inside their repository.  This isn't a
secure approach, but in some cases it can be acceptable if all users are
trusted.

In such a case, if Git LFS always rewrites a hook even if it hasn't
changed, it can result in the file being marked as modified by `git
status`, since `git status` can invoke Git LFS, which in turn invokes
Git LFS, which updates the hooks.  Thus, the hooks will always appear
modified.

Let's solve this by not writing the hooks if they haven't changed.  We
don't include tests here because the only way to determine if changes
have occurred is the mtime, and that's difficult to effectively test in
the testsuite in a portable way without introducing sleeps, which would
make our tests slow.
@bk2204 bk2204 marked this pull request as ready for review April 11, 2022 18:05
@bk2204 bk2204 requested a review from a team as a code owner April 11, 2022 18:05
Copy link
Member

@chrisd8088 chrisd8088 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thanks!

@bk2204 bk2204 merged commit b698a64 into git-lfs:main Apr 11, 2022
@bk2204 bk2204 deleted the avoid-hook-rewrite branch April 11, 2022 20:46
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

Successfully merging this pull request may close these issues.

Avoid overwriting unchanged hooks

2 participants