Page MenuHomePhabricator

Git commit hooks may mutate commits in "arc land", discarding "Differential Revision: ..."
Open, WishlistPublic

Description

See PHI1723. A user installed a Git commit-msg (probably?) or pre-commit hook (maybe?) which mutated the commit generated by arc land, discarding "Differential Revision: ...".

We can git commit --no-verify instead to skip hooks, but there are some legitimate reasons to have hooks. Possible ways forward:

  • Do nothing, users are doing this on purpose to some degree and just going along with it is the best match for user intent.
  • Just --no-verify and skip hooks.
  • --no-verify first, grab the hash, amend -C HEAD, check if the hash changed, warn the user if it did.
  • If pre-commit can not mutate the commit: commit, grab the message, compare the message, warn if it changed (and/or is missing "Differential Revision: ...").

Event Timeline

epriestley triaged this task as Wishlist priority.May 6 2020, 6:00 PM
epriestley created this task.

If pre-commit can not mutate the commit

It seems difficult for pre-commit to mutate the commit. For example, using this hook:

git commit --amend --allow-empty --no-verify -m QUACK

...produces this error:

$ git commit --allow-empty -m 'NO QUACKS ALLOWED'
[master 7b454a1] QUACK
 Date: Fri May 8 13:47:19 2020 -0700
fatal: cannot lock ref 'HEAD': is at 7b454a12a2567bf4a66c8cdd13224310b3b83419 but expected 378d91e344736a7c5c7babef8cb6caf8bdf28b98

Later, pre-push can do whatever it wants (the command above works fine in pre-push), but it only applies locally: git still pushes refs to the remote by commit hash, as far as I can tell. So pre-push can make your local working copy look nothing like the remote, but doesn't seem to be able to make a different set of changes get pushed to the remote.

Finally, post-commit can do whatever it wants and definitely breaks things and it is not disabled by --no-verify! As far as I can tell, nothing short of moving the hook aside can disable post-commit.

So this whole thing is sort of a fool's errand and a determined user can shoot themselves in the foot with post-commit more or less no matter how hard we try to prevent it, with the caveat that they have to be aiming for their foot hard enough to include a mechanism that prevents the hook from re-entering itself over and over again.

Notably, this seems to be not-exactly-true, although it obviously means "git ignores the exit code of the hook":

$ git help githooks

...

   post-commit
       ...

       This hook ... cannot affect the outcome of git commit.