{"thread":{"id":"66759","subject":"Interaction between pre-commit hook and git commit <pathspec>","startedAt":"2026-11-03T19:58:13Z","lastAt":"2026-11-04T16:34:22Z","messageCount":5,"participants":["Kevin Lyles","Kristoffer Haugsbakk","Junio C Hamano","Chris Torek"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"557368","messageId":"CY8PR17MB6236D16AD8CD80720FF1342DAE692@CY8PR17MB6236.namprd17.prod.outlook.com","threadId":"66759","inReplyTo":null,"subject":"Interaction between pre-commit hook and git commit <pathspec>","fromName":"Kevin Lyles","fromEmail":"klyles@epic.com","sentAt":"2026-11-03T19:58:13Z","receivedAt":"2026-11-03T19:58:13Z","isPatch":false,"sender":{"key":"klyles@epic.com","avatar":null},"body":"Hello, folks,\n\nThe documentation for the pathspec option of the commit command claims:\n\"The contents of these files are also staged for the next commit on top\nof what have been staged before.\"\nHowever, that appears to only be true if the pre-commit hook doesn't\nmodify the index contents.\n\nHere's an example pre-commit script:\n    #!/bin/sh\n    \n    git diff --staged --name-only | while read -r file\n    do\n      {\n        git cat-file blob :\"$file\"\n        echo '# Extra text'\n      } |\n        {\n          echo -n \"100644 blob \"\n          echo -n $(git hash-object -w -t blob --path \"$file\" --stdin)\n          echo -e \"\\t$file\"\n        } |\n          git update-index --index-info\n      echo '# Extra text' >> \"$file\"\n    done\n\nThis always appends \"# Extra text\" to the end of any staged files, in\nboth the index and the worktree. It serves to illustrate the issue,\nwithout getting into the weeds of what my pre-commit hook is doing.\nSuffice to say that my hook is (sometimes) modifying files in both the\nindex (to make sure the updated contents are committed) and the\nworktree (to reduce spurious diffs when file contents are changed).\n\nHere's a set of steps to reproduce the issue:\n1. Create a new repository with `git init'\n2. Set up the pre-commit script above\n3. Create a file and commit it\n4. Modify that file\n5. Run `git commit <file path>'\n6. See that the worktree includes the pre-commit changes, but the index\n   is missing them.  Alternatively, see that running `git add <file>'\n   results in that file having no changes.\n\nIf any other files are already staged between steps 3 and 5, those are\nincluded in the commit and the index includes the pre-commit changes.\nAdditionally, adding the file and then committing without specifying a\nfile path results in the expected behavior: both the index and worktree\nend up with the pre-commit changes.\n\nAre these differences in behavior intentional?\n\nIt's possible that there is a better way to achieve my goal than a\npre-commit hook.  I'm open to suggestions.  However, anything that\nwould run per-file instead of per-commit will likely be too slow to use\nin our (~4x the Linux kernel repository by file count) repository, at\nleast for some large commits.\n\nThank you,\nKevin\n\nKevin Lyles (he/him)\nEpic R&D - Hyperspace\nklyles@epic.com\n\n"},{"id":"557369","messageId":"de4f4f35-500f-4d82-acfd-ca601444f2d2@app.fastmail.com","threadId":"66759","inReplyTo":"CY8PR17MB6236D16AD8CD80720FF1342DAE692@CY8PR17MB6236.namprd17.prod.outlook.com","subject":"Re: Interaction between pre-commit hook and git commit <pathspec>","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-11-03T20:13:37Z","receivedAt":"2026-11-03T20:13:37Z","isPatch":false,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Tue, Nov 3, 2026, at 20:58, Kevin Lyles wrote:\n> Hello, folks,\n>\n> The documentation for the pathspec option of the commit command claims:\n> \"The contents of these files are also staged for the next commit on top\n> of what have been staged before.\"\n> However, that appears to only be true if the pre-commit hook doesn't\n> modify the index contents.\n>\n> Here's an example pre-commit script:\n>     #!/bin/sh\n>    \n>     git diff --staged --name-only | while read -r file\n>     do\n>       {\n>         git cat-file blob :\"$file\"\n>         echo '# Extra text'\n>       } |\n>         {\n>           echo -n \"100644 blob \"\n>           echo -n $(git hash-object -w -t blob --path \"$file\" --stdin)\n>           echo -e \"\\t$file\"\n>         } |\n>           git update-index --index-info\n>       echo '# Extra text' >> \"$file\"\n>     done\n>\n> This always appends \"# Extra text\" to the end of any staged files, in\n> both the index and the worktree. It serves to illustrate the issue,\n> without getting into the weeds of what my pre-commit hook is doing.\n> Suffice to say that my hook is (sometimes) modifying files in both the\n> index (to make sure the updated contents are committed) and the\n> worktree (to reduce spurious diffs when file contents are changed).\n>[snip]\n\nReading githooks documentation for\npre-commit. To me it reads like the use case\nis validation/checking, not any mutation.\nThis doesn't sound like a reasonable use case.\n\n"},{"id":"557371","messageId":"xmqqmrrp646a.fsf@gitster.g","threadId":"66759","inReplyTo":"de4f4f35-500f-4d82-acfd-ca601444f2d2@app.fastmail.com","subject":"Re: Interaction between pre-commit hook and git commit <pathspec>","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-11-03T20:38:53Z","receivedAt":"2026-11-03T20:38:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" <kristofferhaugsbakk@fastmail.com> writes:\n\n> Reading githooks documentation for pre-commit. To me it reads like\n> the use case is validation/checking, not any mutation.\n\nYes, the hook was written exactly for validation, to preempt an\nundesirable commit from being created.\n\nWe might want to do two things:\n\n(1) clarify the documentation to say that the hook should not try\n    mutation, and\n\n(2) detect and error out when the hook does try to make any\n    mutation.\n\nI am not sure if (2) is feasible, though.\n\nThanks.\n\n\n\n"},{"id":"557372","messageId":"CAPx1Gvd3H2-e5RBoH9e1r0cWqe2uo1tm-eUSop0ffNBc2Pj6tg@mail.gmail.com","threadId":"66759","inReplyTo":"CY8PR17MB6236D16AD8CD80720FF1342DAE692@CY8PR17MB6236.namprd17.prod.outlook.com","subject":"Re: Interaction between pre-commit hook and git commit <pathspec>","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2026-11-03T20:43:13Z","receivedAt":"2026-11-03T20:43:13Z","isPatch":false,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Tue, Nov 3, 2026 at 12:00 PM Kevin Lyles <klyles@epic.com> wrote:\n> The documentation for the pathspec option of the commit command claims:\n> \"The contents of these files are also staged for the next commit on top\n> of what have been staged before.\"\n> However, that appears to only be true if the pre-commit hook doesn't\n> modify the index contents.\n\nThere's an implementation detail here that makes this tricky.\n\nGit documentation talks about \"the\" index / staging-area.  Or, more\nprecisely, there is one such entity per work-tree: the work-tree's index.\n\nBut when you use \"git commit --only\" or \"git commit --include\", well,\nnow there is a big problem to deal with. If the commit is accepted,\nthe (\"the\") index / staging-area should be updated accordingly. But\nif it's aborted, the index should roll back to how it was before you\nran \"git commit [--only | --include] <pathspecs>\".\n\nIn order to make this work internally, Git actually needs two or\neven three index-es (indices?) / staging-areas for the work-tree\nin question: the one for \"before the commit\", possibly a separate\none for \"making the commit\" (when using --only), and one for\n\"after the commit succeeds, assuming it does succeed\".\n\nTo get a little bit more concrete (which makes describing the\nunderlying process a lot easier), let's note now that \"the index\"\nis really a simple file.  The main one is \".git/index\".  (I'm ignoring\nextra complications like a split index here as they don't affect the\nresult, so this is purely an internal implementation detail, but if\nyou ever dive into the Git code, it's something else to trip over....)\n\nGit handles the \"old version, temporary intermediate version\nif needed, new version\" problem by creating two or three files\ntotal. One of these is named \".git/index.lock\", which serves as\nboth a locking file to prevent two \"git commit\" operations from\nrunning at the same time and stepping on each other, and\nalso \"the future version of .git/index, should the commit succeed\".\n\n(The third file, which doesn't apply to your specific case,\nhas a funky temporary name with a unique set of characters\nsprinkled in.)\n\nIf your pre-commit hook attempts modification(s) of the proposed\nsnapshot for the new commit, it may need to update more than\none of these files to make all three conditions hold, and in general\nit just can't. (It's too hard to know what the correct names are,\nsince they're different in a work-tree and the \"--only\" name is\nrandomized.)  So pre-commit hooks are restricted to \"ok\" or\n\"not ok\" actions.\n\n> It's possible that there is a better way to achieve my goal than a\n> pre-commit hook.\n\nInstead of running \"git commit\" directly, why not run a script\nthat does the appropriate \"git add\"s, then runs \"git commit\"\nfor you?  The pre-commit hook can then inspect to be sure\nthat this has occurred, and if not, remind users to run the\nscript instead.\n\nChris\n\n"},{"id":"557463","messageId":"CY8PR17MB62362ECA0192916E14234BCBAE682@CY8PR17MB6236.namprd17.prod.outlook.com","threadId":"66759","inReplyTo":"CAPx1Gvd3H2-e5RBoH9e1r0cWqe2uo1tm-eUSop0ffNBc2Pj6tg@mail.gmail.com","subject":"RE: Interaction between pre-commit hook and git commit <pathspec>","fromName":"Kevin Lyles","fromEmail":"klyles@epic.com","sentAt":"2026-11-04T16:34:22Z","receivedAt":"2026-11-04T16:34:22Z","isPatch":false,"sender":{"key":"klyles@epic.com","avatar":null},"body":"On Tue, November 3, 2026 14:43 Chris Torek <chris.torek@gmail.com>\nwrote:\n> If your pre-commit hook attempts modification(s) of the proposed\n> snapshot for the new commit, it may need to update more than\n> one of these files to make all three conditions hold, and in general\n> it just can't. (It's too hard to know what the correct names are,\n> since they're different in a work-tree and the \"--only\" name is\n> randomized.)  So pre-commit hooks are restricted to \"ok\" or\n> \"not ok\" actions.\n\nIt sounds like this is one of the \"not ok\" actions.  I did look at\nupdating the \"primary\" index as well in this case, but ran into issues\nwith the current commit needing to be done before I can get that lock.\n\n> Instead of running \"git commit\" directly, why not run a script\n> that does the appropriate \"git add\"s, then runs \"git commit\"\n> for you?  The pre-commit hook can then inspect to be sure\n> that this has occurred, and if not, remind users to run the\n> script instead.\n\nThis will be rolled out across my company once we figure out how to\nimplement it.  The goal is to avoid changing how people interact with\ngit, while still making these changes automatically.  In particular,\nwe'd like it to still work when a file is partially staged.\n\nWe could pretty easily validate that the files are in the correct state\nin the pre-commit hook, but we're trying to avoid an extra step for\neach developer in normal workflows.\n\nI've also looked at implementing this as a clean filter, using a\nlong-running process for performance reasons.  However, also for\nperformance reasons, we need to know the full list of files to be\ncleaned at the start. Otherwise, some commits will take significantly\nlonger than currently, as we need to know the presence/absence of files\nnear the cleaned files to correctly update the file contents.  We can\nbatch finding those files, but only if we either know the full list of\nfiles being cleaned ahead of time or always get the full list of files\nin the repository.  `git ls-files' takes anywhere from ~500 ms (hot\ncache) to ~5 seconds in our repository when listing everything, so we'd\nprefer to avoid that performance hit when possible.\n\n"}]}