threads / patch / 48612

v6, 2 partsblame: prevent error if range ends past end of file

Subject: [PATCH v6 1/2] blame: prevent error if range ends past end of file

## tl;dr

8 messages between May 31, 2018 and Jun 15, 2018. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

istephens@atlassian.com· May 31, 2018, 05:00 UTC · lore
From: Isabella Stephens <istephens@atlassian.com>

If the -L option is used to specify a line range in git blame, and the end of the range is past the end of the file, git will fail with a fatal error. This commit prevents such behavior - instead we display the blame for existing lines within the specified range. Tests are amended accordingly.

This commit also fixes two corner cases. Blaming -L n,-(n+1) now blames the first n lines of a file rather than from n to the end of the file. Blaming -L ,-n will be treated as -L 1,-n and blame the first line of the file, rather than blaming the whole file.

Signed-off-by: Isabella Stephens <istephens@atlassian.com>
---
 builtin/blame.c               |  4 ++--
 line-range.c                  |  2 +-
 t/t8003-blame-corner-cases.sh | 12 ++++++++----
 3 files changed, 11 insertions(+), 7 deletions(-)
Show changes to 3 files +11 −7

builtin/blame.c, line-range.c, t/t8003-blame-corner-cases.sh

diff --git a/builtin/blame.c b/builtin/blame.c
index 9dcb367b9..e1359b192 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -886,13 +886,13 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 				    nth_line_cb, &sb, lno, anchor,
 				    &bottom, &top, sb.path))
 			usage(blame_usage);
-		if (lno < top || ((lno || bottom) && lno < bottom))
+		if ((!lno && (top || bottom)) || lno < bottom)
 			die(Q_("file %s has only %lu line",
 			       "file %s has only %lu lines",
 			       lno), path, lno);
 		if (bottom < 1)
 			bottom = 1;
-		if (top < 1)
+		if (top < 1 || lno < top)
 			top = lno;
 		bottom--;
 		range_set_append_unsafe(&ranges, bottom, top);
diff --git a/line-range.c b/line-range.c
index 323399d16..232c3909e 100644
--- a/line-range.c
+++ b/line-range.c
@@ -47,7 +47,7 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
 			else if (!num)
 				*ret = begin;
 			else
-				*ret = begin + num;
+				*ret = begin + num > 0 ? begin + num : 1;
 			return term;
 		}
 		return spec;
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 661f9d430..c92a47b6d 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -216,14 +216,18 @@ test_expect_success 'blame -L with invalid start' '
 '
 
 test_expect_success 'blame -L with invalid end' '
-	test_must_fail git blame -L1,5 tres 2>errors &&
-	test_i18ngrep "has only 2 lines" errors
+	git blame -L1,5 tres >out &&
+	test_line_count = 2 out
 '
 
 test_expect_success 'blame parses <end> part of -L' '
 	git blame -L1,1 tres >out &&
-	cat out &&
-	test $(wc -l < out) -eq 1
+	test_line_count = 1 out
+'
+
+test_expect_success 'blame -Ln,-(n+1)' '
+	git blame -L3,-4 nine_lines >out &&
+	test_line_count = 3 out
 '
 
 test_expect_success 'indent of line numbers, nine lines' '

-- 
2.14.3 (Apple Git-98)
istephens@atlassian.com· May 31, 2018, 05:00 UTC · re: istephens@atlassian.com · lore

[PATCH v6 2/2] log: prevent error if line range ends past end of file

From: Isabella Stephens <istephens@atlassian.com>

If the -L option is used to specify a line range in git log, and the end of the range is past the end of the file, git will fail with a fatal error. This commit prevents such behaviour - instead we perform the log for existing lines within the specified range.

This commit also fixes a corner case where -L ,-n:file would be treated as a log over the whole file. Now we treat this as -L 1,-n:file and blame the first line of the file instead.

Signed-off-by: Isabella Stephens <istephens@atlassian.com>
---
 line-log.c          | 8 +++++---
 t/t4211-line-log.sh | 5 ++---
 2 files changed, 7 insertions(+), 6 deletions(-)
Show changes to 2 files +7 −6

line-log.c, t/t4211-line-log.sh

diff --git a/line-log.c b/line-log.c
index cdc2257db..4da40dec1 100644
--- a/line-log.c
+++ b/line-log.c
@@ -599,11 +599,13 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args)
 				    lines, anchor, &begin, &end,
 				    full_name))
 			die("malformed -L argument '%s'", range_part);
-		if (lines < end || ((lines || begin) && lines < begin))
-			die("file %s has only %lu lines", name_part, lines);
+		if ((!lines && (begin || end)) || lines < begin)
+			die(Q_("file %s has only %lu line",
+				   "file %s has only %lu lines",
+				   lines), name_part, lines);
 		if (begin < 1)
 			begin = 1;
-		if (end < 1)
+		if (end < 1 || lines < end)
 			end = lines;
 		begin--;
 		line_log_data_insert(&ranges, full_name, begin, end);
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index d0377fae5..436b13ad2 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -60,7 +60,6 @@ test_bad_opts "-L 1:nonexistent" "There is no path"
 test_bad_opts "-L 1:simple" "There is no path"
 test_bad_opts "-L '/foo:b.c'" "argument not .start,end:file"
 test_bad_opts "-L 1000:b.c" "has only.*lines"
-test_bad_opts "-L 1,1000:b.c" "has only.*lines"
 test_bad_opts "-L :b.c" "argument not .start,end:file"
 test_bad_opts "-L :foo:b.c" "no match"
 
@@ -86,12 +85,12 @@ test_expect_success '-L ,Y (Y == nlines)' '
 
 test_expect_success '-L ,Y (Y == nlines + 1)' '
 	n=$(expr $(wc -l <b.c) + 1) &&
-	test_must_fail git log -L ,$n:b.c
+	git log -L ,$n:b.c
 '
 
 test_expect_success '-L ,Y (Y == nlines + 2)' '
 	n=$(expr $(wc -l <b.c) + 2) &&
-	test_must_fail git log -L ,$n:b.c
+	git log -L ,$n:b.c
 '
 
 test_expect_success '-L with --first-parent and a merge' '

-- 
2.14.3 (Apple Git-98)
Junio C Hamano· Jun 1, 2018, 01:23 UTC · re: istephens@atlassian.com · lore

Re: [PATCH v6 2/2] log: prevent error if line range ends past end of file

istephens@atlassian.com writes:
Show 16 quoted lines
> From: Isabella Stephens <istephens@atlassian.com>
>
> If the -L option is used to specify a line range in git log, and the end
> of the range is past the end of the file, git will fail with a fatal
> error. This commit prevents such behaviour - instead we perform the log
> for existing lines within the specified range.
>
> This commit also fixes a corner case where -L ,-n:file would be treated
> as a log over the whole file. Now we treat this as -L 1,-n:file and
> blame the first line of the file instead.
>
> Signed-off-by: Isabella Stephens <istephens@atlassian.com>
> ---
>  line-log.c          | 8 +++++---
>  t/t4211-line-log.sh | 5 ++---
>  2 files changed, 7 insertions(+), 6 deletions(-)
Thanks.  Will queue.
Junio C Hamano· Jun 1, 2018, 01:27 UTC · re: istephens@atlassian.com · lore

Re: [PATCH v6 1/2] blame: prevent error if range ends past end of file

istephens@atlassian.com writes:
Show 19 quoted lines
> From: Isabella Stephens <istephens@atlassian.com>
>
> If the -L option is used to specify a line range in git blame, and the
> end of the range is past the end of the file, git will fail with a fatal
> error. This commit prevents such behavior - instead we display the blame
> for existing lines within the specified range. Tests are amended
> accordingly.
>
> This commit also fixes two corner cases. Blaming -L n,-(n+1) now blames
> the first n lines of a file rather than from n to the end of the file.
> Blaming -L ,-n will be treated as -L 1,-n and blame the first line of
> the file, rather than blaming the whole file.
>
> Signed-off-by: Isabella Stephens <istephens@atlassian.com>
> ---
>  builtin/blame.c               |  4 ++--
>  line-range.c                  |  2 +-
>  t/t8003-blame-corner-cases.sh | 12 ++++++++----
>  3 files changed, 11 insertions(+), 7 deletions(-)
Don't t800[12]-*.sh need adjustment for this change, too?
istephens@atlassian.com· Jun 15, 2018, 06:29 UTC · re: Junio C Hamano · lore

[PATCH v7 1/2] blame: prevent error if range ends past end of file

From: Isabella Stephens <istephens@atlassian.com>

If the -L option is used to specify a line range in git blame, and the end of the range is past the end of the file, git will fail with a fatal error. This commit prevents such behavior - instead we display the blame for existing lines within the specified range. Tests are amended accordingly.

This commit also fixes two corner cases. Blaming -L n,-(n+1) now blames the first n lines of a file rather than from n to the end of the file. Blaming -L ,-n will be treated as -L 1,-n and blame the first line of the file, rather than blaming the whole file.

Signed-off-by: Isabella Stephens <istephens@atlassian.com>
---
 builtin/blame.c               |  4 ++--
 line-range.c                  |  2 +-
 t/annotate-tests.sh           |  4 ++--
 t/t8003-blame-corner-cases.sh | 12 ++++++++----
 4 files changed, 13 insertions(+), 9 deletions(-)
Show changes to 4 files +13 −9

builtin/blame.c, line-range.c, t/annotate-tests.sh, t/t8003-blame-corner-cases.sh

diff --git a/builtin/blame.c b/builtin/blame.c
index 9dcb367b9..e1359b192 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -886,13 +886,13 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 				    nth_line_cb, &sb, lno, anchor,
 				    &bottom, &top, sb.path))
 			usage(blame_usage);
-		if (lno < top || ((lno || bottom) && lno < bottom))
+		if ((!lno && (top || bottom)) || lno < bottom)
 			die(Q_("file %s has only %lu line",
 			       "file %s has only %lu lines",
 			       lno), path, lno);
 		if (bottom < 1)
 			bottom = 1;
-		if (top < 1)
+		if (top < 1 || lno < top)
 			top = lno;
 		bottom--;
 		range_set_append_unsafe(&ranges, bottom, top);
diff --git a/line-range.c b/line-range.c
index 323399d16..232c3909e 100644
--- a/line-range.c
+++ b/line-range.c
@@ -47,7 +47,7 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
 			else if (!num)
 				*ret = begin;
 			else
-				*ret = begin + num;
+				*ret = begin + num > 0 ? begin + num : 1;
 			return term;
 		}
 		return spec;
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 093832fef..6da48a2e0 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -320,11 +320,11 @@ test_expect_success 'blame -L ,Y (Y == nlines)' '
 
 test_expect_success 'blame -L ,Y (Y == nlines + 1)' '
 	n=$(expr $(wc -l <file) + 2) &&
-	test_must_fail $PROG -L,$n file
+	check_count -L,$n A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1
 '
 
 test_expect_success 'blame -L ,Y (Y > nlines)' '
-	test_must_fail $PROG -L,12345 file
+	check_count -L,12345 A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1
 '
 
 test_expect_success 'blame -L multiple (disjoint)' '
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 661f9d430..c92a47b6d 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -216,14 +216,18 @@ test_expect_success 'blame -L with invalid start' '
 '
 
 test_expect_success 'blame -L with invalid end' '
-	test_must_fail git blame -L1,5 tres 2>errors &&
-	test_i18ngrep "has only 2 lines" errors
+	git blame -L1,5 tres >out &&
+	test_line_count = 2 out
 '
 
 test_expect_success 'blame parses <end> part of -L' '
 	git blame -L1,1 tres >out &&
-	cat out &&
-	test $(wc -l < out) -eq 1
+	test_line_count = 1 out
+'
+
+test_expect_success 'blame -Ln,-(n+1)' '
+	git blame -L3,-4 nine_lines >out &&
+	test_line_count = 3 out
 '
 
 test_expect_success 'indent of line numbers, nine lines' '

-- 
2.14.3 (Apple Git-98)
istephens@atlassian.com· Jun 15, 2018, 06:29 UTC · re: istephens@atlassian.com · lore

[PATCH v7 2/2] log: prevent error if line range ends past end of file

From: Isabella Stephens <istephens@atlassian.com>

If the -L option is used to specify a line range in git log, and the end of the range is past the end of the file, git will fail with a fatal error. This commit prevents such behaviour - instead we perform the log for existing lines within the specified range.

This commit also fixes a corner case where -L ,-n:file would be treated as a log over the whole file. Now we treat this as -L 1,-n:file and blame the first line of the file instead.

Signed-off-by: Isabella Stephens <istephens@atlassian.com>
---
 line-log.c          | 4 ++--
 t/t4211-line-log.sh | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)
Show changes to 2 files +4 −5

line-log.c, t/t4211-line-log.sh

diff --git a/line-log.c b/line-log.c
index cdc2257db..8ad074e58 100644
--- a/line-log.c
+++ b/line-log.c
@@ -599,11 +599,11 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args)
 				    lines, anchor, &begin, &end,
 				    full_name))
 			die("malformed -L argument '%s'", range_part);
-		if (lines < end || ((lines || begin) && lines < begin))
+		if ((!lines && (begin || end)) || lines < begin)
 			die("file %s has only %lu lines", name_part, lines);
 		if (begin < 1)
 			begin = 1;
-		if (end < 1)
+		if (end < 1 || lines < end)
 			end = lines;
 		begin--;
 		line_log_data_insert(&ranges, full_name, begin, end);
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index d0377fae5..436b13ad2 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -60,7 +60,6 @@ test_bad_opts "-L 1:nonexistent" "There is no path"
 test_bad_opts "-L 1:simple" "There is no path"
 test_bad_opts "-L '/foo:b.c'" "argument not .start,end:file"
 test_bad_opts "-L 1000:b.c" "has only.*lines"
-test_bad_opts "-L 1,1000:b.c" "has only.*lines"
 test_bad_opts "-L :b.c" "argument not .start,end:file"
 test_bad_opts "-L :foo:b.c" "no match"
 
@@ -86,12 +85,12 @@ test_expect_success '-L ,Y (Y == nlines)' '
 
 test_expect_success '-L ,Y (Y == nlines + 1)' '
 	n=$(expr $(wc -l <b.c) + 1) &&
-	test_must_fail git log -L ,$n:b.c
+	git log -L ,$n:b.c
 '
 
 test_expect_success '-L ,Y (Y == nlines + 2)' '
 	n=$(expr $(wc -l <b.c) + 2) &&
-	test_must_fail git log -L ,$n:b.c
+	git log -L ,$n:b.c
 '
 
 test_expect_success '-L with --first-parent and a merge' '

-- 
2.14.3 (Apple Git-98)
Isabella Stephens· Jun 15, 2018, 06:36 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 1/2] blame: prevent error if range ends past end of file

> Don't t800[12]-*.sh need adjustment for this change, too?
Apologies. I will make sure I've run the whole test suite against my patches in future. I've sent through a 7th (and hopefully final) version.
Junio C Hamano· Jun 15, 2018, 18:28 UTC · re: Isabella Stephens · lore

Re: [PATCH v6 1/2] blame: prevent error if range ends past end of file

Isabella Stephens <istephens@atlassian.com> writes:
Show 6 quoted lines
>> Don't t800[12]-*.sh need adjustment for this change, too?
>
> Apologies. I will make sure I've run the whole test suite against
> my patches in future. I've sent through a 7th (and hopefully
> final) version.
>

Thanks. Will queue and hopefully we can carry this in 'pu' (and later down to 'next' and to 'master').

← back to recent threads