{"thread":{"id":"61320","subject":"prepare-commit-msg hook during rebase","startedAt":"2024-04-15T10:42:14Z","lastAt":"2024-04-15T14:32:44Z","messageCount":3,"participants":["Michael J Gruber","Phillip Wood"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"492935","messageId":"CAA19uiQip=EsbT8uQCs+98brYSTbOqyHX1ZBhssi4df3dZmZ6Q@mail.gmail.com","threadId":"61320","inReplyTo":null,"subject":"prepare-commit-msg hook during rebase","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2024-04-15T10:36:08Z","receivedAt":"2024-04-15T10:42:14Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Hi there\n\nFor a while now, I thought I was using prepare-commit-msg hook wrong\nbut finally took the time to analyse this further. Per the doc, the\nhook receives the source of the commit message as $2\n(message/template/merge/squash/commit or empty).\n\nI notice the following with `rebase -i`:\n- When rb applies a patch to be edited/reworded, the hook is called\nwith `commit` as the message source.\n- When rb applies a patch merely to be picked, the hook is called with\n`message` as the message source.\n\nThe latter also happens when non-interactive rebase applies commits.\n\nI find this confusing for two reasons:\n- Whether edit/reword or pick, there is always a commit being applied,\nand it's the source of the message. So why not `commit` in both cases?\n- The doc says that `message` is for inputs from `-m` or `-F`. So,\ncertainly this should not apply when the message comes from a picked\ncommit.\n\nAlso, I'm not sure whether the claim about `-m` is true, but that's\nanother issue; we even might want to distinguish between `-m` and `-F`\nhere.\n\nDoes the source `message` during rb pick occur due to an\nimplementation detail, maybe since the rewrite to sequencer?\n\nCheers\nMichael\n"},{"id":"492944","messageId":"bf27ed3a-dc6c-4716-95c6-9e6c60604bd3@gmail.com","threadId":"61320","inReplyTo":"CAA19uiQip=EsbT8uQCs+98brYSTbOqyHX1ZBhssi4df3dZmZ6Q@mail.gmail.com","subject":"Re: prepare-commit-msg hook during rebase","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-04-15T14:01:31Z","receivedAt":"2024-04-15T14:01:44Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Michael\n\nOn 15/04/2024 11:36, Michael J Gruber wrote:\n> Hi there\n> \n> For a while now, I thought I was using prepare-commit-msg hook wrong\n> but finally took the time to analyse this further. Per the doc, the\n> hook receives the source of the commit message as $2\n> (message/template/merge/squash/commit or empty).\n> \n> I notice the following with `rebase -i`:\n> - When rb applies a patch to be edited/reworded, the hook is called\n> with `commit` as the message source.\n> - When rb applies a patch merely to be picked, the hook is called with\n> `message` as the message source.\n> \n> The latter also happens when non-interactive rebase applies commits.\n> \n> I find this confusing for two reasons:\n> - Whether edit/reword or pick, there is always a commit being applied,\n> and it's the source of the message. So why not `commit` in both cases?\n> - The doc says that `message` is for inputs from `-m` or `-F`. So,\n> certainly this should not apply when the message comes from a picked\n> commit.\n> \n> Also, I'm not sure whether the claim about `-m` is true, but that's\n> another issue; we even might want to distinguish between `-m` and `-F`\n> here.\n> \n> Does the source `message` during rb pick occur due to an\n> implementation detail, maybe since the rewrite to sequencer?\n\nAll of the hook behavior in rebase is an accident of the implementation \nand stems from the scripted implementation. I'm not sure if the source \npassed to the hook has changed over time but it is quite possible that \nit has. I agree \"commit\" would be a more logical source. Personally I'm \nnot sure it makes sense to run this hook for ordinary picks, though \nperhaps it makes sense to run it when the message is to be edited. Do \nyou have a use for this hook while rebasing or are you just confused by \nthe source argument?\n\nBest Wishes\n\nPhillip\n\n> Cheers\n> Michael\n> \n"},{"id":"492947","messageId":"171319155563.9055.10218254775303206647.git@grubix.eu","threadId":"61320","inReplyTo":"bf27ed3a-dc6c-4716-95c6-9e6c60604bd3@gmail.com","subject":"Re: prepare-commit-msg hook during rebase","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2024-04-15T14:32:35Z","receivedAt":"2024-04-15T14:32:44Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Phillip Wood venit, vidit, dixit 2024-04-15 16:01:31:\n> Hi Michael\n> \n> On 15/04/2024 11:36, Michael J Gruber wrote:\n> > Hi there\n> > \n> > For a while now, I thought I was using prepare-commit-msg hook wrong\n> > but finally took the time to analyse this further. Per the doc, the\n> > hook receives the source of the commit message as $2\n> > (message/template/merge/squash/commit or empty).\n> > \n> > I notice the following with `rebase -i`:\n> > - When rb applies a patch to be edited/reworded, the hook is called\n> > with `commit` as the message source.\n> > - When rb applies a patch merely to be picked, the hook is called with\n> > `message` as the message source.\n> > \n> > The latter also happens when non-interactive rebase applies commits.\n> > \n> > I find this confusing for two reasons:\n> > - Whether edit/reword or pick, there is always a commit being applied,\n> > and it's the source of the message. So why not `commit` in both cases?\n> > - The doc says that `message` is for inputs from `-m` or `-F`. So,\n> > certainly this should not apply when the message comes from a picked\n> > commit.\n> > \n> > Also, I'm not sure whether the claim about `-m` is true, but that's\n> > another issue; we even might want to distinguish between `-m` and `-F`\n> > here.\n> > \n> > Does the source `message` during rb pick occur due to an\n> > implementation detail, maybe since the rewrite to sequencer?\n> \n> All of the hook behavior in rebase is an accident of the implementation \n> and stems from the scripted implementation. I'm not sure if the source \n> passed to the hook has changed over time but it is quite possible that \n> it has. I agree \"commit\" would be a more logical source. Personally I'm \n> not sure it makes sense to run this hook for ordinary picks, though \n> perhaps it makes sense to run it when the message is to be edited. Do \n> you have a use for this hook while rebasing or are you just confused by \n> the source argument?\n\nIt is run during a rebase, and I don't have a use for it there, in\nparticular not for picked commits which I do not amend. That is what\ntriggered my investigation (unintended runs).\n\nIn fact, my hook messes up commits during a rebase if I run rebase from\na subdirectory, and I am sure that it didn't (quite) some time in the\npast. But this can have several reasons:\n\n- prepare-commit-msg was not run for rebase picks at all in the past\n- prepare-commit-msg was run with source \"commit\" in the past\n- prepare-commit-msg was run from the topdir (not cwd) in the past\n\nI can avoid the mess by detecting the rebase state in the hook (or\ncd'ing to topdir before the rebase), but I'm wondering what changed and\nwhat is the right behaviour (to which I would adjust, of course).\n\nAlso, looking at sequencer.c from line 6545, sequencer_determine_whence()\nlooks funny: FROM_CHERRY_PICK_MULTI from the first if will always be\noverwritten by the second if/else. I'm not saying this is strictly\nrelated, but we do have untested code paths in that area. whence is part\nof what makes commit decide on hooks being run and arguments passed to\nthem.\n\nMichael\n"}]}