Verbose Commit Ignore Line Fails via CRLF Line Endings

Verbose Commit Ignore Line Fails via CRLF Line Endings

[Spencer Fretwell]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

gc -v produces CRLF file which does not ignore verbose description.

OSX + git-annex (pre-commit hook disabled just in case)
using subl -w (sublime text 4) as editor, which indicated the change
in line endings
- committing as Unix fixes the issue (workaround, tedious)
- leaving as Windows is the issue

see attached bugreport

[-- Attachment #2: git-bugreport-2024-10-08-1220.txt --]
[-- Type: text/plain, Size: 1283 bytes --]

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

- gc -v

What did you expect to happen? (Expected behavior)

- line endings respect OS
- gc without -v uses Unix line endings
- line ending is visible in sublime (editor = subl -w)

What happened instead? (Actual behavior)

- gc -v uses CRLF (Windows) line endings
- as seen in sublime (editor = subl -w)

What's different between what you expected and what actually happened?

- use of CRLF is leaving verbose output in commit message

Anything else you want to add:

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.46.2
cpu: arm64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
libcurl: 8.4.0
zlib: 1.2.12
uname: Darwin 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:04 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6020 arm64
compiler info: clang: 15.0.0 (clang-1500.3.9.4)
libc info: no libc information available
$SHELL (typically, interactive shell): /bin/zsh


[Enabled Hooks]
post-checkout
post-merge
post-receive

Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[Spencer Fretwell]

On second glance it might involve having a file being committed that
has CRLF endings in it.
Committing files with LF only does not produce issue.

I suspect git either needs to add support for a magic ignore line
ending with a CRLF,
or has to sanitize verbose output when that output has CRLF (less ideal)

Bug originally seen as early as
[2018](https://superuser.com/questions/1367811/sometimes-git-includes-the-diff-to-commit-message-when-using-verbose)

On Tue, Oct 8, 2024 at 12:30 PM Spencer Fretwell
<spencer.fretwell@gmail.com> wrote:
>
> gc -v produces CRLF file which does not ignore verbose description.
>
> OSX + git-annex (pre-commit hook disabled just in case)
> using subl -w (sublime text 4) as editor, which indicated the change
> in line endings
> - committing as Unix fixes the issue (workaround, tedious)
> - leaving as Windows is the issue
>
> see attached bugreport

Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1884 bytes --]

On 2024-10-08 at 18:30:53, Spencer Fretwell wrote:
> gc -v produces CRLF file which does not ignore verbose description.

I'm not sure what software you're suggesting here, but we don't
distribute any software called "gc".  I can't imagine `git gc` is
affecting anything here.  Is this maybe an alias for `git commit` or
something else?  (I'm going to assume so below.)  If so, do you see the
problem with a plain `git commit -v`?

> OSX + git-annex (pre-commit hook disabled just in case)
> using subl -w (sublime text 4) as editor, which indicated the change
> in line endings
> - committing as Unix fixes the issue (workaround, tedious)
> - leaving as Windows is the issue
> 
> see attached bugreport

> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
> 
> What did you do before the bug happened? (Steps to reproduce your issue)
> 
> - gc -v
> 
> What did you expect to happen? (Expected behavior)
> 
> - line endings respect OS
> - gc without -v uses Unix line endings
> - line ending is visible in sublime (editor = subl -w)

Does this happen if you use a different editor as well, such as vi or
nano?  I don't see this on a Linux system with `git commit -v` using
Neovim.

Also, if you do see this problem when you open the editor, can you leave
the editor as it is (don't save or modify the file), and change into
another terminal window and use something like `xxd -g` or `od -tx1` to
see if `.git/COMMIT_EDITMSG` has CRLF pairs (that should show up as `0d
0a`) or just LF (plain `0a`)?

The reason I ask this is that I'm wondering if maybe there's a
configuration of your editor that's causing it to switch into a special
diff mode with `git commit -v` that it thinks should have CRLF endings.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

On 2024-10-08 at 18:37:47, Spencer Fretwell wrote:
> On second glance it might involve having a file being committed that
> has CRLF endings in it.
> Committing files with LF only does not produce issue.
> 
> I suspect git either needs to add support for a magic ignore line
> ending with a CRLF,
> or has to sanitize verbose output when that output has CRLF (less ideal)
> 
> Bug originally seen as early as
> [2018](https://superuser.com/questions/1367811/sometimes-git-includes-the-diff-to-commit-message-when-using-verbose)

Ah, yes, that would probably explain it.  Your editor is seeing some
lines that are CRLF and assuming that the entire file is CRLF, which it
should not.  In Vim and Neovim, you can do `setl ff=unix` when loading a
buffer for `git commit` that will force it to always use Unix line
endings, and there's probably some functionality in Sublime Text that
lets you force a particular line ending.

What might also help is that if these are intended to be text files, you
can set a `.gitattributes` file as outlined in the Git FAQ[0]:

  # By default, guess.
  *	text=auto
  # Mark all C files as text.
  *.c	text
  # Ensure all shell files have LF endings and all batch files have CRLF
  # endings in the working tree and both have LF in the repo.
  *.sh text eol=lf
  *.bat text eol=crlf
  # Mark all JPEG files as binary.
  *.jpg	binary

You can customize the file patterns as you see fit.  Once you do that
and run `git add --renormalize .` (which you should do in the same
commit as adding your `.gitattributes` file) and then commit, your text
files will be stored in the repository as LF even though they may have
CRLF endings in the working tree, and your diff output won't contain
CRLF.

[0] https://git-scm.com/docs/gitfaq#Documentation/gitfaq.txt#recommended-storage-settings
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[Spencer Fretwell]

Thanks Brian,

It appears sublime auto-normalizes endings to "whatever occurs most
frequently in the first 32kB". So, I guess it was witnessing the CRLF
from the verbose output and replacing all lines with CRLF. Thanks for
the reminder about --renormalize.

Is there any chance for git to support a CRLF magic ignore line,
particularly considering the variation in standard line ending across
different platforms? I tried autocrlf=input as well and it sadly
doesn't normalize the commit message file itself. Either way (magic
ignore with CRLF or normalizing line endings in the commit message),
would be appreciated for mixed line ending workflows (especially
considering WSL)

On Tue, Oct 8, 2024 at 1:09 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2024-10-08 at 18:37:47, Spencer Fretwell wrote:
> > On second glance it might involve having a file being committed that
> > has CRLF endings in it.
> > Committing files with LF only does not produce issue.
> >
> > I suspect git either needs to add support for a magic ignore line
> > ending with a CRLF,
> > or has to sanitize verbose output when that output has CRLF (less ideal)
> >
> > Bug originally seen as early as
> > [2018](https://superuser.com/questions/1367811/sometimes-git-includes-the-diff-to-commit-message-when-using-verbose)
>
> Ah, yes, that would probably explain it.  Your editor is seeing some
> lines that are CRLF and assuming that the entire file is CRLF, which it
> should not.  In Vim and Neovim, you can do `setl ff=unix` when loading a
> buffer for `git commit` that will force it to always use Unix line
> endings, and there's probably some functionality in Sublime Text that
> lets you force a particular line ending.
>
> What might also help is that if these are intended to be text files, you
> can set a `.gitattributes` file as outlined in the Git FAQ[0]:
>
>   # By default, guess.
>   *     text=auto
>   # Mark all C files as text.
>   *.c   text
>   # Ensure all shell files have LF endings and all batch files have CRLF
>   # endings in the working tree and both have LF in the repo.
>   *.sh text eol=lf
>   *.bat text eol=crlf
>   # Mark all JPEG files as binary.
>   *.jpg binary
>
> You can customize the file patterns as you see fit.  Once you do that
> and run `git add --renormalize .` (which you should do in the same
> commit as adding your `.gitattributes` file) and then commit, your text
> files will be stored in the repository as LF even though they may have
> CRLF endings in the working tree, and your diff output won't contain
> CRLF.
>
> [0] https://git-scm.com/docs/gitfaq#Documentation/gitfaq.txt#recommended-storage-settings
> --
> brian m. carlson (they/them or he/him)
> Toronto, Ontario, CA

Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

On 2024-10-08 at 19:34:46, Spencer Fretwell wrote:
> Thanks Brian,
> 
> It appears sublime auto-normalizes endings to "whatever occurs most
> frequently in the first 32kB". So, I guess it was witnessing the CRLF
> from the verbose output and replacing all lines with CRLF. Thanks for
> the reminder about --renormalize.
> 
> Is there any chance for git to support a CRLF magic ignore line,
> particularly considering the variation in standard line ending across
> different platforms? I tried autocrlf=input as well and it sadly
> doesn't normalize the commit message file itself. Either way (magic
> ignore with CRLF or normalizing line endings in the commit message),
> would be appreciated for mixed line ending workflows (especially
> considering WSL)

The answer is essentially that I don't know.  We typically make
decisions on whether we'll accept features when we see the patch.  My
guess is that, assuming someone (maybe you) sends a patch, it will
probably be accepted, since I wouldn't expect it would be very difficult
to do or have major impacts on the code.  It might, as with any patch,
take a couple of rounds, though.

I use Linux or rarely other Unix systems and always use LF endings, so I
don't plan to send a patch since this doesn't affect me, but assuming
the patch looked reasonable, I don't see myself having an objection to
it.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[Torsten Bögershausen]

On Tue, Oct 08, 2024 at 07:52:33PM +0000, brian m. carlson wrote:
> On 2024-10-08 at 19:34:46, Spencer Fretwell wrote:
> > Thanks Brian,
> >
> > It appears sublime auto-normalizes endings to "whatever occurs most
> > frequently in the first 32kB". So, I guess it was witnessing the CRLF
> > from the verbose output and replacing all lines with CRLF. Thanks for
> > the reminder about --renormalize.
> >
> > Is there any chance for git to support a CRLF magic ignore line,
> > particularly considering the variation in standard line ending across
> > different platforms? I tried autocrlf=input as well and it sadly
> > doesn't normalize the commit message file itself. Either way (magic
> > ignore with CRLF or normalizing line endings in the commit message),
> > would be appreciated for mixed line ending workflows (especially
> > considering WSL)
>
> The answer is essentially that I don't know.  We typically make
> decisions on whether we'll accept features when we see the patch.  My
> guess is that, assuming someone (maybe you) sends a patch, it will
> probably be accepted, since I wouldn't expect it would be very difficult
> to do or have major impacts on the code.  It might, as with any patch,
> take a couple of rounds, though.
>
> I use Linux or rarely other Unix systems and always use LF endings, so I
> don't plan to send a patch since this doesn't affect me, but assuming
> the patch looked reasonable, I don't see myself having an objection to
> it.

Hej Spencer,

you are probably the first one reporting this, thanks for doing so.

I have the suspicion, that your repo has files commit with CRLF,
and that leads to a CRLF entering the diff, and that leads your
editor to produce CRLF in the commit-message.

In order to debug this a little bit, can you run a
git ls-files --eol | grep i/crlf
in your repo ?

In general, when working with cross-platform, or only with windows,
it is a good practice to use a .gitattributes file to specify the
line endings.
Do you have such a file in your repo, and is it commited ?

Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[Torsten Bögershausen]

On Fri, Oct 11, 2024 at 02:33:46PM +0000, Spencer Fretwell wrote:

(Please try to avoid top-posting here on this list)

> On Fri Oct 11, 2024, 04:45 AM GMT, Torsten Bögershausen <mailto:tboegi@web.de> wrote:
> > On Tue, Oct 08, 2024 at 07:52:33PM +0000, brian m. carlson wrote:
> >> On 2024-10-08 at 19:34:46, Spencer Fretwell wrote:
> >> > Thanks Brian,
> >> >
> >> > It appears sublime auto-normalizes endings to "whatever occurs most
> >> > frequently in the first 32kB". So, I guess it was witnessing the CRLF
> >> > from the verbose output and replacing all lines with CRLF. Thanks for
> >> > the reminder about --renormalize.
> >> >
> >> > Is there any chance for git to support a CRLF magic ignore line,
> >> > particularly considering the variation in standard line ending across
> >> > different platforms? I tried autocrlf=input as well and it sadly
> >> > doesn't normalize the commit message file itself. Either way (magic
> >> > ignore with CRLF or normalizing line endings in the commit message),
> >> > would be appreciated for mixed line ending workflows (especially
> >> > considering WSL)
> >>
> >> The answer is essentially that I don't know. We typically make
> >> decisions on whether we'll accept features when we see the patch. My
> >> guess is that, assuming someone (maybe you) sends a patch, it will
> >> probably be accepted, since I wouldn't expect it would be very difficult
> >> to do or have major impacts on the code. It might, as with any patch,
> >> take a couple of rounds, though.
> >>
> >> I use Linux or rarely other Unix systems and always use LF endings, so I
> >> don't plan to send a patch since this doesn't affect me, but assuming
> >> the patch looked reasonable, I don't see myself having an objection to
> >> it.
> >
> > Hej Spencer,
> >
> > you are probably the first one reporting this, thanks for doing so.
> >
> > I have the suspicion, that your repo has files commit with CRLF,
> > and that leads to a CRLF entering the diff, and that leads your
> > editor to produce CRLF in the commit-message.
> >
> > In order to debug this a little bit, can you run a
> > git ls-files --eol | grep i/crlf
> > in your repo ?
> >
> > In general, when working with cross-platform, or only with windows,
> > it is a good practice to use a .gitattributes file to specify the
> > line endings.
> > Do you have such a file in your repo, and is it commited ?
> >
> >
> >
> Hi Torsten,
>
> To catch you up, yes the problem involved commiting a long CRLF file whose line endings predominated the commit message, so my editor assumed the endings should be CRLF for the message and normalized all lines including the magic ‘comment below’ line. This in turn appears to nullify the magic line so the patch itself saves to the commit message
>
> Would an attribute file LF the patch contents? I guess it should given the patch is what is comitted. The other solution would be for git to handle the CRLF magic line better, either sanitizing to LF during comment removal in the commit message or just by interpreting the magic line with CRLF as still a magic line. One is a process update the other a syntax update, but other than knowing that I am unfamiliar with the got codebase and this wary about attempting a patch…

I am not sure, if I understand your question fully correct.
As far as I know, Git does not manipulate the line endings
in the commit message. That is something that can be done.
However, it seems that 2 things come together at you side:
a) Files are commited with CRLF.
  That is not recommendet, I would say.
  Could you do me a favorite and run
  git ls-files --eol | grep i/crlf

b) Your editor makes the decision to convert all LF into CRLF
  That is, hm, fixable by either using a different editor
  or setting
  git config --global core.editor notepad (under windows).
  The "notepad" executable under Windows is a shell script,
  which converts the commit message into CRLF, invokes notepad.exe,
  and converts the commit message back into LF, before handing it
  to Git.
  That will do the trick as well.

If someone wants to send a patch to normalize the line endings
from the commit-message, I am happy to review it.

Re: Verbose Commit Ignore Line Fails via CRLF Line Endings

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

>> Is there any chance for git to support a CRLF magic ignore line,
>> particularly considering the variation in standard line ending across
>> different platforms? I tried autocrlf=input as well and it sadly
>> doesn't normalize the commit message file itself. Either way (magic
>> ignore with CRLF or normalizing line endings in the commit message),
>> would be appreciated for mixed line ending workflows (especially
>> considering WSL)
>
> The answer is essentially that I don't know.  We typically make
> decisions on whether we'll accept features when we see the patch.  My
> guess is that, assuming someone (maybe you) sends a patch, it will
> probably be accepted, since I wouldn't expect it would be very difficult
> to do or have major impacts on the code.  It might, as with any patch,
> take a couple of rounds, though.
>
> I use Linux or rarely other Unix systems and always use LF endings, so I
> don't plan to send a patch since this doesn't affect me, but assuming
> the patch looked reasonable, I don't see myself having an objection to
> it.

I do not offhand see a downside if we loosened the detection of the
cut line a bit.  What is happening on the Git end (I do not care how
Editor screws up and munges what we wrote before "Do not modify or
remove the line above"---to us, it looks like the user did not honor
that instruction) is:

wt-status.c defines cut_line[] to be

        static const char cut_line[] =
        "------------------------ >8 ------------------------\n";

IOW, we insist that after the dashes we have LF.

Then wt-status.c:wt_status_locate_end() does this:

        size_t wt_status_locate_end(const char *s, size_t len)
        {
                const char *p;
                struct strbuf pattern = STRBUF_INIT;

                strbuf_addf(&pattern, "\n%s %s", comment_line_str, cut_line);
                if (starts_with(s, pattern.buf + 1))
                        len = 0;
                else if ((p = strstr(s, pattern.buf))) {
			size_t newlen = p - s + 1;
			...

That is, if the buffer begins with the cut line (including the LF at
the end of the line, without any CR before it), we found it at the
beginning of the buffer at len==0, otherwise if we find a region of
memory that begins with LF, followed by the scissors, followed by LF
(again, without allowing CR before it), we have a match there.

That is reasonable as long as the user does not muck with the line
that we told them not to touch.  In this case, the editor is
corrupting the line without being instructed by the user so the user
cannot do much about it.

We could loosen the rule by doing something along the following line:

 - define the cut_line constant without LF at the end of the line.

 - teach wt_status_append_cut_line() to compensate for the lack of
   LF in cut_line.

 - teach wt_status_locate_end() to notice and ignore CR if it
   appears at the end of the cut_line.

I'll go offline at the end of this week, so I will stop here at
illustrating the above approach in the form of an untested patch
below.  I am sure I'd have many off by one errors in it, but it
should be sufficient to outline the idea.

 wt-status.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git c/wt-status.c w/wt-status.c
index 6a8c05d1cf..b54bc5de81 100644
--- c/wt-status.c
+++ w/wt-status.c
@@ -39,7 +39,7 @@
 #define UF_DELAY_WARNING_IN_MS (2 * 1000)
 
 static const char cut_line[] =
-"------------------------ >8 ------------------------\n";
+"------------------------ >8 ------------------------";
 
 static char default_wt_status_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */
@@ -1095,15 +1095,28 @@ static void wt_longstatus_print_other(struct wt_status *s,
 	status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 }
 
+static int at_eol(const char *s, size_t len, size_t loc)
+{
+	if (len <= loc)
+		return 0;
+	if (s[loc] == '\r')
+		loc++;
+	if (len <= loc)
+		return 0;
+	return s[loc] == '\n';
+}
+
 size_t wt_status_locate_end(const char *s, size_t len)
 {
 	const char *p;
 	struct strbuf pattern = STRBUF_INIT;
 
-	strbuf_addf(&pattern, "\n%s %s", comment_line_str, cut_line);
-	if (starts_with(s, pattern.buf + 1))
+	strbuf_addf(&pattern, "%s %s", comment_line_str, cut_line);
+	if (starts_with(s, pattern.buf) && at_eol(s, len, pattern.len))
 		len = 0;
-	else if ((p = strstr(s, pattern.buf))) {
+	else if ((p = strstr(s, pattern.buf)) &&
+		 (s < p && p[-1] == '\n') &&
+		 at_eol(p, p - s, pattern.len)) {
 		size_t newlen = p - s + 1;
 		if (newlen < len)
 			len = newlen;
@@ -1116,7 +1129,7 @@ void wt_status_append_cut_line(struct strbuf *buf)
 {
 	const char *explanation = _("Do not modify or remove the line above.\nEverything below it will be ignored.");
 
-	strbuf_commented_addf(buf, comment_line_str, "%s", cut_line);
+	strbuf_commented_addf(buf, comment_line_str, "%s\n", cut_line);
 	strbuf_add_commented_lines(buf, explanation, strlen(explanation), comment_line_str);
 }