Skip to content

locking: add pre-commit hook to detect inappropriately modified files#1816

Closed
ttaylorr wants to merge 8 commits intolocking-pre-pushfrom
locking-pre-commit
Closed

locking: add pre-commit hook to detect inappropriately modified files#1816
ttaylorr wants to merge 8 commits intolocking-pre-pushfrom
locking-pre-commit

Conversation

@ttaylorr
Copy link
Contributor

This pull-request adds a pre-commit hook that detects and prevents commits changing locked files owned by someone other than the current committer.

This hook looks at the currently staged files when it is called (before the user is told to edit the .git/COMMIT_EDITMSG) and inspects each one of them to see if they are locked by someone other than the current committer. The current committer is used since it is the latest and most accurate information determinable about who will commit the file.

Here's a breakdown of the changes:

  1. 4f40215...19eaad8: implement the pre-commit hook
  2. c0843c7: install the pre-commit hook
  3. 479f031...bf24886: add integration tests
  4. 2958106: add a man-page entry for the pre-commit command

Some things that we should investigate before merging:

  • How does this hook behave when changing history, i.e., with filter-branch, rebase or commit --amend?
  • Should we stream the results of git.StagedFiles(), to better handle a case where there are many staged files?

/cc @git-lfs/core

Copy link
Contributor

@sinbad sinbad left a comment

Choose a reason for hiding this comment

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

I could make some other comments here, such as it not supporting cases where you don't commit from staging, but the major, I think insurmountable, problem with this approach is that it makes committing require remote access. This would break a fundamental principle of git that you can work offline if you need to, until push/pull.

I have work ongoing (backported from my earlier PR) for making lockable files read-only in post-checkout which will hopefully discourage people from editing files that should be locked without locking them first. Yes, locking itself requires remote access but the idea is that you do that first before going offline and can then commit as much as you want offline. If you forget to lock beforehand you can always take a risk by making the file read/write locally while offline and hope that no-one else does (will need resolving at push time; this will be more than just checking current locks)

@technoweenie
Copy link
Contributor

I think that's a great point. Let's hold off on this and see if it's actually needed in real use.

@ttaylorr
Copy link
Contributor Author

ttaylorr commented Jan 6, 2017

I think that's a great point. Let's hold off on this and see if it's actually needed in real use.

I agree, let's close for now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants