fix bugs in am/rebase handling of committer ident/date

Quoting from Jeff King's commit message:

    Commit e8cbe2118a (am: stop exporting GIT_COMMITTER_DATE, 2020-08-17)
    rewrote the code for setting the committer date to use fmt_ident(),
    rather than setting an environment variable and letting commit_tree()
    handle it. But it introduced two bugs:

      - we use the author email string instead of the committer email

      - when parsing the committer ident, we used the wrong variable to
	compute the length of the email, resulting in it always being a
	zero-length string

The regression affected both am and rebase.  Apply the upstream fixes.

References:
https://lore.kernel.org/git/20201023070747.GA2198273@coredump.intra.peff.net/
This commit is contained in:
Todd Zullinger 2020-10-23 23:10:29 -04:00
parent f3a190b8da
commit 79a4aef788
2 changed files with 216 additions and 0 deletions

View File

@ -0,0 +1,206 @@
From 56706dba33f5d4457395c651cf1cd033c6c03c7a Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Fri, 23 Oct 2020 03:08:43 -0400
Subject: [PATCH 1/3] t3436: check --committer-date-is-author-date result more
carefully
After running "rebase --committer-date-is-author-date", we confirm that
the committer date is the same as the author date. However, we don't
look at any other parts of the committer ident line to make sure we
didn't screw them up. And indeed, there are a few bugs here. Depending
on the rebase backend in use, we may accidentally use the author email
instead of the committer's, or even an empty string.
Let's teach our test_ctime_is_atime helper to check the committer name
and email, which reveals several failing tests.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t3436-rebase-more-options.sh | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh
index 996e82787e..6f2f49717b 100755
--- a/t/t3436-rebase-more-options.sh
+++ b/t/t3436-rebase-more-options.sh
@@ -65,31 +65,31 @@ test_expect_success '--ignore-whitespace is remembered when continuing' '
'
test_ctime_is_atime () {
- git log $1 --format=%ai >authortime &&
- git log $1 --format=%ci >committertime &&
+ git log $1 --format="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> %ai" >authortime &&
+ git log $1 --format="%cn <%ce> %ci" >committertime &&
test_cmp authortime committertime
}
-test_expect_success '--committer-date-is-author-date works with apply backend' '
+test_expect_failure '--committer-date-is-author-date works with apply backend' '
GIT_AUTHOR_DATE="@1234 +0300" git commit --amend --reset-author &&
git rebase --apply --committer-date-is-author-date HEAD^ &&
test_ctime_is_atime -1
'
-test_expect_success '--committer-date-is-author-date works with merge backend' '
+test_expect_failure '--committer-date-is-author-date works with merge backend' '
GIT_AUTHOR_DATE="@1234 +0300" git commit --amend --reset-author &&
git rebase -m --committer-date-is-author-date HEAD^ &&
test_ctime_is_atime -1
'
-test_expect_success '--committer-date-is-author-date works with rebase -r' '
+test_expect_failure '--committer-date-is-author-date works with rebase -r' '
git checkout side &&
GIT_AUTHOR_DATE="@1234 +0300" git merge --no-ff commit3 &&
git rebase -r --root --committer-date-is-author-date &&
test_ctime_is_atime
'
-test_expect_success '--committer-date-is-author-date works when forking merge' '
+test_expect_failure '--committer-date-is-author-date works when forking merge' '
git checkout side &&
GIT_AUTHOR_DATE="@1234 +0300" git merge --no-ff commit3 &&
PATH="./test-bin:$PATH" git rebase -r --root --strategy=test \
@@ -145,7 +145,7 @@ test_expect_success '--reset-author-date works with rebase -r' '
test_atime_is_ignored
'
-test_expect_success '--reset-author-date with --committer-date-is-author-date works' '
+test_expect_failure '--reset-author-date with --committer-date-is-author-date works' '
test_must_fail git rebase -m --committer-date-is-author-date \
--reset-author-date --onto commit2^^ commit2^ commit3 &&
git checkout --theirs foo &&
From 16b0bb99eac5ebd02a5dcabdff2cfc390e9d92ef Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Fri, 23 Oct 2020 03:09:39 -0400
Subject: [PATCH 2/3] am: fix broken email with --committer-date-is-author-date
Commit e8cbe2118a (am: stop exporting GIT_COMMITTER_DATE, 2020-08-17)
rewrote the code for setting the committer date to use fmt_ident(),
rather than setting an environment variable and letting commit_tree()
handle it. But it introduced two bugs:
- we use the author email string instead of the committer email
- when parsing the committer ident, we used the wrong variable to
compute the length of the email, resulting in it always being a
zero-length string
This commit fixes both, which causes our test of this option via the
rebase "apply" backend to now succeed.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/am.c | 4 ++--
t/t3436-rebase-more-options.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 896cd0f827..af931e588c 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -161,7 +161,7 @@ static void am_state_init(struct am_state *state)
state->committer_name =
xmemdupz(id.name_begin, id.name_end - id.name_begin);
state->committer_email =
- xmemdupz(id.mail_begin, id.mail_end - id.mail_end);
+ xmemdupz(id.mail_begin, id.mail_end - id.mail_begin);
}
/**
@@ -1595,7 +1595,7 @@ static void do_commit(const struct am_state *state)
if (state->committer_date_is_author_date)
committer = fmt_ident(state->committer_name,
- state->author_email, WANT_COMMITTER_IDENT,
+ state->committer_email, WANT_COMMITTER_IDENT,
state->ignore_date ? NULL
: state->author_date,
IDENT_STRICT);
diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh
index 6f2f49717b..3fda2235bd 100755
--- a/t/t3436-rebase-more-options.sh
+++ b/t/t3436-rebase-more-options.sh
@@ -70,7 +70,7 @@ test_ctime_is_atime () {
test_cmp authortime committertime
}
-test_expect_failure '--committer-date-is-author-date works with apply backend' '
+test_expect_success '--committer-date-is-author-date works with apply backend' '
GIT_AUTHOR_DATE="@1234 +0300" git commit --amend --reset-author &&
git rebase --apply --committer-date-is-author-date HEAD^ &&
test_ctime_is_atime -1
From 5f35edd9d7ebca17f205b338d340cc6ce214644a Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Fri, 23 Oct 2020 03:10:15 -0400
Subject: [PATCH 3/3] rebase: fix broken email with
--committer-date-is-author-date
Commit 7573cec52c (rebase -i: support --committer-date-is-author-date,
2020-08-17) copied the committer ident-parsing code from builtin/am.c.
And in doing so, it copied a bug in which we always set the email to an
empty string. We fixed the version in git-am in the previous commit;
this commit fixes the copied code.
Reported-by: VenomVendor <info@venomvendor.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
sequencer.c | 2 +-
t/t3436-rebase-more-options.sh | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 4ccb5451a9..9dcd3db3ef 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4369,7 +4369,7 @@ static int init_committer(struct replay_opts *opts)
opts->committer_name =
xmemdupz(id.name_begin, id.name_end - id.name_begin);
opts->committer_email =
- xmemdupz(id.mail_begin, id.mail_end - id.mail_end);
+ xmemdupz(id.mail_begin, id.mail_end - id.mail_begin);
return 0;
}
diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh
index 3fda2235bd..eaaf4c8d1d 100755
--- a/t/t3436-rebase-more-options.sh
+++ b/t/t3436-rebase-more-options.sh
@@ -76,20 +76,20 @@ test_expect_success '--committer-date-is-author-date works with apply backend' '
test_ctime_is_atime -1
'
-test_expect_failure '--committer-date-is-author-date works with merge backend' '
+test_expect_success '--committer-date-is-author-date works with merge backend' '
GIT_AUTHOR_DATE="@1234 +0300" git commit --amend --reset-author &&
git rebase -m --committer-date-is-author-date HEAD^ &&
test_ctime_is_atime -1
'
-test_expect_failure '--committer-date-is-author-date works with rebase -r' '
+test_expect_success '--committer-date-is-author-date works with rebase -r' '
git checkout side &&
GIT_AUTHOR_DATE="@1234 +0300" git merge --no-ff commit3 &&
git rebase -r --root --committer-date-is-author-date &&
test_ctime_is_atime
'
-test_expect_failure '--committer-date-is-author-date works when forking merge' '
+test_expect_success '--committer-date-is-author-date works when forking merge' '
git checkout side &&
GIT_AUTHOR_DATE="@1234 +0300" git merge --no-ff commit3 &&
PATH="./test-bin:$PATH" git rebase -r --root --strategy=test \
@@ -145,7 +145,7 @@ test_expect_success '--reset-author-date works with rebase -r' '
test_atime_is_ignored
'
-test_expect_failure '--reset-author-date with --committer-date-is-author-date works' '
+test_expect_success '--reset-author-date with --committer-date-is-author-date works' '
test_must_fail git rebase -m --committer-date-is-author-date \
--reset-author-date --onto commit2^^ commit2^ commit3 &&
git checkout --theirs foo &&

View File

@ -129,6 +129,15 @@ Source99: print-failed-test-output
# https://bugzilla.redhat.com/490602
Patch0: git-cvsimport-Ignore-cvsps-2.2b1-Branches-output.patch
# Fix bugs in am/rebase handling of committer ident/date discussed at
# https://lore.kernel.org/git/20201023070747.GA2198273@coredump.intra.peff.net/
#
# The following three commits make up the patch
# https://github.com/git/git/commit/56706dba33f5d4457395c651cf1cd033c6c03c7a
# https://github.com/git/git/commit/16b0bb99eac5ebd02a5dcabdff2cfc390e9d92ef
# https://github.com/git/git/commit/5f35edd9d7ebca17f205b338d340cc6ce214644a
Patch1: git-2.29.0-committer-date-is-author-date-fix.patch
%if %{with docs}
# pod2man is needed to build Git.3pm
BuildRequires: %{_bindir}/pod2man
@ -1078,6 +1087,7 @@ rmdir --ignore-fail-on-non-empty "$testdir"
%changelog
* Sat Oct 24 2020 Todd Zullinger <tmz@pobox.com> - 2.29.1-1
- update to 2.29.1
- fix bugs in am/rebase handling of committer ident/date
* Mon Oct 19 2020 Todd Zullinger <tmz@pobox.com> - 2.29.0-1
- update to 2.29.0