Hi Samuel
On 13/10/2020 11:43, Samuel Čavoj wrote:
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.
Oh sorry for some reason I thought we just ignored opts->gpg_sign before
your patch I'd forgotten we actually passed it without the '-S' - thanks
for correcting me.
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.
It was dropping my emails sent from a gmail account a few weeks back
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.
He's subscribed to the list so hopefully will see them that way
Best Wishes
Phillip
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
