ejoffe / spr

Stacked Pull Requests on GitHub
MIT License
717 stars 65 forks source link

Git commit.verbose breaks spr_reword_helper #316

Open felixge opened 1 year ago

felixge commented 1 year ago

Steps to reproduce:

git config --global commit.verbose true
touch foo
git add foo
git commit -m "add foo"
git spr update

Expected result:

spr should upload my PR.

Actual result:

panic: unable to fetch local commits
 most likely this is an issue with missing commit-id in the commit body

goroutine 1 [running]:
github.com/ejoffe/spr/git.GetLocalCommitStack(0x14000113e10, {0x101188478, 0x140001230c8})
    /Users/runner/work/spr/spr/git/helpers.go:78 +0x314
github.com/ejoffe/spr/github/githubclient.(*client).GetInfo(0x14000123110, {0x1011882f0, 0x14000116000}, {0x101188478, 0x140001230c8})
    /Users/runner/work/spr/spr/github/githubclient/client.go:189 +0x134
github.com/ejoffe/spr/spr.(*stackediff).fetchAndGetGitHubInfo(0x14000129020, {0x1011882f0, 0x14000116000})
    /Users/runner/work/spr/spr/spr/spr.go:477 +0x1bc
github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0x14000129020, {0x1011882f0, 0x14000116000}, {0x0, 0x0, 0x0}, 0x0)
    /Users/runner/work/spr/spr/spr/spr.go:119 +0x6c
main.main.func4(0x1400012b500)
    /Users/runner/work/spr/spr/cmd/spr/main.go:155 +0x160
github.com/urfave/cli/v2.(*Command).Run(0x1400021a900, 0x1400012b280)
    /Users/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/command.go:163 +0x634
github.com/urfave/cli/v2.(*App).RunContext(0x1400019e9c0, {0x1011882f0, 0x14000116000}, {0x1400012e000, 0x2, 0x2})
    /Users/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:313 +0x760
github.com/urfave/cli/v2.(*App).Run(...)
    /Users/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:224
main.main()
    /Users/runner/work/spr/spr/cmd/spr/main.go:224 +0x150c

Analysis:

This seems to be caused by spr_reword_helper not being able to deal with commit messages formatted in verbose format. An example of such a message is shown below:

add foo

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Date:      Fri May 5 09:22:37 2023 +0200
#
# On branch main
# Your branch is ahead of 'origin/main' by 1 commit.
#   (use "git push" to publish your local commits)
#
# Changes to be committed:
#   new file:   foo
#
# ------------------------ >8 ------------------------
# Do not modify or remove the line above.
# Everything below it will be ignored.
diff --git c/foo i/foo
new file mode 100644
index 0000000..e69de29

This could probably be fixed by making appendCommitID a bit smarter. Right now it just appends the commit-id trailer, which doesn't work for verbosely formatted commit messages:

https://github.com/ejoffe/spr/blob/85b71efde7f9454c65f0a916a08e4e76a776d2a5/cmd/reword/main.go#L82-L93

Workaround:

Disabling verbose commit output does the trick.

git config --global commit.verbose false

When needed, one can enable it on demand it via git commit -v. I use this for seeing the code I'm describing in my commit message.

Unfortunately this doesn't work when rewording commit messages via git rebase -i. This is my main use case for having verbose enabled by default. The workaround to this is using my editors ability to show me the diff for the current commit, but it's a bit clunky.

eax-anyscale commented 1 year ago

can't we do something like git interpret-trailers --if-exists doNothing --trailer 'commit-id:ABCDEF --in-place instead of manually re-implementing this?