{"thread":{"id":"39451","subject":"pre-commit hook not run on conflict resolution during rebase","startedAt":"2015-05-28T19:38:02Z","lastAt":"2015-05-29T07:25:28Z","messageCount":3,"participants":["Stefan Haller","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"262367","messageId":"1m55i3m.1fum4zo1fpnhncM%lists@haller-berlin.de","threadId":"39451","inReplyTo":null,"subject":"pre-commit hook not run on conflict resolution during rebase","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2015-05-28T19:38:02Z","receivedAt":"2015-05-28T19:38:02Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"When a rebase stops because of a conflict, and I edit the file to\nresolve the conflict and say \"git rebase --continue\", then the\npre-commit hook doesn't run at that point, which means that I can commit\nbad stuff which the pre-commit hook would normally not allow in. We were\nbitten by this a few times already. (In our case, the hook rejects code\nthat hasn't been run through clang-format. That's easy to forget when\nresolving conflicts during a rebase.)\n\n>From glancing through the githooks manpage, I couldn't see any other\nhook that would help in this situation. Am I missing something?\n\nI guess the next best solution would be to also have a pre-push hook\nthat performs the same checks again, just in case the bad code managed\nto get past the pre-commit hook for some reason or other. This feels\nvery redundant, but I guess it would work well.\n\nAny other suggestions?\n\n\n-- \nStefan Haller\nBerlin, Germany\nhttp://www.haller-berlin.de/\n \n"},{"id":"262369","messageId":"xmqqy4k8fp1g.fsf@gitster.dls.corp.google.com","threadId":"39451","inReplyTo":"1m55i3m.1fum4zo1fpnhncM%lists@haller-berlin.de","subject":"Re: pre-commit hook not run on conflict resolution during rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-28T20:03:07Z","receivedAt":"2015-05-28T20:03:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lists@haller-berlin.de (Stefan Haller) writes:\n\n> When a rebase stops because of a conflict, and I edit the file to\n> resolve the conflict and say \"git rebase --continue\", then the\n> pre-commit hook doesn't run at that point,...\n> From glancing through the githooks manpage, I couldn't see any other\n> hook that would help in this situation. Am I missing something?\n\nI do not think so.  There may be some fallouts, like negatively\naffecting folks who have been relying on the current behaviour, but\nI do not see a fundamental reason why that hook should not trigger\nthere (it may not trigger in the current implementation, but I view\nit as lack of need so far, not a deliberate omission).\n\n> I guess the next best solution would be to also have a pre-push hook\n> that performs the same checks again, just in case the bad code managed\n> to get past the pre-commit hook for some reason or other. This feels\n> very redundant, but I guess it would work well.\n\nI'd say pre-receive would be the most sensible place to check things\nlike this.  Some of your developers may not have pre-commit hook or\neven run \"commit --no-verify\" to bypass the local check, and if you\nhave a central meeting place for the efforts by all your folks, that\nis where you want to enforce the policy.\n\nChecks done anywhere else are what are redundant, including the\npre-commit hook in individual developers.  You can view them as a\nway for individual developers to save their time---by choosing to\ncheck locally, they make sure their commits do not trigger the\npre-receive hook at the central place.\n"},{"id":"262391","messageId":"1m56dnm.1tg204rp3q4paM%lists@haller-berlin.de","threadId":"39451","inReplyTo":"xmqqy4k8fp1g.fsf@gitster.dls.corp.google.com","subject":"Re: pre-commit hook not run on conflict resolution during rebase","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2015-05-29T07:25:28Z","receivedAt":"2015-05-29T07:25:28Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n\n> lists@haller-berlin.de (Stefan Haller) writes:\n> \n> > I guess the next best solution would be to also have a pre-push hook\n> > that performs the same checks again, just in case the bad code managed\n> > to get past the pre-commit hook for some reason or other. This feels\n> > very redundant, but I guess it would work well.\n> \n> I'd say pre-receive would be the most sensible place to check things\n> like this.\n\nYes, I totally agree, and we used to have this setup when we were still\nhosting our code in-house; with pre-receive doing the authorative\nchecks, and pre-commit being optional as a convenience for developers,\nas you say.\n\nNow we have moved most of our code to github, and you can't have\npre-receive hooks there, as far as I can tell. (I should have mentioned\nthat, sorry.)\n\nTo make up for that, we have put considerable effort into ensuring that\neveryone on the team has up-to-date hooks locally, by installing them\nautomatically as part of our build system infrastructure.\n\nIn that light, do you agree that a pre-push hook is the best we can do\nnow to plug this hole?\n\n\n-- \nStefan Haller\nBerlin, Germany\nhttp://www.haller-berlin.de/\n"}]}