Jetpack: Tools: prepare-commit-msg.js is prepending "[not verified]" to commits after rebase.

[pento]

I just rebased #16699 from master, and noticed after I pushed it that [not verified] has been prepended to every commit message. It seems this message is coming from prepare-commit-msg.js.

I suspect that something has changed in the latest version of the GitHub desktop app: this is the first rebase I've done since upgrading it, and I'm not aware of anything else changing that might cause this.

I've marked this issue as High priority, since some quick testing shows that the next rebase will prepend an additional [not verified] to all of the commits already marked with [not verified], and this is going to get out of hand very quickly.

Additionally, I'd appreciate if someone more well versed in CLI git than I could come up with a command to undo the damage.

Steps to reproduce the issue

All of these steps should be performed in the GitHub desktop app.

1. Create a new branch off of master.
2. Make some changes, and commit them. (I don't know if you also need to push the changes.)
3. Open the Branch menu, select "Rebase current branch...", and click "Start rebase".
4. Observe [not verified] being prepended to the commit message.

What I expected

For my commit messages to not be modified.

What happened instead

My commit messages were modified.

@Automattic/jetpack-crew

[jeherve]

I'm not very familiar with the GitHub desktop app, but I am used to experiencing this with my rebases, whenever I do not stop to reword my commits during my rebase. I believe that's expected.

Maybe we could make changes to our pre-commit check so it works around that during rebases?

[pento]

Interesting, this is the first time it's ever happened to me when rebasing, I figured it was some desktop app related jankiness.

This seems like a bug in the pre-commit check, then.

[leogermani]

I thought as well that this was the expected behavior since our hooks don't verify each commit when rebasing.

What I've been doing is:

• reset --hard to the latest commit from master in my branch
• rebase master
• cherry-pick -n each of the commits in my branch again

It's a pain

[jeherve]

I personally do interactive rebases and squash or reword as I go so the pre-commit hook is triggered.

[sergeymitr]

I too think this is the expected behavior. The commits were not verified after all.
Since we squash commits before merging branches into master, I believe this doesn't really matter if [not verified] is there or not.
What I usually do after a rebase is:

1. Check the git log to see how many commits got the [not verified] prefix.
2. Run an interactive rebase as far back as needed, e.g. git rebase -i HEAD~4
3. For each commit I need to rewrite I run git commit --amend and edit the commit message

See https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History, section "Changing Multiple Commit Messages".

[jeherve]

I believe this doesn't really matter if [not verified] is there or not.

It can be useful for reviewers though, so I think it'd be nice to find a good work-around for this. Maybe there is a different hook we could rely on that's called on rebase?

[sergeymitr]

@jeherve after some playing with rebasing, I wasn't able to reproduce the problem.
I tried both the command line git and the GitHub Desktop app, no luck.
I've run into it in the past, but it didn't happen often.

The problem is not rebasing itself, because the prepare-commit-msg hook is not run during the rebase, so [not verified] prefix doesn't get added to commit.
The problem is that in some cases the hook gets run over rebased commits, most likely after resolving conflicts.
In particular, this code is responsible for the [not verified] message (bin/prepare-commit-msg.js):

if ( Date.now() - commitDate > 2000 /* 2sec*/ ) {
    console.log( 'WARNING: git pre-commit hook was skipped!' );
    const commitMsg = fs.readFileSync( '.git/COMMIT_EDITMSG' );
    const newCommitMsg = '[not verified] ' + commitMsg.toString();

    fs.writeFileSync( '.git/COMMIT_EDITMSG', newCommitMsg );
}

So if pre-commit hook is skipped (the one that saves the commitDate), but prepare-commit-msg gets run, we mark the commit as "not verified".
Can you think of a use case when this could happen?

[jeherve]

Good point.

Can you think of a use case when this could happen?

I believe you can get it to happen with any cherry-pick from one branch to another.

[pento]

Since we squash commits before merging branches into master, I believe this doesn't really matter if [not verified] is there or not.

@jeherve mentioned that commit messages in a PR are still useful for reviewers. Also, it will continue to add [not verified] to commit messages on every rebase, which will make the commit messages go from "slightly annoying, but still readable" to "completely unusable" after a few rebases.

The problem is that in some cases the hook gets run over rebased commits, most likely after resolving conflicts.

Ah, that's potentially it, I did have to resolve a conflict in that rebase.

[pento]

What I usually do after a rebase is:

1. Check the `git log` to see how many commits got the `[not verified]` prefix.

2. Run an interactive rebase as far back as needed, e.g. `git rebase -i HEAD~4`

3. For each commit I need to rewrite I run `git commit --amend` and edit the commit message

See git-scm.com/book/en/v2/Git-Tools-Rewriting-History, section "Changing Multiple Commit Messages".

Thank you for that: it worked, though was kind of painful to have to go through 34 commits. At least vim remembers commands between sessions, so I only had to write the search/replace command once. 🙂

[pento]

Husky recommends setting HUSKY_SKIP_HOOKS=1 when doing a rebase. 🙂