[Bug] git branch -v has problems with carriage returns

13 messages, 6 authors, 2017-05-31 · open the first message on its own page

[Bug] git branch -v has problems with carriage returns

From: Animi Vulpis <hidden>
Date: 2017-05-16 23:22:58

Hi,

I upgraded to git v2.13.0 and since then git branch -v has problems
with carriage returns in subject lines.

We are using gitlab (not the newest version). So this bug (It's about
carriage returns in auto-generated merge messages (\r\n)) is not yet
fixed in our version:
https://gitlab.com/gitlab-org/gitlab-ce/issues/31671
That's were the carriage returns are coming from.

In my specific case the auto-generated merge message has three lines
with empty lines in between.
So every line ends with `\r\n\r\n`

If I do `git branch -v` with such a subject line somehow the third and
second line get combined before the hash. Example:

$ git branch -v
See merge request !XXXX temp space 84e18d22fd Merge branch
'feature-XXX' into 'develop'
# <begins with third line> <ending of seconds line (if longer than
third)> <commit hash (correct)> <subject line (correct)>

Before git v2.13.0 `git branch -v` worked completely normal.

I was not able to create a minimal local example, because my manually
created \r\n in commit messages were transformed into \n\n

Please let me know if I can provide any more information that would be helpful.

Cheers

Re: [Bug] git branch -v has problems with carriage returns

From: Atousa Duprat <hidden>
Date: 2017-05-19 21:55:28

Sorry for the noise with previous response...

I have tried to repro this issue but git goes out of its way to store
the commit messages using unix end-of-line format.
I think that git itself cannot create a repo exhibiting this problem.

Most helpful would be if you could create a mini repo using gitlab.
All it would need is one file, two branches, and a merge.
With that in hand, it should be pretty easy to track down the problem
and fix git.

You mentioned that the previous version you were using was working
fine, can you tell me which version that was?
It'll help to narrow down the changes that could have affected the issue.

Thanks,

Atousa

On Tue, May 16, 2017 at 4:22 PM, Animi Vulpis [off-list ref] wrote:
Hi,

I upgraded to git v2.13.0 and since then git branch -v has problems
with carriage returns in subject lines.

We are using gitlab (not the newest version). So this bug (It's about
carriage returns in auto-generated merge messages (\r\n)) is not yet
fixed in our version:
https://gitlab.com/gitlab-org/gitlab-ce/issues/31671
That's were the carriage returns are coming from.

In my specific case the auto-generated merge message has three lines
with empty lines in between.
So every line ends with `\r\n\r\n`

If I do `git branch -v` with such a subject line somehow the third and
second line get combined before the hash. Example:

$ git branch -v
See merge request !XXXX temp space 84e18d22fd Merge branch
'feature-XXX' into 'develop'
# <begins with third line> <ending of seconds line (if longer than
third)> <commit hash (correct)> <subject line (correct)>

Before git v2.13.0 `git branch -v` worked completely normal.

I was not able to create a minimal local example, because my manually
created \r\n in commit messages were transformed into \n\n

Please let me know if I can provide any more information that would be helpful.

Cheers

Re: [Bug] git branch -v has problems with carriage returns

From: Animi Vulpis <hidden>
Date: 2017-05-19 23:20:53

No problem, thanks for taking the time to help me.

I managed to create a minimal repository that shows the bug.
(I was able to deploy gitlab-ce-v8.15.8-ce.0 from docker locally and
create the repo, create the merge request and merge it)

I created a github repository so everybody interested can use it:
https://github.com/AnimiVulpis/git-bug
A few additional informations are in the README.md inside the repository.

FYI: I also tried a lot of things to create commit messages with \r\n
but without success. git does a good job preventing this.

Based on the history of the homebrew git formula
(https://github.com/Homebrew/homebrew-core/commits/master/Formula/git.rb)
and the fact that I `brew udpate` at least once a week I am pretty
sure that this bug does not exist in
git v2.12.2

Hope that helps
Have a nice weekend
David

2017-05-19 23:55 GMT+02:00 Atousa Duprat [off-list ref]:
Sorry for the noise with previous response...

I have tried to repro this issue but git goes out of its way to store
the commit messages using unix end-of-line format.
I think that git itself cannot create a repo exhibiting this problem.

Most helpful would be if you could create a mini repo using gitlab.
All it would need is one file, two branches, and a merge.
With that in hand, it should be pretty easy to track down the problem
and fix git.

You mentioned that the previous version you were using was working
fine, can you tell me which version that was?
It'll help to narrow down the changes that could have affected the issue.

Thanks,

Atousa

On Tue, May 16, 2017 at 4:22 PM, Animi Vulpis [off-list ref] wrote:
quoted
Hi,

I upgraded to git v2.13.0 and since then git branch -v has problems
with carriage returns in subject lines.

We are using gitlab (not the newest version). So this bug (It's about
carriage returns in auto-generated merge messages (\r\n)) is not yet
fixed in our version:
https://gitlab.com/gitlab-org/gitlab-ce/issues/31671
That's were the carriage returns are coming from.

In my specific case the auto-generated merge message has three lines
with empty lines in between.
So every line ends with `\r\n\r\n`

If I do `git branch -v` with such a subject line somehow the third and
second line get combined before the hash. Example:

$ git branch -v
See merge request !XXXX temp space 84e18d22fd Merge branch
'feature-XXX' into 'develop'
# <begins with third line> <ending of seconds line (if longer than
third)> <commit hash (correct)> <subject line (correct)>

Before git v2.13.0 `git branch -v` worked completely normal.

I was not able to create a minimal local example, because my manually
created \r\n in commit messages were transformed into \n\n

Please let me know if I can provide any more information that would be helpful.

Cheers

Re: [Bug] git branch -v has problems with carriage returns

From: Johannes Sixt <hidden>
Date: 2017-05-20 06:48:53

Am 19.05.2017 um 23:55 schrieb Atousa Duprat:
I have tried to repro this issue but git goes out of its way to store
the commit messages using unix end-of-line format.
I think that git itself cannot create a repo exhibiting this problem.
Here is a recipe to reproduce the error:

   git init
   git commit --allow-empty -m initial
   git branch crlf $(printf '%s\r\n' subject '' line3_long line4 |
        git commit-tree HEAD:)

The reason for the "bug" is obviously that a line having CR in addition 
to LF is not "an empty line". Consequently, the second line is not 
treated as a separator between subject and body, whereupon Git 
concatenates all lines into one large subject line. This strips the LFs 
but leaves the CRs in tact, which, when printed on a terminal move the 
cursor to the beginning of the line, so that text after the CRs 
overwrites what is already in the terminal.

This is just to give you a head start. I'm not going to look into this.

-- Hannes
quoted
If I do `git branch -v` with such a subject line somehow the third and
second line get combined before the hash. Example:

$ git branch -v
See merge request !XXXX temp space 84e18d22fd Merge branch
'feature-XXX' into 'develop'
# <begins with third line> <ending of seconds line (if longer than
third)> <commit hash (correct)> <subject line (correct)>

Before git v2.13.0 `git branch -v` worked completely normal.

[PATCH] ref-filter: treat CRLF as same as LF in find_subpos

From: DOAN Tran Cong Danh <hidden>
Date: 2017-05-21 13:42:37

Starting from commit 949af06 (branch: use ref-filter printing APIs, 2017-01-10),
`git branch -v` doesn't treat CRLF as line separator anymore.

Quote from git mailing-list:
Here is a recipe to reproduce the error:

   git init
   git commit --allow-empty -m initial
   git branch crlf $(printf '%s\r\n' subject '' line3_long line4 |
       git commit-tree HEAD:)
The reason for the "bug" is obviously that a line having CR in addition
to LF is not "an empty line". Consequently, the second line is not
treated as a separator between subject and body, whereupon Git
concatenates all line into one large subject line. This strips the LFs
but leaves the CRS in tact, which, when printed on a terminal move the
cursor to the beginning of the line, so that text after the CRs
overwrites what is already in the terminal.
Reported-by: Animi Vulpis <redacted>
Helped-by: Johannes Sixt [off-list ref]
Signed-off-by: DOAN Tran Cong Danh <redacted>
---
 ref-filter.c             | 19 +++++++++++++++----
 t/t3203-branch-output.sh |  3 ++-
 2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1fc5e9970..b3c2276a5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -967,7 +967,8 @@ static void find_subpos(const char *buf, unsigned long sz,
 	/* subject is first non-empty line */
 	*sub = buf;
 	/* subject goes to first empty line */
-	while (buf < *sig && *buf && *buf != '\n') {
+	while (buf < *sig && *buf && *buf != '\n'
+	       && !(*buf == '\r' && *(buf + 1) == '\n')) {
 		eol = strchrnul(buf, '\n');
 		if (*eol)
 			eol++;
@@ -975,12 +976,22 @@ static void find_subpos(const char *buf, unsigned long sz,
 	}
 	*sublen = buf - *sub;
 	/* drop trailing newline, if present */
-	if (*sublen && (*sub)[*sublen - 1] == '\n')
+	if (*sublen && (*sub)[*sublen - 1] == '\n') {
 		*sublen -= 1;
+		/* also drop trailing CR before that LF */
+		if ((*sublen) && (*sub)[*sublen - 1] == '\r')
+			*sublen -= 1;
+	}
 
 	/* skip any empty lines */
-	while (*buf == '\n')
-		buf++;
+	while (1) {
+		if (*buf == '\n')
+			buf++;
+		else if (*buf == '\r' && *(buf + 1) == '\n')
+			buf += 2;
+		else
+			break;
+	}
 	*body = buf;
 	*bodylen = strlen(buf);
 	*nonsiglen = *sig - buf;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 5778c0afe..29b392066 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -13,7 +13,8 @@ test_expect_success 'make commits' '
 
 test_expect_success 'make branches' '
 	git branch branch-one &&
-	git branch branch-two HEAD^
+	git branch branch-two $(printf "%s\r\n" one "" line3_long line4 |
+	     git commit-tree HEAD:)
 '
 
 test_expect_success 'make remote branches' '
-- 
2.13.0.67.g10c78a1

[PATCH] ref-filter: treat CRLF as same as LF in find_subpos

From: DOAN Tran Cong Danh <hidden>
Date: 2017-05-21 14:11:30

Starting from commit 949af06 (branch: use ref-filter printing APIs, 2017-01-10),
`git branch -v` doesn't treat CRLF as line separator anymore.

Quote from git mailing-list:
Here is a recipe to reproduce the error:

   git init
   git commit --allow-empty -m initial
   git branch crlf $(printf '%s\r\n' subject '' line3_long line4 |
       git commit-tree HEAD:)
The reason for the "bug" is obviously that a line having CR in addition
to LF is not "an empty line". Consequently, the second line is not
treated as a separator between subject and body, whereupon Git
concatenates all line into one large subject line. This strips the LFs
but leaves the CRS in tact, which, when printed on a terminal move the
cursor to the beginning of the line, so that text after the CRs
overwrites what is already in the terminal.
Reported-by: Animi Vulpis <redacted>
Helped-by: Johannes Sixt [off-list ref]
Signed-off-by: DOAN Tran Cong Danh <redacted>
---
 ref-filter.c             | 19 +++++++++++++++----
 t/t3203-branch-output.sh |  3 ++-
 2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1fc5e9970..b3c2276a5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -967,7 +967,8 @@ static void find_subpos(const char *buf, unsigned long sz,
 	/* subject is first non-empty line */
 	*sub = buf;
 	/* subject goes to first empty line */
-	while (buf < *sig && *buf && *buf != '\n') {
+	while (buf < *sig && *buf && *buf != '\n'
+	       && !(*buf == '\r' && *(buf + 1) == '\n')) {
 		eol = strchrnul(buf, '\n');
 		if (*eol)
 			eol++;
@@ -975,12 +976,22 @@ static void find_subpos(const char *buf, unsigned long sz,
 	}
 	*sublen = buf - *sub;
 	/* drop trailing newline, if present */
-	if (*sublen && (*sub)[*sublen - 1] == '\n')
+	if (*sublen && (*sub)[*sublen - 1] == '\n') {
 		*sublen -= 1;
+		/* also drop trailing CR before that LF */
+		if ((*sublen) && (*sub)[*sublen - 1] == '\r')
+			*sublen -= 1;
+	}
 
 	/* skip any empty lines */
-	while (*buf == '\n')
-		buf++;
+	while (1) {
+		if (*buf == '\n')
+			buf++;
+		else if (*buf == '\r' && *(buf + 1) == '\n')
+			buf += 2;
+		else
+			break;
+	}
 	*body = buf;
 	*bodylen = strlen(buf);
 	*nonsiglen = *sig - buf;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 5778c0afe..29b392066 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -13,7 +13,8 @@ test_expect_success 'make commits' '
 
 test_expect_success 'make branches' '
 	git branch branch-one &&
-	git branch branch-two HEAD^
+	git branch branch-two $(printf "%s\r\n" one "" line3_long line4 |
+	     git commit-tree HEAD:)
 '
 
 test_expect_success 'make remote branches' '
-- 
2.13.0.67.g10c78a1

[PATCH v2] ref-filter: trim end whitespace in subject

From: DOAN Tran Cong Danh <hidden>
Date: 2017-05-22 14:58:19

From: ĐOÀN Trần Công Danh <redacted>

Commit 949af0684 ("branch: use ref-filter printing APIs", 2017-01-10)
make `git branch -v` stops trimming end-whitespace in subject,
and it stops treating next all-whitespace-line as an empty line.

Quote from git mailing-list:
Here is a recipe to reproduce the error:

   git init
   git commit --allow-empty -m initial
   git branch crlf $(printf '%s\r\n' subject '' line3_long line4 |
       git commit-tree HEAD:)
The reason for the "bug" is obviously that a line having CR in addition
to LF is not "an empty line". Consequently, the second line is not
treated as a separator between subject and body, whereupon Git
concatenates all line into one large subject line. This strips the LFs
but leaves the CRS in tact, which, when printed on a terminal move the
cursor to the beginning of the line, so that text after the CRs
overwrites what is already in the terminal.
To recover previous behavior, trim all whitespace at the end of
first line, and treat all-white-space line as empty line

Reported-by: Animi Vulpis <redacted>
Helped-by: Johannes Sixt [off-list ref]
Signed-off-by: ĐOÀN Trần Công Danh <redacted>
---
 ref-filter.c             | 21 +++++++++++++++++----
 t/t3203-branch-output.sh |  3 ++-
 2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1fc5e9970..3625d543c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -942,6 +942,17 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	}
 }
 
+/*
+ * check if line in range [start, end) is a blank line or not
+ * data in range [start, end) must be valid before calling this function
+ */
+static int is_blank_line(const char *start, const char *end)
+{
+	while (start != end && isspace(*start))
+		++start;
+	return start == end;
+}
+
 static void find_subpos(const char *buf, unsigned long sz,
 			const char **sub, unsigned long *sublen,
 			const char **body, unsigned long *bodylen,
@@ -967,19 +978,21 @@ static void find_subpos(const char *buf, unsigned long sz,
 	/* subject is first non-empty line */
 	*sub = buf;
 	/* subject goes to first empty line */
-	while (buf < *sig && *buf && *buf != '\n') {
+	while (buf < *sig) {
 		eol = strchrnul(buf, '\n');
 		if (*eol)
 			eol++;
+		if (is_blank_line(buf, eol))
+			break;
 		buf = eol;
 	}
 	*sublen = buf - *sub;
-	/* drop trailing newline, if present */
-	if (*sublen && (*sub)[*sublen - 1] == '\n')
+	/* drop trailing whitespace, if present */
+	while (*sublen && isspace((*sub)[*sublen - 1]))
 		*sublen -= 1;
 
 	/* skip any empty lines */
-	while (*buf == '\n')
+	while (isspace(*buf))
 		buf++;
 	*body = buf;
 	*bodylen = strlen(buf);
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 5778c0afe..fa4441868 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -13,7 +13,8 @@ test_expect_success 'make commits' '
 
 test_expect_success 'make branches' '
 	git branch branch-one &&
-	git branch branch-two HEAD^
+	git branch branch-two $(printf "%s\r\n" one " " line3_long line4 |
+	     git commit-tree HEAD:)
 '
 
 test_expect_success 'make remote branches' '
-- 
2.13.0.67.g10c78a1

[PATCH v3] ref-filter: trim end whitespace in subject

From: DOAN Tran Cong Danh <hidden>
Date: 2017-05-22 17:10:39

From: ĐOÀN Trần Công Danh <redacted>

Commit 949af0684 ("branch: use ref-filter printing APIs", 2017-01-10)
make `git branch -v` stops trimming end-whitespace in subject,
and it stops treating next all-whitespace-line as an empty line.

Quote from git mailing-list:
Here is a recipe to reproduce the error:

   git init
   git commit --allow-empty -m initial
   git branch crlf $(printf '%s\r\n' subject '' line3_long line4 |
       git commit-tree HEAD:)
The reason for the "bug" is obviously that a line having CR in addition
to LF is not "an empty line". Consequently, the second line is not
treated as a separator between subject and body, whereupon Git
concatenates all line into one large subject line. This strips the LFs
but leaves the CRS in tact, which, when printed on a terminal move the
cursor to the beginning of the line, so that text after the CRs
overwrites what is already in the terminal.
To recover previous behavior, trim all whitespace at the end of
first line, and treat all-white-space line as empty line

Reported-by: Animi Vulpis <redacted>
Helped-by: Johannes Sixt [off-list ref]
Signed-off-by: ĐOÀN Trần Công Danh <redacted>
---
Sorry for the noise, after sending out v2,
I found that the body is calculated incorrectly.

 ref-filter.c             | 40 ++++++++++++++++++++++++++++++++--------
 t/t3203-branch-output.sh |  3 ++-
 2 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1fc5e9970..4b30edf61 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -942,6 +942,25 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	}
 }
 
+/*
+ * check if line in range [start, end) is a blank line or not
+ * data in range [start, end) must be valid before calling this function
+ */
+static int is_blank_line(const char *start, const char *end)
+{
+	while (start != end && isspace(*start))
+		start++;
+	return start == end;
+}
+
+static const char* find_next_eol(const char *buf)
+{
+	const char* eol = strchrnul(buf, '\n');
+	if (*eol)
+		eol++;
+	return eol;
+}
+
 static void find_subpos(const char *buf, unsigned long sz,
 			const char **sub, unsigned long *sublen,
 			const char **body, unsigned long *bodylen,
@@ -949,6 +968,7 @@ static void find_subpos(const char *buf, unsigned long sz,
 			const char **sig, unsigned long *siglen)
 {
 	const char *eol;
+	int has_empty_line = 0;
 	/* skip past header until we hit empty line */
 	while (*buf && *buf != '\n') {
 		eol = strchrnul(buf, '\n');
@@ -967,20 +987,24 @@ static void find_subpos(const char *buf, unsigned long sz,
 	/* subject is first non-empty line */
 	*sub = buf;
 	/* subject goes to first empty line */
-	while (buf < *sig && *buf && *buf != '\n') {
-		eol = strchrnul(buf, '\n');
-		if (*eol)
-			eol++;
+	while (buf < *sig && !has_empty_line) {
+		eol = find_next_eol(buf);
+		has_empty_line = is_blank_line(buf, eol);
 		buf = eol;
 	}
 	*sublen = buf - *sub;
-	/* drop trailing newline, if present */
-	if (*sublen && (*sub)[*sublen - 1] == '\n')
+	/* drop trailing whitespace, if present */
+	while (*sublen && isspace((*sub)[*sublen - 1]))
 		*sublen -= 1;
 
 	/* skip any empty lines */
-	while (*buf == '\n')
-		buf++;
+	while (buf < *sig) {
+		eol = find_next_eol(buf);
+		if (is_blank_line(buf, eol))
+			buf = eol;
+		else
+			break;
+	}
 	*body = buf;
 	*bodylen = strlen(buf);
 	*nonsiglen = *sig - buf;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 5778c0afe..fa4441868 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -13,7 +13,8 @@ test_expect_success 'make commits' '
 
 test_expect_success 'make branches' '
 	git branch branch-one &&
-	git branch branch-two HEAD^
+	git branch branch-two $(printf "%s\r\n" one " " line3_long line4 |
+	     git commit-tree HEAD:)
 '
 
 test_expect_success 'make remote branches' '
-- 
Danh

Re: [PATCH v3] ref-filter: trim end whitespace in subject

From: Johannes Sixt <hidden>
Date: 2017-05-22 19:48:11

Am 22.05.2017 um 19:10 schrieb DOAN Tran Cong Danh:
quoted hunk
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 5778c0afe..fa4441868 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -13,7 +13,8 @@ test_expect_success 'make commits' '
  
  test_expect_success 'make branches' '
  	git branch branch-one &&
-	git branch branch-two HEAD^
+	git branch branch-two $(printf "%s\r\n" one " " line3_long line4 |
+	     git commit-tree HEAD:)
  '
  
  test_expect_success 'make remote branches' '
This updated test shows nothing, I am afraid: If I apply only this 
change without the rest of the patch, then all test in t3203 still pass. 
And I do not see how the code change could make any difference at all. 
What am I missing?

-- Hannes

Re: [PATCH v3] ref-filter: trim end whitespace in subject

From: Jeff King <hidden>
Date: 2017-05-22 19:53:56

On Mon, May 22, 2017 at 09:47:59PM +0200, Johannes Sixt wrote:
Am 22.05.2017 um 19:10 schrieb DOAN Tran Cong Danh:
quoted
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 5778c0afe..fa4441868 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -13,7 +13,8 @@ test_expect_success 'make commits' '
  test_expect_success 'make branches' '
  	git branch branch-one &&
-	git branch branch-two HEAD^
+	git branch branch-two $(printf "%s\r\n" one " " line3_long line4 |
+	     git commit-tree HEAD:)
  '
  test_expect_success 'make remote branches' '
This updated test shows nothing, I am afraid: If I apply only this change
without the rest of the patch, then all test in t3203 still pass. And I do
not see how the code change could make any difference at all. What am I
missing?
It does for me here on Linux; I wonder if the CRs are being eaten by the
shell expansion.

-Peff

Re: [PATCH v3] ref-filter: trim end whitespace in subject

From: Johannes Sixt <hidden>
Date: 2017-05-22 20:19:07

Am 22.05.2017 um 21:53 schrieb Jeff King:
On Mon, May 22, 2017 at 09:47:59PM +0200, Johannes Sixt wrote:
quoted
Am 22.05.2017 um 19:10 schrieb DOAN Tran Cong Danh:
quoted
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 5778c0afe..fa4441868 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -13,7 +13,8 @@ test_expect_success 'make commits' '
   test_expect_success 'make branches' '
   	git branch branch-one &&
-	git branch branch-two HEAD^
+	git branch branch-two $(printf "%s\r\n" one " " line3_long line4 |
I didn't notice earlier that there is a blank between the dq here.
quoted
quoted
+	     git commit-tree HEAD:)
   '
   test_expect_success 'make remote branches' '
This updated test shows nothing, I am afraid: If I apply only this change
without the rest of the patch, then all test in t3203 still pass. And I do
not see how the code change could make any difference at all. What am I
missing?
And I didn't look carefully enough at t3203. Some tests do check branch 
-v output.
It does for me here on Linux; I wonder if the CRs are being eaten by the
shell expansion.
And I tested on Linux, too, but on the wrong branch. On a branch closer 
to master I see a failure as well. Sorry for the noise.

There are no CRs on the command line, BTW, only on stdin of commit-tree.

-- Hannes

Re: [Bug] git branch -v has problems with carriage returns

From: Atousa Duprat <hidden>
Date: 2017-05-31 05:32:59

Here is my first attempt at fixing the issue.

There are two problems in ref-filter.c:

First, copy_subject() has been modified to turn '\n' into a space and
every other ascii control character to be ignored.

Second, find_subpos() doesn't realize that a line that only contains a
'\r\n' is a blank line – at least when using crlf convention.
I have changed things so that a sequence of either '\n' or "\r\n"
separate the subject from the body of the commit message.
I am not looking at the crlf setting because it doesn't seem like a
useful distinction – when one would we ever care for \r\n not to be a
blank line?  But it could be done...

Both fixes are minimal, but it feels like they are a issues with the
specific encoding.  Does git mandate ascii or utf-8 commit messages?
If not, there may be a larger issue here with encodings and line-end
conventions at the very least in ref-filter.c
Guidance would be appreciated for how to deal with this issue...

Patch attached.


Atousa


On Fri, May 19, 2017 at 11:48 PM, Johannes Sixt [off-list ref] wrote:
Am 19.05.2017 um 23:55 schrieb Atousa Duprat:
quoted
I have tried to repro this issue but git goes out of its way to store
the commit messages using unix end-of-line format.
I think that git itself cannot create a repo exhibiting this problem.

Here is a recipe to reproduce the error:

  git init
  git commit --allow-empty -m initial
  git branch crlf $(printf '%s\r\n' subject '' line3_long line4 |
       git commit-tree HEAD:)

The reason for the "bug" is obviously that a line having CR in addition to
LF is not "an empty line". Consequently, the second line is not treated as a
separator between subject and body, whereupon Git concatenates all lines
into one large subject line. This strips the LFs but leaves the CRs in tact,
which, when printed on a terminal move the cursor to the beginning of the
line, so that text after the CRs overwrites what is already in the terminal.

This is just to give you a head start. I'm not going to look into this.

-- Hannes

quoted
quoted
If I do `git branch -v` with such a subject line somehow the third and
second line get combined before the hash. Example:

$ git branch -v
See merge request !XXXX temp space 84e18d22fd Merge branch
'feature-XXX' into 'develop'
# <begins with third line> <ending of seconds line (if longer than
third)> <commit hash (correct)> <subject line (correct)>

Before git v2.13.0 `git branch -v` worked completely normal.

Re: [Bug] git branch -v has problems with carriage returns

From: Stefan Beller <hidden>
Date: 2017-05-31 20:58:24

On Tue, May 30, 2017 at 10:32 PM, Atousa Duprat [off-list ref] wrote:
Here is my first attempt at fixing the issue.
Cool you're looking into this. :)
There are two problems in ref-filter.c:

First, copy_subject() has been modified to turn '\n' into a space and
every other ascii control character to be ignored.

Second, find_subpos() doesn't realize that a line that only contains a
'\r\n' is a blank line – at least when using crlf convention.
I have changed things so that a sequence of either '\n' or "\r\n"
separate the subject from the body of the commit message.
I am not looking at the crlf setting because it doesn't seem like a
useful distinction – when one would we ever care for \r\n not to be a
blank line?  But it could be done...

Both fixes are minimal, but it feels like they are a issues with the
specific encoding.  Does git mandate ascii or utf-8 commit messages?
If not, there may be a larger issue here with encodings and line-end
conventions at the very least in ref-filter.c
Guidance would be appreciated for how to deal with this issue...

Patch attached.
Please read Documentation/SubmittingPatches
(tl;dr:
(a) please sign your patch, read https://developercertificate.org/
(b) if possible please send patches inline instead of attached)
quoted hunk
diff --git a/ref-filter.c b/ref-filter.c
index 3a640448f..bc573f481 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -836,11 +836,15 @@ static const char *copy_email(const char *buf)
 static char *copy_subject(const char *buf, unsigned long len)
 {
         char *r = xmemdupz(buf, len);
-        int i;
+        int i, j;

-        for (i = 0; i < len; i++)
+        for (i = 0, j = 0; i < len; i++, j++)
                 if (r[i] == '\n')
-                        r[i] = ' ';
+                        r[j] = ' ';
+                else if (r[i] < 32)
+                    j--; // skip ascii control characters that are not '\n'
/*
 * Our comment style uses the other way,
 * as it is compatible with more compilers, still.
 */

This seems to solve a different problem than the carriage return
discussed? So it could go into a separate patch.

quoted hunk
+                else r[j] = r[i];
+        r[j]=0;

         return r;
 }
@@ -956,9 +960,12 @@ static void find_subpos(const char *buf, unsigned long sz,
                         eol++;
                 buf = eol;
         }
+
stray new line?
         /* skip any empty lines */
         while (*buf == '\n')
                 buf++;
+        while (*buf == '\r' && *(buf+1) == '\n')
+                buf += 2;
This first skips LF empty lines and then skips CRLF empty
lines. What if they are mixed? I'd think if we extend the
empty line detection we'd want to robust to such as well,
so maybe

    while (*buf == '\r' || *buf == '\n')
        buf++;

Maybe this is a bit too greedy?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help