git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Philippe Blain <levraiphilippeblain@gmail.com>,
	Philippe Blain <levraiphilippeblain@gmail.com>
Subject: [PATCH v5 0/2] ref-filter: handle CRLF at end-of-line more gracefully
Date: Thu, 29 Oct 2020 12:48:27 +0000	[thread overview]
Message-ID: <pull.576.v5.git.1603975709.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.576.v4.git.1603335680.gitgitgadget@gmail.com>

Changes since v4 :

 * tweaked the wording of both commit messages to not call the behaviour a
   bug
 * added || return 1 in one test - that had already been squashed in by
   Junio


----------------------------------------------------------------------------

The ref-filter code does not correctly handle commit or tag messages that
use CRLF as the line terminator. Such messages can be created with the
--cleanup=verbatim option of git commit and git tag, or by using git
commit-tree directly.

The function find_subpos in ref-filter.c looks for two consecutive '\n' to
find the end of the subject line, a sequence which is absent in messages
using CRLF. This results in the whole message being parsed as the subject
line (%(contents:subject)), and the body of the message (%(contents:body))
being empty.

Moreover, in copy_subject, '\n' is replaced by space, but '\r' is untouched.

This behaviour is a regression for git branch --verbose, which bisects down
to 949af06 (branch: use ref-filter printing APIs, 2017-01-10).

The first patch hardens the code in ref-filter.c to handle these messages
more gracefully and adds a test script to check that the ref-filter and
pretty APIs deal correctly with CRLF messages.

The second patch adds tests that check the behaviour of git log andgit show 
in the presence of CRLF in messages, to prevent futur regressions.

Cc: Michael J Gruber git@grubix.eu [git@grubix.eu], Matthieu Moy 
git@matthieu-moy.fr [git@matthieu-moy.fr], John Keeping john@keeping.me.uk
[john@keeping.me.uk], Karthik Nayak karthik.188@gmail.com
[karthik.188@gmail.com], Jeff King peff@peff.net [peff@peff.net], Alex
Henrie alexhenrie24@gmail.com [alexhenrie24@gmail.com]cc: Eric Sunshine 
sunshine@sunshineco.com [sunshine@sunshineco.com]cc: Junio C Hamano 
gitster@pobox.com [gitster@pobox.com]

Philippe Blain (2):
  ref-filter: handle CRLF at end-of-line more gracefully
  log, show: add tests for messages containing CRLF

 ref-filter.c             |  36 ++++++-----
 t/t3920-crlf-messages.sh | 126 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 14 deletions(-)
 create mode 100755 t/t3920-crlf-messages.sh


base-commit: a5fa49ff0a8f3252c6bff49f92b85e7683868f8a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-576%2Fphil-blain%2Ffix-branch-verbose-crlf-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-576/phil-blain/fix-branch-verbose-crlf-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/576

Range-diff vs v4:

 1:  03b2d7d78a ! 1:  06231c315f ref-filter: handle CRLF at end-of-line more gracefully
     @@ Commit message
          This impacts the output of `git branch`, `git tag` and `git
          for-each-ref`.
      
     -    This bug is a regression for `git branch --verbose`, which
     +    This behaviour is a regression for `git branch --verbose`, which
          bisects down to 949af0684c (branch: use ref-filter printing APIs,
          2017-01-10).
      
     -    Fix this bug in ref-filter by hardening the logic in `copy_subject` and
     -    `find_subpos` to correctly parse messages containing CRLF.
     +    Adjust the ref-filter code to be more lenient by hardening the logic in
     +    `copy_subject` and `find_subpos` to correctly parse messages containing
     +    CRLF.
      
          Add a new test script, 't3920-crlf-messages.sh', to test the behaviour
          of commands using either the ref-filter or the pretty APIs with messages
 2:  75a87887be ! 2:  f536fee695 log, show: add tests for messages containing CRLF
     @@ Metadata
       ## Commit message ##
          log, show: add tests for messages containing CRLF
      
     -    A previous commit fixed a bug in ref-filter.c causing messages
     -    containing CRLF to be incorrectly parsed and displayed.
     +    A previous commit adjusted the code in ref-filter.c so that messages
     +    containing CRLF are now correctly parsed and displayed.
      
          Add tests to also check that `git log` and `git show` correctly handle
          such messages, to prevent futur regressions if these commands are
     @@ t/t3920-crlf-messages.sh: test_crlf_subject_body_and_contents tag --list tag-crl
      +		cut -d" " -f2- <tmp-branch >actual-branch &&
      +		cut -d" " -f2- <tmp-tag >actual-tag &&
      +		test_cmp expect actual-branch &&
     -+		test_cmp expect actual-tag
     ++		test_cmp expect actual-tag || return 1
      +	done
      +'
      +

-- 
gitgitgadget

  parent reply	other threads:[~2020-10-29 12:48 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-08 18:29 [PATCH 0/3] Teach ref-filter API to correctly handle CRLF in messages Philippe Blain via GitGitGadget
2020-03-08 18:29 ` [PATCH 1/3] t: add lib-crlf-messages.sh for messages containing CRLF Philippe Blain via GitGitGadget
2020-03-08 18:29 ` [PATCH 2/3] ref-filter: teach the API to correctly handle CRLF Philippe Blain via GitGitGadget
2020-03-08 18:29 ` [PATCH 3/3] log: add tests for messages containing CRLF to t4202 Philippe Blain via GitGitGadget
2020-03-09 15:14 ` [PATCH 0/3] Teach ref-filter API to correctly handle CRLF in messages Junio C Hamano
2020-03-10  2:19   ` Philippe Blain
2020-03-10  2:24 ` [PATCH v2 " Philippe Blain via GitGitGadget
2020-03-10  2:24   ` [PATCH v2 1/3] t: add lib-crlf-messages.sh for messages containing CRLF Philippe Blain via GitGitGadget
2020-03-10  2:24   ` [PATCH v2 2/3] ref-filter: fix the API to correctly handle CRLF Philippe Blain via GitGitGadget
2020-03-10 17:50     ` Junio C Hamano
2020-03-10  2:24   ` [PATCH v2 3/3] log: add tests for messages containing CRLF to t4202 Philippe Blain via GitGitGadget
2020-03-10  3:31   ` [PATCH v2 0/3] Teach ref-filter API to correctly handle CRLF in messages Junio C Hamano
2020-03-10 17:24   ` Junio C Hamano
2020-10-12 18:09   ` [PATCH v3 0/3] ref-filter: handle CRLF at end-of-line more gracefully Philippe Blain via GitGitGadget
2020-10-12 18:09     ` [PATCH v3 1/3] t: add lib-crlf-messages.sh for messages containing CRLF Philippe Blain via GitGitGadget
2020-10-12 22:22       ` Junio C Hamano
2020-10-14 13:22         ` Philippe Blain
2020-10-12 22:47       ` Eric Sunshine
2020-10-14 13:20         ` Philippe Blain
2020-10-14 13:45           ` Eric Sunshine
2020-10-14 13:52             ` Philippe Blain
2020-10-14 23:01               ` Eric Sunshine
2020-10-22  3:09         ` Philippe Blain
2020-10-12 18:09     ` [PATCH v3 2/3] ref-filter: handle CRLF at end-of-line more gracefully Philippe Blain via GitGitGadget
2020-10-12 22:24       ` Junio C Hamano
2020-10-14 13:09         ` Philippe Blain
2020-10-12 18:09     ` [PATCH v3 3/3] log, show: add tests for messages containing CRLF Philippe Blain via GitGitGadget
2020-10-22  3:01     ` [PATCH v4 0/2] ref-filter: handle CRLF at end-of-line more gracefully Philippe Blain via GitGitGadget
2020-10-22  3:01       ` [PATCH v4 1/2] " Philippe Blain via GitGitGadget
2020-10-23  0:52         ` Junio C Hamano
2020-10-23  1:46           ` Philippe Blain
2020-10-28 20:24             ` Junio C Hamano
2020-10-29  1:29               ` Philippe Blain
2020-10-22  3:01       ` [PATCH v4 2/2] log, show: add tests for messages containing CRLF Philippe Blain via GitGitGadget
2020-10-22 19:24         ` Philippe Blain
2020-10-29 12:48       ` Philippe Blain via GitGitGadget [this message]
2020-10-29 12:48         ` [PATCH v5 1/2] ref-filter: handle CRLF at end-of-line more gracefully Philippe Blain via GitGitGadget
2020-10-29 12:48         ` [PATCH v5 2/2] log, show: add tests for messages containing CRLF Philippe Blain via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.576.v5.git.1603975709.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).