> The primary reason commit-msg hook is there is *not* because we need > a way to tweak the log message. As you said, prepare-commit-msg and > templates are much better way to give some sort of default. Yes, I completely agree. I meant my original message to refer only to the case of an empty commit message. > The purpose of the hook is to serve as the gatekeeper to cause an > attempt with a bad commit message to fail. And a properly written > commit-msg hook would be rejecting an empty message, instead of > inserting cruft into an empty message file. It doesn't conceptually make sense for Git to be calling the commit-msg hook in this case to begin with though. The developer has already expressed an intent to abort the commit, and that intuitively feels like it should be the end of the process. When someone sits down to write a commit-msg hook, it's very natural to think: "how should I handle this commit message?" It's less natural to think: "How should I handle the case where the developer aborted the commit?" This after all has nothing to do with the intention of the commit-msg hook being written, so it will tend not to be on peoples' minds. If they do think about it without investigating thoroughly, they are liable to assume that Git wouldn't call their hook in this case since the commit has no message and is being aborted. Worse still, even if they do recognize that their hook will be called, they are most likely to assume that they should reject an empty commit message by exiting with an error code in the typical guard-against-bad-input sense, but this is also wrong. The correct action is in a sense doubly inverted: you want to explicitly always allow an empty commit message by exiting successfully without output. In essence, your script must pretend that nothing happened, because it shouldn't have, which is rarely the correct action to take as a programmer. Every single commit-msg hook author is presently being relied on to get this right, which automatically means more mistakes will be made. I would be curious if anyone is aware of any hook using the pre-commit framework that implements this correctly by default (without relying on the end developer to pass additional arguments). My experience using the pre-commit framework has been a spew of erroneous errors from commit-msg hooks in the aborted commit case, because none of them made it even to the first step of considering the possibility that the commit may have already been aborted. > So, from that point of view, if we were to change anything, a more > useful thing to do might be to forbid commit-msg hook from modifying > the file and make sure it would only verify but not modify, I > suspect. Doing so would have a side effect of making sure that no > commit-msg hook will turn an empty message file into a non-empty > message file ;-). I would not recommend this, no. What other hook would be better suited to programmatically modify a non-empty commit message? Such a restriction would break, for instance, the witty sign-commit pre-commit hook: https://github.com/mattlqx/pre-commit-sign. Implementing sign-commit without the power to change the commit message from the commit-msg hook boxes you into using a prepare-commit-msg hook, which is a worse user experience. It puts the signature in a human-editable field where they can accidentally mangle or delete the signature, put their message beneath it instead of above it, delimit it from their commit message inconsistently, etc. Also, this would not solve the problem of preventing the commit-msg hook from being run in the first place when the commit message is empty. > I'd think we'd want to call it on an empty message, e.g. maybe someone > depends on that with empty message = auto-generate a message for me. This is the backwards compatibility point I was referring to. Definitely understand if this is a show stopper since I consider this more of a quirk than a serious issue. I would think some amount of warning would be in order if the behavior of commit-msg hooks were to change to avoid unnecessary chaos, and I would be curious if anyone is specifically aware of any commit-msg hooks that intend to consume empty commit message. The better way to implement the auto-generated message in my opinion would be with a commit message template or a prepare-commit-msg hook. In theory, one could potentially also offer a new hook that is similar to a commit-msg hook but that didn't get called if a commit was aborted, but that feels like overkill. > But for those that don't, doesn't the default behavior of "git commit" > catch this in either case, i.e. it wouldn't let it pass without > --allow-empty-message. I agree with your intuition in the sense that that would be preferable behavior, but no, this is not how Git behaves presently. The commit-msg hook still gets called, and a maintainer of the pre-commit framework has assured me that they follow Git's behavior to the tee in this regard. > I understood this report as the hook taking the empty message (e.g. the > user using it as a shorthand to abort), and their hook "helpfully" > inserting some "default" message or template. I believe that sign-commit does exactly what you suggest, yes. The cases I personally have encountered were simply misleading validation errors (e.g., the commit message doesn't follow conventional commit or isn't long enough). > I tend to agree with you that, especially if we are to keep the > "commit-msg hook is allowed to change the message, even though it is > a job for other hooks", we should invoke it even on an empty file. Do you have a use case in mind for a commit-msg hook that runs on an empty commit message that wouldn't be better implemented some other way? More to the point, why continue with the commit process at all at this point? It's vaguely analogous to a program allowing third-party extensions to automatically relaunch a program after the user closed it. Presently commit hooks are given the power to overrule what is most often a developer's intention to abort a commit, and the most common case in practice that this behavior manifests is in incorrectly implemented commit-msg hooks that either spew misleading error messages or abort the abort process unintentionally. > They do not fall into either of the two camps. Their hook does futz > with the message indiscriminately and adds some boilerplate material > blindly, even to an empty file. > The complaint is "the message is otherwise without any substance, > but because the hook blindly added some boilerplate material into > the empty original, it appears to be non-empty and fails to trigger > the --no-allow-empty-message machinery." None of the hooks I have personally used do this, but such hooks exist and are problematic. Be well, Kurt El vie, 17 sept 2021 a las 12:44, Junio C Hamano (<gitster@xxxxxxxxx>) escribió: > > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > > > I'd think we'd want to call it on an empty message, e.g. maybe someone > > depends on that with empty message = auto-generate a message for me. > > I tend to agree with you that, especially if we are to keep the > "commit-msg hook is allowed to change the message, even though it is > a job for other hooks", we should invoke it even on an empty file. > > > But for those that don't, doesn't the default behavior of "git commit" > > catch this in either case, i.e. it wouldn't let it pass without > > --allow-empty-message. > > > > I understood this report as the hook taking the empty message (e.g. the > > user using it as a shorthand to abort), and their hook "helpfully" > > inserting some "default" message or template. > > My understanding largely overlaps with yours but with some > differences. > > They do not fall into either of the two camps. Their hook does futz > with the message indiscriminately and adds some boilerplate material > blindly, even to an empty file. > > The complaint is "the message is otherwise without any substance, > but because the hook blindly added some boilerplate material into > the empty original, it appears to be non-empty and fails to trigger > the --no-allow-empty-message machinery." > > >
