Re: [PATCH v2] sequencer: require trailing NL in footers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 26, 2017, 02:08 UTC
- Message-ID
- <xmqqinlrudij.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170426000741.GS28740@aiede.svl.corp.google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 20 quoted lines
> Jonathan Tan wrote: > >> Reported-by: Brian Norris <computersforpeace@gmail.com> >> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> >> --- > [...] >> sequencer.c | 11 +++++++++++ >> t/t3511-cherry-pick-x.sh | 14 ++++++++++++++ >> 2 files changed, 25 insertions(+) > > Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> > > This is still pretty subtle (using the added newline that is added after > a non-footer to turn the invalid footer into a valid one), but > > * it is clear from the code that it will work correctly > * the new test ensures we'll continue to support this case > * it is understandable after a little head scratching > * I'm hoping the subtlety will go away once the code learns to deal > with ordinary non-footer text that has a missing newline
Hmph, perhaps we need another test that documents a known failure as well in the meantime?
Show 20 quoted lines
> [...]
>> --- a/t/t3511-cherry-pick-x.sh
>> +++ b/t/t3511-cherry-pick-x.sh
>> @@ -208,6 +208,20 @@ test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists fo
>> test_cmp expect actual
>> '
>>
>> +test_expect_success 'cherry-pick -x handles commits with no NL at end of message' '
>> + pristine_detach initial &&
>> + sha1=$(printf "title\n\nSigned-off-by: a" | git commit-tree -p initial mesg-with-footer^{tree}) &&
>
> nit: Should this use a more typical sign-off line with an email
> address, to avoid a false-positive success in case git becomes more
> strict about its signoffs in the future?
>
> Something like
>
> printf "title\n\nSigned-off-by: S. I. Gner <signer@example.com>" >msg &&
> sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) &&
> ...That is a good point and has an added benefit that the test script becomes easier to follow.
t/t3511-cherry-pick-x.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh index 6f518020b2..c2b143802d 100755 --- a/t/t3511-cherry-pick-x.sh +++ b/t/t3511-cherry-pick-x.sh @@ -210,12 +210,14 @@ test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists fo test_expect_success 'cherry-pick -x handles commits with no NL at end of message' ' pristine_detach initial && - sha1=$(printf "title\n\nSigned-off-by: a" | git commit-tree -p initial mesg-with-footer^{tree}) && + signer="S. I. Gner <signer@example.com>" && + printf "title\n\nSigned-off-by: %s" "$signer" >msg && + sha1=$(git commit-tree -p initial mesg-with-footer^{tree} <msg) && git cherry-pick -x $sha1 && cat <<-EOF >expect && title - Signed-off-by: a + Signed-off-by: $signer (cherry picked from commit $sha1) EOF git log -1 --pretty=format:%B >actual &&