Hi Phillip, On 13.10.2020 10:55, Phillip Wood wrote: > Hi Samuel > > Thanks for re-rolling this > > On 13/10/2020 00:49, Samuel Čavoj wrote: > > From: Johannes Schindelin <Johannes.Schindelin@xxxxxx> > > > > When performing a rebase with --rebase-merges using either a custom > > strategy specified with -s or an octopus merge, and at the same time > > having gpgsign enabled (either rebase -S or config commit.gpgsign), the > > operation would fail on making the merge commit. > > Small nit-pick: I think it worked fine with if commit.gpgsign was set and > the user did not pass -S or --no-gpg-sign because merge would sign the > commits as commit.gpgsign was set, it was only if the user passed a gpg > signing option to rebase that we had problems. I'm not sure it's worth a > re-roll just for that though This is not the case. That's how I encountered the problem initially, I have commit.gpgsign set to true globally. I ran a rebase -ir, over an octopus merge and then it would fail with an error in the lines of 'not something we can merge'. I later found out it didn't happen on my laptop, where gpgsign is not set, so that's what gave it away. In either case, I did not pass neither -S nor --no-gpg-sign to rebase. Yes, _if_ the merge command went through, it would have choosen the correct signing behaviour (i.e. the default), but the merge died, because an empty string was being passed to it as one of the commits to merge. off-topic p.s.: My mail server does not currently have a proper rDNS record (yeah yeah, I know) and for this reason, gmx.net drops my emails unconditionally. As such, I am unable to send emails directly to Johannes.Schindelin@xxxxxx, without major hackery, anyway. I am dropping him from Cc, as to prevent sad mailservers. I hope these messages reach him via the mailing list. Regards, Samuel > > Best Wishes > > Phillip > > > Instead of "-S%s" with > > the key id substituted, only the bare key id would get passed to the > > underlying merge command, which tried to interpret it as a ref. > > > > Fix the issue and add a test case as suggested by Johannes Schindelin. > > > > Signed-off-by: Samuel Čavoj <samuel@xxxxxxxxx> > > --- > > changed v1 -> v2: > > added test case > > --- > > sequencer.c | 2 +- > > t/t3435-rebase-gpg-sign.sh | 6 ++++++ > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/sequencer.c b/sequencer.c > > index 00acb12496..88ccff4838 100644 > > --- a/sequencer.c > > +++ b/sequencer.c > > @@ -3677,7 +3677,7 @@ static int do_merge(struct repository *r, > > strvec_push(&cmd.args, "-F"); > > strvec_push(&cmd.args, git_path_merge_msg(r)); > > if (opts->gpg_sign) > > - strvec_push(&cmd.args, opts->gpg_sign); > > + strvec_pushf(&cmd.args, "-S%s", opts->gpg_sign); > > /* Add the tips to be merged */ > > for (j = to_merge; j; j = j->next) > > diff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh > > index b47c59c190..f70b280f5c 100755 > > --- a/t/t3435-rebase-gpg-sign.sh > > +++ b/t/t3435-rebase-gpg-sign.sh > > @@ -68,4 +68,10 @@ test_expect_failure 'rebase -p --no-gpg-sign override commit.gpgsign' ' > > test_must_fail git verify-commit HEAD > > ' > > +test_expect_success 'rebase -r, GPG and merge strategies' ' > > + git reset --hard merged && > > + git rebase -fr --gpg-sign -s resolve --root && > > + git verify-commit HEAD > > +' > > + > > test_done > > >
