spr icon indicating copy to clipboard operation
spr copied to clipboard

Adding git commit-msg hook fails if a hook already exists

Open josephschmitt opened this issue 3 years ago • 6 comments

The current logic for installing the commit-msg hook is to symlink the spr_commit_hook bin directly into .git/hooks/commit-msg. However, if a user already has a commit hook of this name, it just fails silently, and you don't find out it was a problem until you try to spr update and it complaints about the commit-id trailer being missing.

Instead, we should check to see if a hook already exists, and if so, append spr_commit_hook "$@" to the bottom of it (if we want to be REALLY safe, we can check that it's a bash plaintext file first). If one doesn't exist, we can continue with the existing behavior.

PR to fix this incoming shortly.

josephschmitt avatar Jan 18 '22 18:01 josephschmitt

@josephschmitt - I think this was fixed, right?

ejoffe avatar Mar 30 '22 22:03 ejoffe

I’m honestly not sure, I submitted this PR a while ago. I can look through the commit history.

josephschmitt avatar Mar 31 '22 03:03 josephschmitt

I just ran into this issue on the latest release.

Version: 0.9.1 Platform: linux Installed by: copying files from the tar to ~/.local/bin

TheNumberOne avatar Aug 05 '22 23:08 TheNumberOne

@TheNumberOne - can you paste the error? does it happened only once? or keeps happening?

ejoffe avatar Aug 06 '22 02:08 ejoffe

➜  triplebyte git:(rosetta/candidate-pipeline-revamp/cleaned-up) git spr update
> git rev-parse --show-toplevel
> git fetch
> git branch --no-color
> git rebase origin/master --autostash
> github fetch pull requests
> git branch --no-color
> git branch --no-color
> git branch --no-color
> git log --no-color origin/master..HEAD
> git rev-parse --git-common-dir
> git branch --no-color
> git rebase origin/master -i --autosquash --autostash
> git log --no-color origin/master..HEAD
panic: unable to fetch local commits
 most likely this is an issue with missing commit-id in the commit body
 which is caused by the commit-msg hook not being installed propertly


goroutine 1 [running]:
github.com/ejoffe/spr/spr.(*stackediff).getLocalCommitStack(0xc0000ce960)
        /Users/runner/work/spr/spr/spr/spr.go:334 +0x1db
github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0xc0000ce960, {0x927870, 0xc000028110}, {0x0, 0x0, 0x0}, 0x0)
        /Users/runner/work/spr/spr/spr/spr.go:118 +0xe5
main.main.func3(0xc0000cea80)
        /Users/runner/work/spr/spr/cmd/spr/main.go:143 +0xdf
github.com/urfave/cli/v2.(*Command).Run(0xc0000dd8c0, 0xc0000aa640)
        /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:163 +0x64a
github.com/urfave/cli/v2.(*App).RunContext(0xc0000824e0, {0x927870, 0xc000028110}, {0xc00001e040, 0x2, 0x2})
        /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:313 +0x81e
github.com/urfave/cli/v2.(*App).Run(...)
        /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:224
main.main()
        /Users/runner/work/spr/spr/cmd/spr/main.go:200 +0x1099
➜  triplebyte git:(rosetta/candidate-pipeline-revamp/cleaned-up) cat ./.git/hooks/commit-msg
#!/bin/sh
# husky

# Created by Husky v4.2.5 (https://github.com/typicode/husky#readme)
#   At: 7/11/2022, 1:10:24 AM
#   From: /home/rose/repos/triplebyte/triplebyte/node_modules/husky (https://github.com/typicode/husky#readme)

. "$(dirname "$0")/husky.sh"

/home/rose/.local/bin/spr_commit_hook "$@"
➜  triplebyte git:(rosetta/candidate-pipeline-revamp/cleaned-up) 

TheNumberOne avatar Aug 08 '22 20:08 TheNumberOne

For some reason which we haven't root caused yet, the commit message hook did not add a commit id to one of your commits. One workaround to make forward progress is to just manually insert a commit-id at the end of the commit message, you can reword the commit and add it. Then git spr update should work fine. the format for the commit id is: commit-id:a04d4b03 you can choose any unique random id you want, it must be 8 hex characters.

ejoffe avatar Aug 09 '22 22:08 ejoffe