git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH 0/1] contrib: Add script to show uncovered "new" lines
@ 2018-09-12 16:45 Derrick Stolee via GitGitGadget
  2018-09-12 16:45 ` [PATCH 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2018-09-12 16:45 UTC (permalink / raw)
  To: git; +Cc: peff, Junio C Hamano

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

make coverage-test
make coverage-report

This leaves the repo in a state where every X.c file that was covered has an
X.c.gcov file containing the coverage counts for every line, and "#####" at
every uncovered line.

There have been a few bugs in recent patches what would have been caught if
the test suite covered those blocks (including a few of mine). I want to
work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks in error
cases do not need to be covered.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by
the test suite. For example, I ran this against the 'jch' branch (d3c0046)
versus 'next' (dd90340) and got the following output:

builtin/commit.c
859fdc0c3cf     (Derrick Stolee 2018-08-29 05:49:04 -0700       1657)           write_commit_graph_reachable(get_object_directory(), 0);
builtin/rev-list.c
250edfa8c87     (Harald Nordgren        2018-04-18 23:05:35 +0200       431)                    bisect_flags |= BISECT_FIND_ALL;
builtin/worktree.c
e5353bef550     (Eric Sunshine  2018-08-28 17:20:19 -0400       60)             error_errno(_("failed to delete '%s'"), sb.buf);
e19831c94f9     (Eric Sunshine  2018-08-28 17:20:23 -0400       251)                die(_("unable to re-add worktree '%s'"), path);
68a6b3a1bd4     (Eric Sunshine  2018-08-28 17:20:24 -0400       793)                    die(_("cannot move a locked working tree, lock reason: %s\nuse 'move -f -f' to override or unlock first"),
f4143101cbb     (Eric Sunshine  2018-08-28 17:20:25 -0400       906)                    die(_("cannot remove a locked working tree, lock reason: %s\nuse 'remove -f -f' to override or unlock first"),
read-cache.c
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1754)           const unsigned char *cp = (const unsigned char *)name;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1757)           previous_len = previous_ce ? previous_ce->ce_namelen : 0;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1758)           strip_len = decode_varint(&cp);
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1759)           if (previous_len < strip_len) {
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1760)                   if (previous_ce)
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1761)                           die(_("malformed name field in the index, near path '%s'"),
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1762)                               previous_ce->name);
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1764)                           die(_("malformed name field in the index in the first path"));
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1766)           copy_len = previous_len - strip_len;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1767)           name = (const char *)cp;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1773)                   len += copy_len;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1794)           if (copy_len)
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1795)                   memcpy(ce->name, previous_ce->name, copy_len);
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1796)           memcpy(ce->name + copy_len, name, len + 1 - copy_len);
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1797)           *ent_size = (name - ((char *)ondisk)) + len + 1 - copy_len;
remote-curl.c
c3b9bc94b9b     (Elijah Newren  2018-09-05 10:03:07 -0700       181)            options.filter = xstrdup(value);

Using this 'git blame' output, we can quickly inspect whether the uncovered
lines are appropriate. For instance:

 1. The line in builtin/commit.c is due to writing the commit-graph file
    when GIT_TEST_COMMIT_GRAPH is enabled, which is not on by default in the
    test suite. Being uncovered is expected here.
    
    
 2. The lines in builtin/worktree.c are all related to error conditions.
    This is acceptable.
    
    
 3. The line in builtin/rev-list.c is a flag replacement in a block that is
    otherwise unchanged. It must not be covered by the test suite normally.
    This could be worth adding a test to ensure the new logic maintains old
    behavior.
    
    
 4. The lines in read-cache.c are part of a new block for the condition "if
    (expand_name_field)" as part of an optimization. These lines should
    probably be covered before that series is merged to 'next'. I understand
    that Ben and Duy are continuing work in this direction [1].
    
    

I used this approach for 'next' over 'master' and got a larger list, some of
which I have already submitted tests to increase coverage [2] or will be
covered by topics not in 'next' [3].

Thanks, -Stolee

[1] 
https://public-inbox.org/git/20180912161832.55324-1-benpeart@microsoft.com/T/#u

[2] https://public-inbox.org/git/pull.37.git.gitgitgadget@gmail.com/

[3] https://public-inbox.org/git/pull.34.git.gitgitgadget@gmail.com/

Derrick Stolee (1):
  contrib: add coverage-diff script

 contrib/coverage-diff.sh | 70 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100755 contrib/coverage-diff.sh


base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-40%2Fderrickstolee%2Fcoverage-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-40/derrickstolee/coverage-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/40
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/1] contrib: add coverage-diff script
  2018-09-12 16:45 [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
@ 2018-09-12 16:45 ` Derrick Stolee via GitGitGadget
  2018-09-12 22:13   ` Junio C Hamano
  2018-09-12 19:14 ` [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2018-09-12 16:45 UTC (permalink / raw)
  To: git; +Cc: peff, Junio C Hamano, Derrick Stolee

From: Derrick Stolee <dstolee@microsoft.com>

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

    make coverage-test
    make coverage-report

This leaves the repo in a state where every X.c file that was covered has
an X.c.gcov file containing the coverage counts for every line, and "#####"
at every uncovered line.

There have been a few bugs in recent patches what would have been caught
if the test suite covered those blocks (including a few of mine). I want
to work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks in error
cases do not need to be covered.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

    contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by the
test suite. The output uses 'git blame -c' format so you can find the commits
responsible and view the line numbers for quick access to the context.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 contrib/coverage-diff.sh | 70 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100755 contrib/coverage-diff.sh

diff --git a/contrib/coverage-diff.sh b/contrib/coverage-diff.sh
new file mode 100755
index 0000000000..22acb13d38
--- /dev/null
+++ b/contrib/coverage-diff.sh
@@ -0,0 +1,70 @@
+#!/bin/bash
+
+# Usage: 'contrib/coverage-diff.sh <version1> <version2>
+# Outputs a list of new lines in version2 compared to version1 that are
+# not covered by the test suite. Assumes you ran
+# 'make coverage-test coverage-report' from root first, so .gcov files exist.
+
+V1=$1
+V2=$2
+
+diff-lines() {
+    local path=
+    local line=
+    while read; do
+	esc=$'\033'
+	if [[ $REPLY =~ ---\ (a/)?.* ]]; then
+	    continue
+	elif [[ $REPLY =~ \+\+\+\ (b/)?([^[:blank:]$esc]+).* ]]; then
+	    path=${BASH_REMATCH[2]}
+	elif [[ $REPLY =~ @@\ -[0-9]+(,[0-9]+)?\ \+([0-9]+)(,[0-9]+)?\ @@.* ]]; then
+	    line=${BASH_REMATCH[2]}
+	elif [[ $REPLY =~ ^($esc\[[0-9;]+m)*([\ +-]) ]]; then
+	    echo "$path:$line:$REPLY"
+	    if [[ ${BASH_REMATCH[2]} != - ]]; then
+		((line++))
+	    fi
+	fi
+    done
+}
+
+git diff --raw $V1 $V2 | grep \.c$ | awk 'NF>1{print $NF}' >files.txt
+
+for file in $(cat files.txt)
+do
+	hash_file=${file//\//\#}
+
+	git diff $V1 $V2 -- $file \
+		| diff-lines \
+		| grep ":+" \
+		>"diff_file.txt"
+
+	cat diff_file.txt \
+		| sed -E 's/:/ /g' \
+		| awk '{print $2}' \
+		| sort \
+		>new_lines.txt
+
+	cat "$hash_file.gcov" \
+		| grep \#\#\#\#\# \
+		| sed 's/    #####: //g' \
+		| sed 's/\:/ /g' \
+		| awk '{print $1}' \
+		| sort \
+		>uncovered_lines.txt
+
+	comm -12 uncovered_lines.txt new_lines.txt \
+		| sed -e 's/$/\)/' \
+		| sed -e 's/^/\t/' \
+		>uncovered_new_lines.txt
+
+	grep -q '[^[:space:]]' < uncovered_new_lines.txt && \
+		echo $file && \
+		git blame -c $file \
+			| grep -f uncovered_new_lines.txt
+
+	rm -f diff_file.txt new_lines.txt \
+		uncovered_lines.txt uncovered_new_lines.txt
+done
+
+rm -rf files.txt
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/1] contrib: Add script to show uncovered "new" lines
  2018-09-12 16:45 [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
  2018-09-12 16:45 ` [PATCH 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
@ 2018-09-12 19:14 ` Derrick Stolee
  2018-09-12 19:33 ` Ben Peart
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Derrick Stolee @ 2018-09-12 19:14 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget, git; +Cc: peff, Junio C Hamano

On 9/12/2018 12:45 PM, Derrick Stolee via GitGitGadget wrote:
> For example, I ran this against the 'jch' branch (d3c0046)
> versus 'next' (dd90340)

As another example, I ran this against the 'pu' branch (4c416a53) versus 
'jch' (d3c0046) and got the following output, submitted here without 
commentary:

builtin/bisect--helper.c
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
43)             free((void *) terms->term_good);
3d3237b0e6b     (Pranit Bauva   2017-10-27 15:06:37 +0000 
162)            if (get_oid_commit(commit, &oid))
3d3237b0e6b     (Pranit Bauva   2017-10-27 15:06:37 +0000 
163)                    return error(_("'%s' is not a valid commit"), 
commit);
3d3237b0e6b     (Pranit Bauva   2017-10-27 15:06:37 +0000 
164)            strbuf_addstr(&branch, commit);
3d3237b0e6b     (Pranit Bauva   2017-10-27 15:06:37 +0000 
172)                    error(_("Could not check out original HEAD '%s'. 
Try "
3d3237b0e6b     (Pranit Bauva   2017-10-27 15:06:37 +0000 
174)                    strbuf_release(&branch);
3d3237b0e6b     (Pranit Bauva   2017-10-27 15:06:37 +0000 
175)                    argv_array_clear(&argv);
3d3237b0e6b     (Pranit Bauva   2017-10-27 15:06:37 +0000 
176)                    return -1;
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
215)            error(_("Bad bisect_write argument: %s"), state);
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
216)            goto fail;
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
220)            error(_("couldn't get the oid of the rev '%s'"), rev);
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
221)            goto fail;
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
226)            goto fail;
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
230)            error_errno(_("couldn't open the file '%s'"), 
git_path_bisect_log());
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
231)            goto fail;
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 242)fail:
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 243)    retval 
= -1;
a919f328ba3     (Pranit Bauva   2017-10-27 15:06:37 +0000 
323)            yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
a919f328ba3     (Pranit Bauva   2017-10-27 15:06:37 +0000 
324)            if (starts_with(yesno, "N") || starts_with(yesno, "n"))
a919f328ba3     (Pranit Bauva   2017-10-27 15:06:37 +0000 
327)            goto finish;
a919f328ba3     (Pranit Bauva   2017-10-27 15:06:37 +0000 
336)            error(_("You need to start by \"git bisect start\". You "
a919f328ba3     (Pranit Bauva   2017-10-27 15:06:37 +0000 
341)            goto fail;
a919f328ba3     (Pranit Bauva   2017-10-27 15:06:37 +0000 345)fail:
35f7ca528ae     (Pranit Bauva   2017-10-27 15:06:37 +0000 
387)            return error(_("--bisect-term requires exactly one 
argument"));
35f7ca528ae     (Pranit Bauva   2017-10-27 15:06:37 +0000 
400)                    error(_("BUG: invalid argument %s for 'git 
bisect terms'.\n"
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
416)            return -1;
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
419)            goto fail;
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
423)            goto fail;
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 427)fail:
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 428)    retval 
= -1;
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
466)                    no_checkout = 1;
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
488)                     !one_of(arg, "--term-good", "--term-bad", NULL)) {
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
489)                    return error(_("unrecognised option: '%s'"), arg);
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
523)            if (get_oid("HEAD", &head_oid))
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
524)                    return error(_("Bad HEAD - I need a HEAD"));
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
539)                            error(_("checking out '%s' failed. Try 
'git "
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
559)                            return error(_("won't bisect on 
cg-seek'ed tree"));
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
562)                    return error(_("Bad HEAD - strange symbolic ref"));
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
570)            return -1;
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
588)                    goto fail;
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
598)                    goto fail;
5dfeec316b8     (Pranit Bauva   2017-10-27 15:06:37 +0000 
606)            goto fail;
3d3237b0e6b     (Pranit Bauva   2017-10-27 15:06:37 +0000 
686)                    return error(_("--bisect-reset requires either 
no argument or a commit"));
0b1f0fd910c     (Pranit Bauva   2017-10-27 15:06:37 +0000 
690)                    return error(_("--bisect-write requires either 4 
or 5 arguments"));
20edf353b72     (Pranit Bauva   2017-10-27 15:06:37 +0000 
697)                    return error(_("--check-and-set-terms requires 3 
arguments"));
a919f328ba3     (Pranit Bauva   2017-10-27 15:06:37 +0000 
703)                    return error(_("--bisect-next-check requires 2 
or 3 arguments"));
builtin/blame.c
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 922)    case 
DATE_HUMAN:
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
924)            blame_date_width = sizeof("Thu Oct 19 16:00");
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
925)            break;
builtin/gc.c
3029970275b     (Jonathan Nieder        2018-07-16 23:57:40 -0700       
461)            ret = error_errno(_("cannot stat '%s'"), gc_log_path);
3029970275b     (Jonathan Nieder        2018-07-16 23:57:40 -0700       
470)            ret = error_errno(_("cannot read '%s'"), gc_log_path);
fec2ed21871     (Jonathan Nieder        2018-07-16 23:54:16 -0700       
495)            die(FAILED_RUN, pack_refs_cmd.argv[0]);
fec2ed21871     (Jonathan Nieder        2018-07-16 23:54:16 -0700       
498)            die(FAILED_RUN, reflog.argv[0]);
3029970275b     (Jonathan Nieder        2018-07-16 23:57:40 -0700       
585)                            exit(128);
fec2ed21871     (Jonathan Nieder        2018-07-16 23:54:16 -0700       
637)                    die(FAILED_RUN, repack.argv[0]);
fec2ed21871     (Jonathan Nieder        2018-07-16 23:54:16 -0700       
647)                            die(FAILED_RUN, prune.argv[0]);
fec2ed21871     (Jonathan Nieder        2018-07-16 23:54:16 -0700       
654)                    die(FAILED_RUN, prune_worktrees.argv[0]);
fec2ed21871     (Jonathan Nieder        2018-07-16 23:54:16 -0700       
658)            die(FAILED_RUN, rerere.argv[0]);
builtin/rebase--interactive.c
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
24)             return error(_("no HEAD?"));
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
51)             return error_errno(_("could not create temporary %s"), 
path_state_dir());
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
57)             return error_errno(_("could not mark as interactive"));
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
77)             return -1;
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
81)             return -1;
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
87)             free(revisions);
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
88)             free(shortrevisions);
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
90)             return -1;
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
98)             free(revisions);
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
99)             free(shortrevisions);
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
101)            return error_errno(_("could not open %s"), 
rebase_path_todo());
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
106)            argv_array_push(&make_script_args, restrict_revision);
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
114)            error(_("could not generate todo list"));
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 205) 
usage_with_options(builtin_rebase_interactive_usage, options);
93420467efe     (Alban Gruin    2018-08-28 14:10:42 +0200 
219)            warning(_("--[no-]rebase-cousins has no effect without "
adb4f8f6b72     (Alban Gruin    2018-08-28 14:10:43 +0200 
225)                    die(_("a base commit must be provided with 
--upstream or --onto"));
b3fe2e1f8cb     (Alban Gruin    2018-08-28 14:10:45 +0200 259)    case 
REARRANGE_SQUASH:
b3fe2e1f8cb     (Alban Gruin    2018-08-28 14:10:45 +0200 
260)            ret = rearrange_squash();
b3fe2e1f8cb     (Alban Gruin    2018-08-28 14:10:45 +0200 
261)            break;
b3fe2e1f8cb     (Alban Gruin    2018-08-28 14:10:45 +0200 262)    case 
ADD_EXEC:
b3fe2e1f8cb     (Alban Gruin    2018-08-28 14:10:45 +0200 
263)            ret = sequencer_add_exec_commands(cmd);
b3fe2e1f8cb     (Alban Gruin    2018-08-28 14:10:45 +0200 
264)            break;
adb4f8f6b72     (Alban Gruin    2018-08-28 14:10:43 +0200 265)    default:
adb4f8f6b72     (Alban Gruin    2018-08-28 14:10:43 +0200 
266)            BUG("invalid command '%d'", command);
builtin/rebase.c
55071ea248e     (Pratik Karki   2018-08-07 01:16:09 +0545 61)     
strbuf_trim(&out);
55071ea248e     (Pratik Karki   2018-08-07 01:16:09 +0545 62)     ret = 
!strcmp("true", out.buf);
55071ea248e     (Pratik Karki   2018-08-07 01:16:09 +0545 63)     
strbuf_release(&out);
002ee2fe682     (Pratik Karki   2018-09-04 14:59:57 -0700 114)    case 
REBASE_AM:
002ee2fe682     (Pratik Karki   2018-09-04 14:59:57 -0700 
115)            die(_("%s requires an interactive rebase"), option);
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
148)            return error_errno(_("could not read '%s'"), path);
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
162)            return -1;
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
167)            return error(_("could not get 'onto': '%s'"), buf.buf);
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
178)                    return -1;
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 179)    } else 
if (read_one(state_dir_path("head", opts), &buf))
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
180)            return -1;
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
182)            return error(_("invalid orig-head: '%s'"), buf.buf);
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
186)            return -1;
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
188)            opts->flags &= ~REBASE_NO_QUIET;
73d51ed0a59     (Pratik Karki   2018-09-04 14:59:50 -0700 
196)            opts->signoff = 1;
73d51ed0a59     (Pratik Karki   2018-09-04 14:59:50 -0700 
197)            opts->flags |= REBASE_FORCE;
ead98c111b8     (Pratik Karki   2018-09-04 14:59:52 -0700 
204)                    return -1;
28a02c5a790     (Pratik Karki   2018-09-04 15:00:00 -0700 
219)                    return -1;
399a505296a     (Pratik Karki   2018-09-04 15:00:11 -0700 
227)                    return -1;
399a505296a     (Pratik Karki   2018-09-04 15:00:11 -0700 
235)                    return -1;
7debdaa4ad1     (Pratik Karki   2018-09-04 15:00:02 -0700 
255)            return error(_("Could not read '%s'"), path);
7debdaa4ad1     (Pratik Karki   2018-09-04 15:00:02 -0700 
271)                    res = error(_("Cannot store %s"), autostash.buf);
7debdaa4ad1     (Pratik Karki   2018-09-04 15:00:02 -0700 
275)                    return res;
b2263c13613     (Johannes Schindelin    2018-08-29 07:31:17 -0700       
373) argv_array_pushf(&child.args,
b2263c13613     (Johannes Schindelin    2018-08-29 07:31:17 -0700       
375) oid_to_hex(&opts->restrict_revision->object.oid));
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 479)    case 
REBASE_INTERACTIVE:
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
480)            backend = "git-rebase--interactive";
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
481)            backend_func = "git_rebase__interactive";
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
482)            break;
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 491)    default:
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
492)            BUG("Unhandled rebase type %d", opts->type);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
509)            struct strbuf dir = STRBUF_INIT;
7debdaa4ad1     (Pratik Karki   2018-09-04 15:00:02 -0700 
511)            apply_autostash(opts);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
512)            strbuf_addstr(&dir, opts->state_dir);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
513)            remove_dir_recursively(&dir, 0);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
514)            strbuf_release(&dir);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
515)            die("Nothing to do");
d4c569f8f4c     (Pratik Karki   2018-09-04 14:27:20 -0700 
542)            BUG("Not a fully qualified branch: '%s'", switch_to_branch);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
545)            return -1;
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
549)                    rollback_lock_file(&lock);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
550)                    return error(_("could not determine HEAD 
revision"));
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
567)            rollback_lock_file(&lock);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
568)            return error(_("could not read index"));
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
572)            error(_("failed to find tree of %s"), oid_to_hex(oid));
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
573)            rollback_lock_file(&lock);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
574)            free((void *)desc.buffer);
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
575)            return -1;
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
588)            ret = error(_("could not write index"));
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
592)            return ret;
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 608)    } else 
if (old_orig)
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
609)            delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
bff014dac7d     (Pratik Karki   2018-09-04 14:27:13 -0700 
637)                    opts->flags &= !REBASE_DIFFSTAT;
9a48a615b47     (Pratik Karki   2018-09-04 14:27:16 -0700 
671)                    return 1;
9a48a615b47     (Pratik Karki   2018-09-04 14:27:16 -0700 
687)            return 0;
55071ea248e     (Pratik Karki   2018-08-07 01:16:09 +0545 
894)            const char *path = mkpath("%s/git-legacy-rebase",
55071ea248e     (Pratik Karki   2018-08-07 01:16:09 +0545 
897)            if (sane_execvp(path, (char **)argv) < 0)
55071ea248e     (Pratik Karki   2018-08-07 01:16:09 +0545 
898)                    die_errno(_("could not exec %s"), path);
55071ea248e     (Pratik Karki   2018-08-07 01:16:09 +0545 
900)                    BUG("sane_execvp() returned???");
0eabf4b95ca     (Pratik Karki   2018-08-08 20:51:22 +0545 
916)            die(_("It looks like 'git am' is in progress. Cannot 
rebase."));
f28d40d3a99     (Pratik Karki   2018-09-04 14:27:07 -0700 
953)            usage_with_options(builtin_rebase_usage,
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
973)                    die(_("Cannot read HEAD"));
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
977)                    die(_("could not read index"));
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
991)                    exit(1);
122420c2953     (Pratik Karki   2018-08-08 20:51:17 +0545 
1003)                   die(_("could not discard worktree changes"));
122420c2953     (Pratik Karki   2018-08-08 20:51:17 +0545 
1005)                   exit(1);
5e5d96197ca     (Pratik Karki   2018-08-08 20:51:18 +0545 
1016)                   exit(1);
5e5d96197ca     (Pratik Karki   2018-08-08 20:51:18 +0545 
1019)                   die(_("could not move back to %s"),
5a61494539b     (Pratik Karki   2018-08-08 20:51:19 +0545 
1029)                   die(_("could not remove '%s'"), options.state_dir);
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 1042)   default:
51e9ea6da76     (Pratik Karki   2018-08-08 20:51:20 +0545 
1043)           BUG("action: %d", action);
c54dacb50ea     (Pratik Karki   2018-09-04 14:27:18 -0700 
1048)           const char *last_slash = strrchr(options.state_dir, '/');
c54dacb50ea     (Pratik Karki   2018-09-04 14:27:18 -0700 
1049)           const char *state_dir_base =
c54dacb50ea     (Pratik Karki   2018-09-04 14:27:18 -0700 
1050)                   last_slash ? last_slash + 1 : options.state_dir;
c54dacb50ea     (Pratik Karki   2018-09-04 14:27:18 -0700 
1051)           const char *cmd_live_rebase =
c54dacb50ea     (Pratik Karki   2018-09-04 14:27:18 -0700 
1053)           strbuf_reset(&buf);
c54dacb50ea     (Pratik Karki   2018-09-04 14:27:18 -0700 
1054)           strbuf_addf(&buf, "rm -fr \"%s\"", options.state_dir);
c54dacb50ea     (Pratik Karki   2018-09-04 14:27:18 -0700 
1055)           die(_("It seems that there is already a %s directory, and\n"
53f9e5be94e     (Pratik Karki   2018-09-04 14:59:56 -0700 
1079)           strbuf_addstr(&options.git_am_opt, " --ignore-date");
53f9e5be94e     (Pratik Karki   2018-09-04 14:59:56 -0700 
1080)           options.flags |= REBASE_FORCE;
c7ee2134d42     (Pratik Karki   2018-09-04 15:00:01 -0700 
1092)           strbuf_addf(&options.git_am_opt, " -C%d", opt_c);
0073df2bd31     (Pratik Karki   2018-09-04 15:00:07 -0700 
1124)           else if (strcmp("no-rebase-cousins", rebase_merges))
0073df2bd31     (Pratik Karki   2018-09-04 15:00:07 -0700 
1125)                   die(_("Unknown mode: %s"), rebase_merges);
399a505296a     (Pratik Karki   2018-09-04 15:00:11 -0700 
1146)           case REBASE_AM:
399a505296a     (Pratik Karki   2018-09-04 15:00:11 -0700 
1147)                   die(_("--strategy requires --merge or 
--interactive"));
399a505296a     (Pratik Karki   2018-09-04 15:00:11 -0700 
1156)           default:
399a505296a     (Pratik Karki   2018-09-04 15:00:11 -0700 
1157)                   BUG("unhandled rebase type (%d)", options.type);
6dc73173f6c     (Pratik Karki   2018-08-08 21:21:33 +0545 
1165)           strbuf_addstr(&options.git_format_patch_opt, " --progress");
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 1173)   case 
REBASE_AM:
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
1174)           options.state_dir = apply_dir();
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
1175)           break;
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
1252)                   die(_("invalid upstream '%s'"), 
options.upstream_name);
bfa5147095f     (Pratik Karki   2018-09-04 15:00:12 -0700 
1258)                           die(_("Could not create new root commit"));
e65123a71d0     (Pratik Karki   2018-09-04 14:27:21 -0700 
1308)                   die(_("fatal: no such branch/commit '%s'"),
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
1316)                   die(_("No such ref: %s"), "HEAD");
ac7f467fef8     (Pratik Karki   2018-08-07 01:16:11 +0545 
1328)                   die(_("Could not resolve HEAD to a revision"));
e65123a71d0     (Pratik Karki   2018-09-04 14:27:21 -0700 
1330)           BUG("unexpected number of arguments left to parse");
e0333e5c63f     (Pratik Karki   2018-09-04 14:27:14 -0700 
1341)           die(_("could not read index"));
7debdaa4ad1     (Pratik Karki   2018-09-04 15:00:02 -0700 
1368)                           die(_("Cannot autostash"));
7debdaa4ad1     (Pratik Karki   2018-09-04 15:00:02 -0700 
1371)                           die(_("Unexpected stash response: '%s'"),
7debdaa4ad1     (Pratik Karki   2018-09-04 15:00:02 -0700 
1377)                           die(_("Could not create directory for 
'%s'"),
7debdaa4ad1     (Pratik Karki   2018-09-04 15:00:02 -0700 
1383)                           die(_("could not reset --hard"));
e65123a71d0     (Pratik Karki   2018-09-04 14:27:21 -0700 
1427)                                   ret = !!error(_("could not parse 
'%s'"),
e65123a71d0     (Pratik Karki   2018-09-04 14:27:21 -0700 
1429)                                   goto cleanup;
e65123a71d0     (Pratik Karki   2018-09-04 14:27:21 -0700 
1438)                                   ret = !!error(_("could not 
switch to "
1ed9c14ff25     (Pratik Karki   2018-09-04 14:27:17 -0700 
1448)                            resolve_ref_unsafe("HEAD", 0, NULL, &flag))
1ed9c14ff25     (Pratik Karki   2018-09-04 14:27:17 -0700 
1449)                           puts(_("HEAD is up to date."));
9a48a615b47     (Pratik Karki   2018-09-04 14:27:16 -0700 
1458)                    resolve_ref_unsafe("HEAD", 0, NULL, &flag))
9a48a615b47     (Pratik Karki   2018-09-04 14:27:16 -0700 
1459)                   puts(_("HEAD is up to date, rebase forced."));
builtin/rev-list.c
0eee403f2f7     (Matthew DeVore 2018-09-04 11:05:47 -0700 
227)            die("unexpected missing %s object '%s'",
0eee403f2f7     (Matthew DeVore 2018-09-04 11:05:47 -0700 
228)                type_name(obj->type), oid_to_hex(&obj->oid));
builtin/stash.c
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
130)            free_stash_info(info);
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
131)            error(_("'%s' is not a stash-like commit"), revision);
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
132)            exit(128);
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
165)                    free_stash_info(info);
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
166)                    fprintf_ln(stderr, _("No stash entries found."));
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
167)                    return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 201)    
default: /* Invalid or ambiguous */
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
202)            free_stash_info(info);
31f109a3618     (Joel Teichroeb 2018-08-31 00:40:37 +0300 
229)            return error(_("git stash clear with parameters is 
unimplemented"));
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
244)            return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
252)            return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
265)            return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
268)            return error(_("unable to write new index file"));
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
374)            remove_path(stash_index_path.buf);
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
375)            return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
402)            return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
405)            return error(_("Cannot apply a stash in the middle of a 
merge"));
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
415)                            strbuf_release(&out);
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
416)                            return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
422)                            return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
427)                            return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
434)            return error(_("Could not restore untracked files from 
stash"));
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
465)                    return -1;
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
470)                    strbuf_release(&out);
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
475)                    strbuf_release(&out);
93871263d18     (Joel Teichroeb 2018-08-31 00:40:36 +0300 
476)                    return -1;
31f109a3618     (Joel Teichroeb 2018-08-31 00:40:37 +0300 
551)            return error(_("%s: Could not drop stash entry"),
b3513da4bd9     (Joel Teichroeb 2018-08-31 00:40:39 +0300 
623)            printf_ln(_("The stash entry is kept in case you need it 
again."));
8ceb24b2c38     (Paul-Sebastian Ungureanu       2018-08-31 00:40:41 
+0300       754)            free_stash_info(&info);
129f0b0a009     (Paul-Sebastian Ungureanu       2018-08-31 00:40:48 
+0300       755) usage_with_options(git_stash_show_usage, options);
0ac06fb81f2     (Paul-Sebastian Ungureanu       2018-08-31 00:40:43 
+0300       808)            fprintf_ln(stderr, _("\"git stash store\" 
requires one <commit> argument"));
0ac06fb81f2     (Paul-Sebastian Ungureanu       2018-08-31 00:40:43 
+0300       809)            return -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       884)            return 1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       925)            ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       926)            goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       931)            ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       932)            goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       937)            ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       938)            goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       966)            ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       967)            goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       978)            ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       979)            goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       984)            ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       985)            goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       992)            ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       993)            goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1018)           ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1019)           goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1031)           ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1032)           goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1037)           ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1038)           goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1049)           ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1050)           goto done;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1055)           ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1056)           goto done;
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1089)                   fprintf_ln(stderr, _("You do not 
have the initial commit yet"));
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1110)           if (!quiet)
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1111)                   fprintf_ln(stderr, _("Cannot save 
the current index state"));
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1112)           ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1113)           *stash_msg = NULL;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1114)           goto done;
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1119)                   if (!quiet)
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1120) fprintf_ln(stderr, _("Cannot save the untracked files"));
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1121)                   ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1122)                   *stash_msg = NULL;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1123)                   goto done;
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1131)                   if (!quiet)
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1132) fprintf_ln(stderr, _("Cannot save the current worktree 
state"));
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1133)                   goto done;
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1139)                   if (!quiet)
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1140) fprintf_ln(stderr, _("Cannot save the current worktree 
state"));
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1141)                   ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1142)                   *stash_msg = NULL;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1143)                   goto done;
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1166)           if (!quiet)
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1167)                   fprintf_ln(stderr, _("Cannot record 
working tree state"));
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1168)           ret = -1;
f6f191b3f25     (Paul-Sebastian Ungureanu       2018-08-31 00:40:44 
+0300       1169)           goto done;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1251)           return -1;
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1260)           if (!quiet)
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1261)                   fprintf_ln(stderr, _("Cannot 
initialize stash"));
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1262)           return -1;
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1272)           if (!quiet)
8002b9e6264     (Paul-Sebastian Ungureanu       2018-08-31 00:40:46 
+0300       1273)                   fprintf_ln(stderr, _("Cannot save 
the current status"));
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1274)           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1275)           goto done;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1292)                           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1311)                           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1312)                           goto done;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1321)                           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1322)                           goto done;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1330)                           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1339)                           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1350)                           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1359)                           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1360)                           goto done;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1368)                           ret = -1;
48c061fa443     (Paul-Sebastian Ungureanu       2018-08-31 00:40:45 
+0300       1394)                           ret = -1;
129f0b0a009     (Paul-Sebastian Ungureanu       2018-08-31 00:40:48 
+0300       1524) usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), 
argv[0]),
129f0b0a009     (Paul-Sebastian Ungureanu       2018-08-31 00:40:48 
+0300       1554)                           continue;
builtin/submodule--helper.c
9d34daefb74     (Antonio Ospite 2018-08-14 13:05:22 +0200 2174)   
die("submodule--helper config takes 1 or 2 arguments: name [value]");
commit-graph.c
5cef295f283     (Derrick Stolee 2018-08-20 18:24:32 +0000 
66)             return 0;
20fd6d57996     (Derrick Stolee 2018-08-20 18:24:30 +0000 
78)             return 0;
date.c
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
113)            die("Timestamp too large for this system: %"PRItime, time);
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
216)            if (tm->tm_mon == human_tm->tm_mon) {
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
217)                    if (tm->tm_mday > human_tm->tm_mday) {
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
219)                    } else if (tm->tm_mday == human_tm->tm_mday) {
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
220)                            hide.date = hide.wday = 1;
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
221)                    } else if (tm->tm_mday + 5 > human_tm->tm_mday) {
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
223)                            hide.date = 1;
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
231)            gettimeofday(&now, NULL);
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
232)            show_date_relative(time, tz, &now, buf);
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
233)            return;
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
246)            hide.seconds = 1;
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
247)            hide.tz |= !hide.date;
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
248)            hide.wday = hide.time = !hide.year;
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
262)            strbuf_rtrim(buf);
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
287)            gettimeofday(&now, NULL);
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
290)            human_tz = local_time_tzoffset(now.tv_sec, &human_tm);
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 886)static int 
auto_date_style(void)
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 888)    return 
(isatty(1) || pager_in_use()) ? DATE_HUMAN : DATE_NORMAL;
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
909)            return DATE_HUMAN;
74e8221b523     (Linus Torvalds 2018-07-07 15:02:35 -0700 
911)            return auto_date_style();
diff-lib.c
fa49029577c     (Alex Vandiver  2018-01-02 19:04:56 -0800 
205)                    continue;
list-objects-filter.c
dbebce653b5     (Matthew DeVore 2018-09-04 11:05:49 -0700 
47)             BUG("unknown filter_situation: %d", filter_situation);
7329b7c6cdc     (Matthew DeVore 2018-09-04 11:05:50 -0700 100)    default:
7329b7c6cdc     (Matthew DeVore 2018-09-04 11:05:50 -0700 
101)            BUG("unknown filter_situation: %d", filter_situation);
dbebce653b5     (Matthew DeVore 2018-09-04 11:05:49 -0700 
152)            BUG("unknown filter_situation: %d", filter_situation);
dbebce653b5     (Matthew DeVore 2018-09-04 11:05:49 -0700 
257)            BUG("unknown filter_situation: %d", filter_situation);
dbebce653b5     (Matthew DeVore 2018-09-04 11:05:49 -0700 
438)            BUG("invalid list-objects filter choice: %d",
list-objects.c
f447a499dbb     (Matthew DeVore 2018-08-13 11:14:28 -0700 
197)                    ctx->show_object(obj, base->buf, ctx->show_data);
rebase-interactive.c
64a43cbd5da     (Alban Gruin    2018-08-10 18:51:31 +0200 
61)             return error_errno(_("could not read '%s'."), todo_file);
64a43cbd5da     (Alban Gruin    2018-08-10 18:51:31 +0200 
65)             strbuf_release(&buf);
64a43cbd5da     (Alban Gruin    2018-08-10 18:51:31 +0200 
66)             return -1;
a9f5476fbca     (Alban Gruin    2018-08-10 18:51:35 +0200 
74)             return error_errno(_("could not read '%s'."), todo_file);
a9f5476fbca     (Alban Gruin    2018-08-10 18:51:35 +0200 
78)             strbuf_release(&buf);
a9f5476fbca     (Alban Gruin    2018-08-10 18:51:35 +0200 
79)             return -1;
64a43cbd5da     (Alban Gruin    2018-08-10 18:51:31 +0200 
85)             return -1;
refs.c
4a6067cda51     (Stefan Beller  2018-08-20 18:24:16 +0000 
1405)           return 0;
sequencer.c
65850686cf0     (Alban Gruin    2018-08-28 14:10:40 +0200 
2276)           return;
65850686cf0     (Alban Gruin    2018-08-28 14:10:40 +0200 
2373)           write_file(rebase_path_quiet(), "%s\n", quiet);
2c58483a598     (Alban Gruin    2018-08-10 18:51:33 +0200 
3371)                   return error(_("could not checkout %s"), commit);
4df66c40b08     (Alban Gruin    2018-08-10 18:51:34 +0200 
3385)           return error(_("%s: not a valid OID"), orig_head);
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4750)           return -1;
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4753)           return -1;
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4759)           return error_errno(_("could not read '%s'."), todo_file);
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4762)           todo_list_release(&todo_list);
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4763)           return error(_("unusable todo list: '%s'"), todo_file);
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4782)           todo_list_release(&todo_list);
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4783)           return -1;
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4787)           return error(_("could not copy '%s' to '%s'."), todo_file,
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4791)           return error(_("could not transform the todo list"));
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4820)           return error(_("could not transform the todo list"));
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4823)           return error(_("could not skip unnecessary pick commands"));
b97e1873649     (Alban Gruin    2018-08-28 14:10:36 +0200 
4829)           return -1;
strbuf.c
f95736288a3     (Pratik Karki   2018-08-08 20:51:16 +0545 
127)                    --sb->len;
submodule-config.c
83a66534e63     (Antonio Ospite 2018-08-14 13:05:19 +0200 
714)            return CONFIG_INVALID_KEY;
150984f5339     (Antonio Ospite 2018-08-14 13:05:20 +0200 
729)            warning(_("Could not update .gitmodules entry %s"), key);
t/helper/test-dump-fsmonitor.c
6e1123ec573     (Alex Vandiver  2018-01-02 19:04:54 -0800 
25)                     valid++;
t/helper/test-repository.c
b7758963424     (Derrick Stolee 2018-08-20 18:24:24 +0000 
21)             die("Couldn't init repo");
b7758963424     (Derrick Stolee 2018-08-20 18:24:24 +0000 
47)             die("Couldn't init repo");
wrapper.c
7e621449185     (Pranit Bauva   2017-10-27 15:06:37 +0000 
701)            die_errno(_("could not stat %s"), filename);
wt-status.c
f3bd35fa0dd     (Stephen P. Smith       2018-09-05 17:53:29 -0700       
671)                    s->committable = 1;
1c623c066c2     (Junio C Hamano 2018-09-07 15:32:24 -0700 
1949)                   if (s->state.rebase_in_progress ||
1c623c066c2     (Junio C Hamano 2018-09-07 15:32:24 -0700 1950) 
s->state.rebase_interactive_in_progress)
1c623c066c2     (Junio C Hamano 2018-09-07 15:32:24 -0700 
1951)                           branch_name = s->state.onto;
1c623c066c2     (Junio C Hamano 2018-09-07 15:32:24 -0700 
1952)                   else if (s->state.detached_from)
1c623c066c2     (Junio C Hamano 2018-09-07 15:32:24 -0700 
1953)                           branch_name = s->state.detached_from;


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/1] contrib: Add script to show uncovered "new" lines
  2018-09-12 16:45 [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
  2018-09-12 16:45 ` [PATCH 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
  2018-09-12 19:14 ` [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee
@ 2018-09-12 19:33 ` Ben Peart
  2018-09-12 19:53 ` Junio C Hamano
  2018-09-13 14:56 ` [PATCH v2 " Derrick Stolee via GitGitGadget
  4 siblings, 0 replies; 22+ messages in thread
From: Ben Peart @ 2018-09-12 19:33 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget, git; +Cc: peff, Junio C Hamano



On 9/12/2018 12:45 PM, Derrick Stolee via GitGitGadget wrote:
> We have coverage targets in our Makefile for using gcov to display line
> coverage based on our test suite. The way I like to do it is to run:
> 
> make coverage-test
> make coverage-report
> 

Very nice, I was unaware of the coverage test make targets.  I like the 
new report; it makes it easier to verify any new changes are actually 
tested.

>      
>   4. The lines in read-cache.c are part of a new block for the condition "if
>      (expand_name_field)" as part of an optimization. These lines should
>      probably be covered before that series is merged to 'next'. I understand
>      that Ben and Duy are continuing work in this direction [1].
>      

This code is only exercised when the index format is V4 but the default 
is version 2/3 [1].  To enable the test suite to use version 4 and test 
those code paths will require the addition of a new 
GIT_TEST_INDEX_VERSION environment variable.  I'll add that to my TODO list.

[1] https://git-scm.com/docs/git/2.1.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/1] contrib: Add script to show uncovered "new" lines
  2018-09-12 16:45 [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
                   ` (2 preceding siblings ...)
  2018-09-12 19:33 ` Ben Peart
@ 2018-09-12 19:53 ` Junio C Hamano
  2018-09-13 14:56 ` [PATCH v2 " Derrick Stolee via GitGitGadget
  4 siblings, 0 replies; 22+ messages in thread
From: Junio C Hamano @ 2018-09-12 19:53 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget; +Cc: git, peff

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> There have been a few bugs in recent patches what would have been caught if
> the test suite covered those blocks (including a few of mine). I want to
> work towards a "sensible" amount of coverage on new topics. In my opinion,
> this means that any logic should be covered, but the 'die()' blocks in error
> cases do not need to be covered.

Nice.

> It is important to not measure the coverage of the codebase by what old code
> is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
> After creating the coverage statistics at a version (say, 'topic') you can
> then run
>
> contrib/coverage-diff.sh base topic
> ...
> Using this 'git blame' output, we can quickly inspect whether the uncovered
> lines are appropriate. For instance:
> ...
> I used this approach for 'next' over 'master' and got a larger list, some of
> which I have already submitted tests to increase coverage [2] or will be
> covered by topics not in 'next' [3].
>
> Thanks, -Stolee

Thanks for working on this.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/1] contrib: add coverage-diff script
  2018-09-12 16:45 ` [PATCH 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
@ 2018-09-12 22:13   ` Junio C Hamano
  2018-09-12 22:54     ` Junio C Hamano
  0 siblings, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2018-09-12 22:13 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget; +Cc: git, peff, Derrick Stolee

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  contrib/coverage-diff.sh | 70 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100755 contrib/coverage-diff.sh

I fully appreciate the motivation.  But it is a bit sad that this
begins with "#!/bin/bash" but it seems that the script is full of
bash-isms.  I haven't gone through the script to see if these are
inevitable or gratuitous yet, but I'd assume it made it easier for
you to write it to step outside the pure POSIX shell?

> +V1=$1
> +V2=$2
> +
> +diff-lines() {

Being able to use '-' in identifier is probably a bash-ism that you
did not have to use.

> +    local path=
> +    local line=
> +    while read; do

Being able to omit variable to be read into and can use the implicit
variable $REPLY also is.

> +	esc=$'\033'
> +	if [[ $REPLY =~ ---\ (a/)?.* ]]; then
> +	    continue
> +	elif [[ $REPLY =~ \+\+\+\ (b/)?([^[:blank:]$esc]+).* ]]; then
> +	    path=${BASH_REMATCH[2]}

OK, it probably is easier to write in bash than using expr if you
want to do regexp.  Where do these escape code come from in "git
diff" output, by the way?

> +	elif [[ $REPLY =~ @@\ -[0-9]+(,[0-9]+)?\ \+([0-9]+)(,[0-9]+)?\ @@.* ]]; then
> +	    line=${BASH_REMATCH[2]}
> +	elif [[ $REPLY =~ ^($esc\[[0-9;]+m)*([\ +-]) ]]; then
> +	    echo "$path:$line:$REPLY"
> +	    if [[ ${BASH_REMATCH[2]} != - ]]; then
> +		((line++))
> +	    fi
> +	fi
> +    done
> +}
> +
> +git diff --raw $V1 $V2 | grep \.c$ | awk 'NF>1{print $NF}' >files.txt

Hmph, not 

	git diff --name-only "$V1" "$V2" -- "*.c"

Do we (or do we not) want "--no-renames"?

> +for file in $(cat files.txt)
> +do
> +	hash_file=${file//\//\#}
> +
> +	git diff $V1 $V2 -- $file \
> +		| diff-lines \
> +		| grep ":+" \
> +		>"diff_file.txt"

Style:

	cmd1 |
	cmd2 |
	cmd3 >output

is easier to read without backslashes.

> +	cat diff_file.txt \
> +		| sed -E 's/:/ /g' \
> +		| awk '{print $2}' \
> +		| sort \
> +		>new_lines.txt
> +
> +	cat "$hash_file.gcov" \
> +		| grep \#\#\#\#\# \
> +		| sed 's/    #####: //g' \
> +		| sed 's/\:/ /g' \
> +		| awk '{print $1}' \
> +		| sort \
> +		>uncovered_lines.txt

OK, so we assume that we have run coverage in $V2 checkout so that
we can pick up the postimage line numbers in "diff $V1 $V2" and find
corresponding record in .gcov file in the filesystem.  I did not
realize the significance of 'topic' being the later argument to the
script in this part

    After creating the coverage statistics at a version (say,
    'topic') you can then run

        contrib/coverage-diff.sh base topic

of your description before I see this implementation.  Also the
comment at the beginning

    # Usage: 'contrib/coverage-diff.sh <version1> <version2>
    # Outputs a list of new lines in version2 compared to version1 that are
    # not covered by the test suite. Assumes you ran
    # 'make coverage-test coverage-report' from root first, so .gcov files exist.

would want to make it clear that we want coverage run from root
for version2 before using this script.

> +	comm -12 uncovered_lines.txt new_lines.txt \
> +		| sed -e 's/$/\)/' \
> +		| sed -e 's/^/\t/' \
> +		>uncovered_new_lines.txt
> +
> +	grep -q '[^[:space:]]' < uncovered_new_lines.txt && \

Style: when you end a line with && (or || or | for that matter), the
shell knows that you have not finished speaking, and will wait to
listen to you to finish the sentence.  No need for backslash there.

> +		echo $file && \
> +		git blame -c $file \
> +			| grep -f uncovered_new_lines.txt
> +
> +	rm -f diff_file.txt new_lines.txt \
> +		uncovered_lines.txt uncovered_new_lines.txt
> +done
> +
> +rm -rf files.txt

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/1] contrib: add coverage-diff script
  2018-09-12 22:13   ` Junio C Hamano
@ 2018-09-12 22:54     ` Junio C Hamano
  2018-09-13 12:21       ` Derrick Stolee
  0 siblings, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2018-09-12 22:54 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget; +Cc: git, peff, Derrick Stolee

Junio C Hamano <gitster@pobox.com> writes:

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>>  contrib/coverage-diff.sh | 70 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 70 insertions(+)
>>  create mode 100755 contrib/coverage-diff.sh
>
> I fully appreciate the motivation.  But it is a bit sad that this
> begins with "#!/bin/bash" but it seems that the script is full of
> bash-isms.  I haven't gone through the script to see if these are
> inevitable or gratuitous yet, but I'd assume it made it easier for
> you to write it to step outside the pure POSIX shell?
> ...
>> +	elif [[ $REPLY =~ \+\+\+\ (b/)?([^[:blank:]$esc]+).* ]]; then
>> +	    path=${BASH_REMATCH[2]}
>
> OK, it probably is easier to write in bash than using expr if you
> want to do regexp.

Just to clarify. I am saying that it is OK to give up writing in
pure POSIX and relying on bash-isms after seeing these lines.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/1] contrib: add coverage-diff script
  2018-09-12 22:54     ` Junio C Hamano
@ 2018-09-13 12:21       ` Derrick Stolee
  2018-09-13 14:59         ` Junio C Hamano
  0 siblings, 1 reply; 22+ messages in thread
From: Derrick Stolee @ 2018-09-13 12:21 UTC (permalink / raw)
  To: Junio C Hamano, Derrick Stolee via GitGitGadget; +Cc: git, peff, Derrick Stolee

On 9/12/2018 6:54 PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>>   contrib/coverage-diff.sh | 70 ++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 70 insertions(+)
>>>   create mode 100755 contrib/coverage-diff.sh
>> I fully appreciate the motivation.  But it is a bit sad that this
>> begins with "#!/bin/bash" but it seems that the script is full of
>> bash-isms.  I haven't gone through the script to see if these are
>> inevitable or gratuitous yet, but I'd assume it made it easier for
>> you to write it to step outside the pure POSIX shell?

I completely forgot to avoid bash, as I wrote this first as an experiment.

>> ...
>>> +	elif [[ $REPLY =~ \+\+\+\ (b/)?([^[:blank:]$esc]+).* ]]; then
>>> +	    path=${BASH_REMATCH[2]}
>> OK, it probably is easier to write in bash than using expr if you
>> want to do regexp.
> Just to clarify. I am saying that it is OK to give up writing in
> pure POSIX and relying on bash-isms after seeing these lines.

I'll try rewriting it using POSIX shell and see how hard it is.

Thanks,

-Stolee


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 0/1] contrib: Add script to show uncovered "new" lines
  2018-09-12 16:45 [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
                   ` (3 preceding siblings ...)
  2018-09-12 19:53 ` Junio C Hamano
@ 2018-09-13 14:56 ` " Derrick Stolee via GitGitGadget
  2018-09-13 14:56   ` [PATCH v2 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
  2018-09-21 15:15   ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
  4 siblings, 2 replies; 22+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2018-09-13 14:56 UTC (permalink / raw)
  To: git; +Cc: peff, Junio C Hamano

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

make coverage-test
make coverage-report

This leaves the repo in a state where every X.c file that was covered has an
X.c.gcov file containing the coverage counts for every line, and "#####" at
every uncovered line.

There have been a few bugs in recent patches what would have been caught if
the test suite covered those blocks (including a few of mine). I want to
work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks in error
cases do not need to be covered.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by
the test suite. For example, I ran this against the 'jch' branch (d3c0046)
versus 'next' (dd90340) and got the following output:

builtin/commit.c
859fdc0c3cf     (Derrick Stolee 2018-08-29 05:49:04 -0700       1657)           write_commit_graph_reachable(get_object_directory(), 0);
builtin/rev-list.c
250edfa8c87     (Harald Nordgren        2018-04-18 23:05:35 +0200       431)                    bisect_flags |= BISECT_FIND_ALL;
builtin/worktree.c
e5353bef550     (Eric Sunshine  2018-08-28 17:20:19 -0400       60)             error_errno(_("failed to delete '%s'"), sb.buf);
e19831c94f9     (Eric Sunshine  2018-08-28 17:20:23 -0400       251)                die(_("unable to re-add worktree '%s'"), path);
68a6b3a1bd4     (Eric Sunshine  2018-08-28 17:20:24 -0400       793)                    die(_("cannot move a locked working tree, lock reason: %s\nuse 'move -f -f' to override or unlock first"),
f4143101cbb     (Eric Sunshine  2018-08-28 17:20:25 -0400       906)                    die(_("cannot remove a locked working tree, lock reason: %s\nuse 'remove -f -f' to override or unlock first"),
read-cache.c
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1754)           const unsigned char *cp = (const unsigned char *)name;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1757)           previous_len = previous_ce ? previous_ce->ce_namelen : 0;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1758)           strip_len = decode_varint(&cp);
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1759)           if (previous_len < strip_len) {
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1760)                   if (previous_ce)
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1761)                           die(_("malformed name field in the index, near path '%s'"),
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1762)                               previous_ce->name);
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1764)                           die(_("malformed name field in the index in the first path"));
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1766)           copy_len = previous_len - strip_len;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1767)           name = (const char *)cp;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1773)                   len += copy_len;
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1794)           if (copy_len)
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1795)                   memcpy(ce->name, previous_ce->name, copy_len);
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1796)           memcpy(ce->name + copy_len, name, len + 1 - copy_len);
67922abbbb3     (Nguyễn Thái Ngọc Duy   2018-09-02 15:19:33 +0200       1797)           *ent_size = (name - ((char *)ondisk)) + len + 1 - copy_len;
remote-curl.c
c3b9bc94b9b     (Elijah Newren  2018-09-05 10:03:07 -0700       181)            options.filter = xstrdup(value);

Using this 'git blame' output, we can quickly inspect whether the uncovered
lines are appropriate. For instance:

 1. The line in builtin/commit.c is due to writing the commit-graph file
    when GIT_TEST_COMMIT_GRAPH is enabled, which is not on by default in the
    test suite. Being uncovered is expected here.
    
    
 2. The lines in builtin/worktree.c are all related to error conditions.
    This is acceptable.
    
    
 3. The line in builtin/rev-list.c is a flag replacement in a block that is
    otherwise unchanged. It must not be covered by the test suite normally.
    This could be worth adding a test to ensure the new logic maintains old
    behavior.
    
    
 4. The lines in read-cache.c are part of a new block for the condition "if
    (expand_name_field)" as part of an optimization. These lines should
    probably be covered before that series is merged to 'next'. I understand
    that Ben and Duy are continuing work in this direction [1].
    
    

I used this approach for 'next' over 'master' and got a larger list, some of
which I have already submitted tests to increase coverage [2] or will be
covered by topics not in 'next' [3].

Thanks, -Stolee

CHANGES IN V2: I converted the script from bash to sh (there may still be
POSIX-compliance issues with the new script, I don't know a lot when it
comes to that area). I also streamlined the machinery to add line numbers to
the diff. One downside is that the script runs a little slower with all of
the grep process invocations.

[1] 
https://public-inbox.org/git/20180912161832.55324-1-benpeart@microsoft.com/T/#u

[2] https://public-inbox.org/git/pull.37.git.gitgitgadget@gmail.com/

[3] https://public-inbox.org/git/pull.34.git.gitgitgadget@gmail.com/

Derrick Stolee (1):
  contrib: add coverage-diff script

 contrib/coverage-diff.sh | 63 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100755 contrib/coverage-diff.sh


base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-40%2Fderrickstolee%2Fcoverage-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-40/derrickstolee/coverage-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/40

Range-diff vs v1:

 1:  e4124471e5 < -:  ---------- contrib: add coverage-diff script
 -:  ---------- > 1:  7714b0659e contrib: add coverage-diff script

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/1] contrib: add coverage-diff script
  2018-09-13 14:56 ` [PATCH v2 " Derrick Stolee via GitGitGadget
@ 2018-09-13 14:56   ` Derrick Stolee via GitGitGadget
  2018-09-13 17:40     ` Junio C Hamano
  2018-09-13 17:54     ` Eric Sunshine
  2018-09-21 15:15   ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
  1 sibling, 2 replies; 22+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2018-09-13 14:56 UTC (permalink / raw)
  To: git; +Cc: peff, Junio C Hamano, Derrick Stolee

From: Derrick Stolee <dstolee@microsoft.com>

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

    make coverage-test
    make coverage-report

This leaves the repo in a state where every X.c file that was covered has
an X.c.gcov file containing the coverage counts for every line, and "#####"
at every uncovered line.

There have been a few bugs in recent patches what would have been caught
if the test suite covered those blocks (including a few of mine). I want
to work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks in error
cases do not need to be covered.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

    contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by the
test suite. The output uses 'git blame -c' format so you can find the commits
responsible and view the line numbers for quick access to the context.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 contrib/coverage-diff.sh | 63 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100755 contrib/coverage-diff.sh

diff --git a/contrib/coverage-diff.sh b/contrib/coverage-diff.sh
new file mode 100755
index 0000000000..0f226f038c
--- /dev/null
+++ b/contrib/coverage-diff.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+# Usage: 'contrib/coverage-diff.sh <version1> <version2>
+# Outputs a list of new lines in version2 compared to version1 that are
+# not covered by the test suite. Assumes you ran
+# 'make coverage-test coverage-report' from root first, so .gcov files exist.
+
+V1=$1
+V2=$2
+
+diff_lines () {
+	while read line
+	do
+		if echo $line | grep -q -e "^@@ -([0-9]+)(,[0-9]+)? \\+([0-9]+)(,[0-9]+)? @@.*"
+		then
+			line_num=$(echo $line \
+				| awk 'match($0, "@@ -([0-9]+)(,[0-9]+)? \\+([0-9]+)(,[0-9]+)? @@.*", m) { print m[3] }')
+		else
+			echo "$line_num:$line"
+			if ! echo $line | grep -q -e "^-"
+			then
+				line_num=$(($line_num + 1))
+			fi
+		fi
+	done
+}
+
+files=$(git diff --raw $V1 $V2 \
+	| grep \.c$ \
+	| awk 'NF>1{print $NF}')
+
+for file in $files
+do
+	git diff $V1 $V2 -- $file \
+		| diff_lines \
+		| grep ":+" \
+		| sed 's/:/ /g' \
+		| awk '{print $1}' \
+		| sort \
+		>new_lines.txt
+
+	hash_file=$(echo $file | sed "s/\//\#/")
+	cat "$hash_file.gcov" \
+		| grep \#\#\#\#\# \
+		| sed 's/    #####: //g' \
+		| sed 's/\:/ /g' \
+		| awk '{print $1}' \
+		| sort \
+		>uncovered_lines.txt
+
+	comm -12 uncovered_lines.txt new_lines.txt \
+		| sed -e 's/$/\)/' \
+		| sed -e 's/^/\t/' \
+		>uncovered_new_lines.txt
+
+	grep -q '[^[:space:]]' < uncovered_new_lines.txt && \
+		echo $file && \
+		git blame -c $file \
+			| grep -f uncovered_new_lines.txt
+
+	rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt
+done
+
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/1] contrib: add coverage-diff script
  2018-09-13 12:21       ` Derrick Stolee
@ 2018-09-13 14:59         ` Junio C Hamano
  0 siblings, 0 replies; 22+ messages in thread
From: Junio C Hamano @ 2018-09-13 14:59 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: Derrick Stolee via GitGitGadget, git, peff, Derrick Stolee

Derrick Stolee <stolee@gmail.com> writes:

> On 9/12/2018 6:54 PM, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>>
>>>>   contrib/coverage-diff.sh | 70 ++++++++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 70 insertions(+)
>>>>   create mode 100755 contrib/coverage-diff.sh
>>> I fully appreciate the motivation.  But it is a bit sad that this
>>> begins with "#!/bin/bash" but it seems that the script is full of
>>> bash-isms.  I haven't gone through the script to see if these are
>>> inevitable or gratuitous yet, but I'd assume it made it easier for
>>> you to write it to step outside the pure POSIX shell?
>
> I completely forgot to avoid bash, as I wrote this first as an experiment.
>
>>> ...
>>>> +	elif [[ $REPLY =~ \+\+\+\ (b/)?([^[:blank:]$esc]+).* ]]; then
>>>> +	    path=${BASH_REMATCH[2]}
>>> OK, it probably is easier to write in bash than using expr if you
>>> want to do regexp.
>> Just to clarify. I am saying that it is OK to give up writing in
>> pure POSIX and relying on bash-isms after seeing these lines.
>
> I'll try rewriting it using POSIX shell and see how hard it is.

Thanks.  Don't waste too much time on it and try to bend backwards
too far, though.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/1] contrib: add coverage-diff script
  2018-09-13 14:56   ` [PATCH v2 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
@ 2018-09-13 17:40     ` Junio C Hamano
  2018-09-13 18:15       ` Derrick Stolee
  2018-09-13 17:54     ` Eric Sunshine
  1 sibling, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2018-09-13 17:40 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget; +Cc: git, peff, Derrick Stolee

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> We have coverage targets in our Makefile for using gcov to display line
> coverage based on our test suite. The way I like to do it is to run:
>
>     make coverage-test
>     make coverage-report
>
> This leaves the repo in a state where every X.c file that was covered has
> an X.c.gcov file containing the coverage counts for every line, and "#####"
> at every uncovered line.
>
> There have been a few bugs in recent patches what would have been caught
> if the test suite covered those blocks (including a few of mine). I want
> to work towards a "sensible" amount of coverage on new topics. In my opinion,
> this means that any logic should be covered, but the 'die()' blocks in error
> cases do not need to be covered.
>
> It is important to not measure the coverage of the codebase by what old code
> is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
> After creating the coverage statistics at a version (say, 'topic') you can
> then run
>
>     contrib/coverage-diff.sh base topic
>
> to see the lines added between 'base' and 'topic' that are not covered by the
> test suite. The output uses 'git blame -c' format so you can find the commits
> responsible and view the line numbers for quick access to the context.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  contrib/coverage-diff.sh | 63 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100755 contrib/coverage-diff.sh
>
> diff --git a/contrib/coverage-diff.sh b/contrib/coverage-diff.sh
> new file mode 100755
> index 0000000000..0f226f038c
> --- /dev/null
> +++ b/contrib/coverage-diff.sh
> @@ -0,0 +1,63 @@
> +#!/bin/sh
> +
> +# Usage: 'contrib/coverage-diff.sh <version1> <version2>
> +# Outputs a list of new lines in version2 compared to version1 that are
> +# not covered by the test suite. Assumes you ran
> +# 'make coverage-test coverage-report' from root first, so .gcov files exist.

I presume that it is "from root first while you have a checkout of
version2, so .gcov files for version2 exist in the working tree"?

> +V1=$1
> +V2=$2
> +
> +diff_lines () {
> +	while read line
> +	do
> +		if echo $line | grep -q -e "^@@ -([0-9]+)(,[0-9]+)? \\+([0-9]+)(,[0-9]+)? @@.*"

As you know you are reading from diff output, you do not have to be
so strict in this if condition.  It's not like you try to carefully
reject a line that began with "@@" but did not match this pattern in
the corresponding else block.

"read line" does funny things to backslashes and whitespaces in the
payload ("read -r line" sometimes helps), and echoing $line without
quoting will destroy its contents here and in the line below (but
you do not care because all you care about is if it begins with a
dash).

> +		then
> +			line_num=$(echo $line \
> +				| awk 'match($0, "@@ -([0-9]+)(,[0-9]+)? \\+([0-9]+)(,[0-9]+)? @@.*", m) { print m[3] }')
> +		else
> +			echo "$line_num:$line"
> +			if ! echo $line | grep -q -e "^-"

If a patch removes a line with a solitary 'n' on it, possibly
followed by a SP and something else, such a line would say "-n
something else", and some implementation of "echo -n something else"
would do what this line does not expect.  A safer way to do this,
as you only care if it is a deletion line, is to do

	case "$line" in -*) ;; *) line_num=$(( $line_num + 1 ));; esac

Also you can make the echoing of "$line_num:$line" above
conditional, which will allow you to lose "grep ':+'" in the
pipeline that consumes output from this thing, e.g.

	case "$line" in +*) echo "$line_num:$line";; esac

iff you must write this in shell (but see below).

> +			then
> +				line_num=$(($line_num + 1))
> +			fi
> +		fi
> +	done

I have a feeling that a single Perl script instead of a shell loop
that runs many grep and awk as subprocesses performs better even on
Windows, and it would be more readable and maintainable.

perl -e '
	my $line_num;
	while (<>) {
		# Hunk header?  Grab the beginning in postimage.
		if (/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/) {
			$line_num = $1;
			next;
		}

		# Have we seen a hunk?  Ignore "diff --git" etc.
		next unless defined $line_num;

		# Deleted line? Ignore.
		if (/^-/) {
			next;
		}

		# Show only the line number of added lines.
		if (/^\+/) {
			print "$line_num\n";
		}
		# Either common context or added line appear in
		# the postimage.  Count it.
		$line_num++;
	}
'

or something like that, given that you seem to only need line
numbers in new_lines.txt anyway?

> +}
> +
> +files=$(git diff --raw $V1 $V2 \
> +	| grep \.c$ \
> +	| awk 'NF>1{print $NF}')

Do we need these other commands in the pipe?  How is this different
from, say,

	git diff --name-only "$V1" "$V2" -- \*.c

?

> +for file in $files
> +do
> +	git diff $V1 $V2 -- $file \
> +		| diff_lines \
> +		| grep ":+" \

I think you meant to match "^<linenum>:+<added contents>" you are
echoing out in the above helper function, but this will find a
removed line that happens to have colon followed by a plus (which is
not all that uncommon in a patch to a shell script).

> +		| sed 's/:/ /g' \

Turning "<linenum>:+<added contents>" to "<linenum> +<added contents>"
with this, so that the next "awk" can show <linenum> only?  Then you
do not need 'g' at the end.  I see you use 'g' in many sed scripts
for uncovered_lines.txt, and I suspect most of 'g's you do not mean.

> +		| awk '{print $1}' \

Then we get a list of $line_num here?

> +		| sort \

This is sorted textually, which goes against intuition because these
lines are all line numbers, but it is done so that you can run
"comm" on it later and "comm" requires us to feed lines sorted
textually.

> +		>new_lines.txt

If I were writing this part, I'd probably write a small Perl script
that takes output from 'git diff "$V1" "$V2" -- "$file"' and then
reduces it to a list of line numbers for newly introduced lines.
Then pipe it to "sort >new_lines.txt" to match what we see below.

> +	hash_file=$(echo $file | sed "s/\//\#/")
> +	cat "$hash_file.gcov" \
> +		| grep \#\#\#\#\# \
> +		| sed 's/    #####: //g' \
> +		| sed 's/\:/ /g' \
> +		| awk '{print $1}' \
> +		| sort \
> +		>uncovered_lines.txt

Do not cat a single regular file into a pipe.  Write pipe at the end
of the line, not the beginning of next line.  Do not grep into sed.
Do not feed sed into sed unless needed.

Without knowing what the above is trying to do, but just
mechanically translating, I'd get something like this:

	sed -ne '/#####/{
		s/    #####: //g
		s/:.*//
                p
	}' "$hash_file.gcov" |
	sort >uncovered_lines.txt

> +	comm -12 uncovered_lines.txt new_lines.txt \
> +		| sed -e 's/$/\)/' \
> +		| sed -e 's/^/\t/' \
> +		>uncovered_new_lines.txt

;-)  This creates something like

	1)
	2)
	3)

out of a list of numbers.  Cute.

> +	grep -q '[^[:space:]]' < uncovered_new_lines.txt && \
> +		echo $file && \
> +		git blame -c $file \
> +			| grep -f uncovered_new_lines.txt
> +
> +	rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt
> +done
> +

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/1] contrib: add coverage-diff script
  2018-09-13 14:56   ` [PATCH v2 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
  2018-09-13 17:40     ` Junio C Hamano
@ 2018-09-13 17:54     ` Eric Sunshine
  1 sibling, 0 replies; 22+ messages in thread
From: Eric Sunshine @ 2018-09-13 17:54 UTC (permalink / raw)
  To: gitgitgadget; +Cc: Git List, Jeff King, Junio C Hamano, Derrick Stolee

On Thu, Sep 13, 2018 at 10:56 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> There have been a few bugs in recent patches what would have been caught
> if the test suite covered those blocks (including a few of mine). I want
> to work towards a "sensible" amount of coverage on new topics. In my opinion,
> this means that any logic should be covered, but the 'die()' blocks in error
> cases do not need to be covered.

The bit about die() blocks is perhaps a bit too general. While it's
true that some die()'s signal very unlikely (or near-impossible)
conditions, others are merely reporting invalid user or other input to
the program. The latter category is often very much worth testing, as
the number of test_must_fail() invocations in the test suite shows.
68a6b3a1bd (worktree: teach 'move' to override lock when --force given
twice, 2018-08-28), which was highlighted in your cover letter,
provides a good example of legitimately testing that a die() is
covered. So, perhaps the above can be toned-down a bit by saying
something like:

    ...but die() blocks covering very unlikely (or near-impossible)
    situations may not warrant coverage.

> It is important to not measure the coverage of the codebase by what old code
> is not covered.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/1] contrib: add coverage-diff script
  2018-09-13 17:40     ` Junio C Hamano
@ 2018-09-13 18:15       ` Derrick Stolee
  0 siblings, 0 replies; 22+ messages in thread
From: Derrick Stolee @ 2018-09-13 18:15 UTC (permalink / raw)
  To: Junio C Hamano, Derrick Stolee via GitGitGadget; +Cc: git, peff, Derrick Stolee

On 9/13/2018 1:40 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> +			then
>> +				line_num=$(($line_num + 1))
>> +			fi
>> +		fi
>> +	done
> I have a feeling that a single Perl script instead of a shell loop
> that runs many grep and awk as subprocesses performs better even on
> Windows, and it would be more readable and maintainable.
>
> perl -e '
> 	my $line_num;
> 	while (<>) {
> 		# Hunk header?  Grab the beginning in postimage.
> 		if (/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/) {
> 			$line_num = $1;
> 			next;
> 		}
>
> 		# Have we seen a hunk?  Ignore "diff --git" etc.
> 		next unless defined $line_num;
>
> 		# Deleted line? Ignore.
> 		if (/^-/) {
> 			next;
> 		}
>
> 		# Show only the line number of added lines.
> 		if (/^\+/) {
> 			print "$line_num\n";
> 		}
> 		# Either common context or added line appear in
> 		# the postimage.  Count it.
> 		$line_num++;
> 	}
> '
>
> or something like that, given that you seem to only need line
> numbers in new_lines.txt anyway?

Thanks for the deep dive here, especially with the perl assistance. I've 
never written any perl, but it seems like the right tool here. I'll have 
time to revisit this next week.

Thanks,

-Stolee


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines
  2018-09-13 14:56 ` [PATCH v2 " Derrick Stolee via GitGitGadget
  2018-09-13 14:56   ` [PATCH v2 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
@ 2018-09-21 15:15   ` Derrick Stolee via GitGitGadget
  2018-09-21 15:15     ` [PATCH v3 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
                       ` (2 more replies)
  1 sibling, 3 replies; 22+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2018-09-21 15:15 UTC (permalink / raw)
  To: git; +Cc: peff, Junio C Hamano

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

make coverage-test
make coverage-report

This leaves the repo in a state where every X.c file that was covered has an
X.c.gcov file containing the coverage counts for every line, and "#####" at
every uncovered line.

There have been a few bugs in recent patches what would have been caught if
the test suite covered those blocks (including a few of mine). I want to
work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks in error
cases do not need to be covered.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by
the test suite. For example, I ran this against the 'next' branch (22e244b)
versus 'master' (150f307) and got the following output:

fsck.c
fb8952077df     (René Scharfe   2018-09-03 14:49:26 +0000       212)            die_errno("Could not read '%s'", path);
list-objects-filter-options.c
f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       56)                     if (errbuf) {
f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       57)                             strbuf_init(errbuf, 0);
f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       58)                             strbuf_addstr(
f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       62)                     return 1;
list-objects-filter.c
77d7a65d502     (Matthew DeVore 2018-09-13 17:55:26 -0700       47)             BUG("unknown filter_situation: %d", filter_situation);
f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       100)    default:
f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       101)            BUG("unknown filter_situation: %d", filter_situation);
77d7a65d502     (Matthew DeVore 2018-09-13 17:55:26 -0700       152)            BUG("unknown filter_situation: %d", filter_situation);
77d7a65d502     (Matthew DeVore 2018-09-13 17:55:26 -0700       257)            BUG("unknown filter_situation: %d", filter_situation);
77d7a65d502     (Matthew DeVore 2018-09-13 17:55:26 -0700       438)            BUG("invalid list-objects filter choice: %d",
list-objects.c
f447a499dbb     (Matthew DeVore 2018-08-13 11:14:28 -0700       197)                    ctx->show_object(obj, base->buf, ctx->show_data);
ll-merge.c
d64324cb60e     (Torsten Bögershausen   2018-09-12 21:32:02 +0200       379)                    marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
midx.c
56ee7ff1565     (Derrick Stolee 2018-09-13 11:02:13 -0700       949)            return 0;
cc6af73c029     (Derrick Stolee 2018-09-13 11:02:25 -0700       990)                    midx_report(_("failed to load pack-index for packfile %s"),
cc6af73c029     (Derrick Stolee 2018-09-13 11:02:25 -0700       991)                                e.p->pack_name);
cc6af73c029     (Derrick Stolee 2018-09-13 11:02:25 -0700       992)                    break;
remote-curl.c
c3b9bc94b9b     (Elijah Newren  2018-09-05 10:03:07 -0700       181)            options.filter = xstrdup(value);
submodule.c
df255b8cac7     (Brandon Williams       2018-08-08 15:33:22 -0700       1738)           die(_("could not create directory '%s'"), new_gitdir.buf);

Using this 'git blame' output, we can quickly inspect whether the uncovered
lines are appropriate. For instance:

 1. The line in builtin/commit.c is due to writing the commit-graph file
    when GIT_TEST_COMMIT_GRAPH is enabled, which is not on by default in the
    test suite. Being uncovered is expected here.
    
    
 2. The lines in builtin/worktree.c are all related to error conditions.
    This is acceptable.
    
    
 3. The line in builtin/rev-list.c is a flag replacement in a block that is
    otherwise unchanged. It must not be covered by the test suite normally.
    This could be worth adding a test to ensure the new logic maintains old
    behavior.
    
    
 4. The lines in read-cache.c are part of a new block for the condition "if
    (expand_name_field)" as part of an optimization. These lines should
    probably be covered before that series is merged to 'next'. I understand
    that Ben and Duy are continuing work in this direction [1].
    
    

I used this approach for 'next' over 'master' and got a larger list, some of
which I have already submitted tests to increase coverage [2] or will be
covered by topics not in 'next' [3].

Thanks, -Stolee

CHANGES IN V3: I took Junio's perl script verbatim, which speeds up the
performance greatly. Some of the other sed commands needed some massaging,
but also added extra cleanup. Thanks for the help!

[1] 
https://public-inbox.org/git/20180912161832.55324-1-benpeart@microsoft.com/T/#u

[2] https://public-inbox.org/git/pull.37.git.gitgitgadget@gmail.com/

[3] https://public-inbox.org/git/pull.34.git.gitgitgadget@gmail.com/

Derrick Stolee (1):
  contrib: add coverage-diff script

 contrib/coverage-diff.sh | 79 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100755 contrib/coverage-diff.sh


base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-40%2Fderrickstolee%2Fcoverage-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-40/derrickstolee/coverage-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/40

Range-diff vs v2:

 1:  7714b0659e < -:  ---------- contrib: add coverage-diff script
 -:  ---------- > 1:  21214cc321 contrib: add coverage-diff script

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 1/1] contrib: add coverage-diff script
  2018-09-21 15:15   ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
@ 2018-09-21 15:15     ` Derrick Stolee via GitGitGadget
  2018-09-25 18:36       ` Junio C Hamano
  2018-09-21 15:20     ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee
  2018-10-08 14:52     ` [PATCH v4 " Derrick Stolee via GitGitGadget
  2 siblings, 1 reply; 22+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2018-09-21 15:15 UTC (permalink / raw)
  To: git; +Cc: peff, Junio C Hamano, Derrick Stolee

From: Derrick Stolee <dstolee@microsoft.com>

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

    make coverage-test
    make coverage-report

This leaves the repo in a state where every X.c file that was covered has
an X.c.gcov file containing the coverage counts for every line, and "#####"
at every uncovered line.

There have been a few bugs in recent patches what would have been caught
if the test suite covered those blocks (including a few of mine). I want
to work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks covering
very unlikely (or near-impossible) situations may not warrant coverage.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

    contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by the
test suite. The output uses 'git blame -c' format so you can find the commits
responsible and view the line numbers for quick access to the context.

Helped-by: Junio C Hamano <gister@pobox.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 contrib/coverage-diff.sh | 79 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100755 contrib/coverage-diff.sh

diff --git a/contrib/coverage-diff.sh b/contrib/coverage-diff.sh
new file mode 100755
index 0000000000..48b9a3ae96
--- /dev/null
+++ b/contrib/coverage-diff.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+# Usage: Run 'contrib/coverage-diff.sh <version1> <version2>' from source-root
+# after running
+#
+#     make coverage-test
+#     make coverage-report
+#
+# while checked out at <version2>. This script combines the *.gcov files
+# generated by the 'make' commands above with 'git diff <version1> <version2>'
+# to report new lines that are not covered by the test suite.
+
+V1=$1
+V2=$2
+
+diff_lines () {
+	perl -e '
+		my $line_num;
+		while (<>) {
+			# Hunk header?  Grab the beginning in postimage.
+			if (/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/) {
+				$line_num = $1;
+				next;
+			}
+
+			# Have we seen a hunk?  Ignore "diff --git" etc.
+			next unless defined $line_num;
+
+			# Deleted line? Ignore.
+			if (/^-/) {
+				next;
+			}
+
+			# Show only the line number of added lines.
+			if (/^\+/) {
+				print "$line_num\n";
+			}
+			# Either common context or added line appear in
+			# the postimage.  Count it.
+			$line_num++;
+		}
+	'
+}
+
+files=$(git diff --name-only $V1 $V2 -- *.c)
+
+for file in $files
+do
+	git diff $V1 $V2 -- $file \
+		| diff_lines \
+		| sort >new_lines.txt
+
+	if ! test -s new_lines.txt
+	then
+		continue
+	fi
+
+	hash_file=$(echo $file | sed "s/\//\#/")
+	sed -ne '/#####:/{
+			s/    #####://
+			s/:.*//
+			s/ //g
+			p
+		}' "$hash_file.gcov" \
+		| sort >uncovered_lines.txt
+
+	comm -12 uncovered_lines.txt new_lines.txt \
+		| sed -e 's/$/\)/' \
+		| sed -e 's/^/\t/' \
+		>uncovered_new_lines.txt
+
+	grep -q '[^[:space:]]' < uncovered_new_lines.txt && \
+		echo $file && \
+		git blame -c $file \
+			| grep -f uncovered_new_lines.txt
+
+	rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt
+done
+
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines
  2018-09-21 15:15   ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
  2018-09-21 15:15     ` [PATCH v3 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
@ 2018-09-21 15:20     ` Derrick Stolee
  2018-10-08 14:52     ` [PATCH v4 " Derrick Stolee via GitGitGadget
  2 siblings, 0 replies; 22+ messages in thread
From: Derrick Stolee @ 2018-09-21 15:20 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget, git; +Cc: peff, Junio C Hamano

On 9/21/2018 11:15 AM, Derrick Stolee via GitGitGadget wrote:
> For example, I ran this against the 'next' branch (22e244b)
> versus 'master' (150f307) and got the following output:
>
> fsck.c
> fb8952077df     (René Scharfe   2018-09-03 14:49:26 +0000       212)            die_errno("Could not read '%s'", path);
> list-objects-filter-options.c
> f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       56)                     if (errbuf) {
> f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       57)                             strbuf_init(errbuf, 0);
> f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       58)                             strbuf_addstr(
> f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       62)                     return 1;
> list-objects-filter.c
> 77d7a65d502     (Matthew DeVore 2018-09-13 17:55:26 -0700       47)             BUG("unknown filter_situation: %d", filter_situation);
> f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       100)    default:
> f12b8fc6d3b     (Matthew DeVore 2018-09-13 17:55:27 -0700       101)            BUG("unknown filter_situation: %d", filter_situation);
> 77d7a65d502     (Matthew DeVore 2018-09-13 17:55:26 -0700       152)            BUG("unknown filter_situation: %d", filter_situation);
> 77d7a65d502     (Matthew DeVore 2018-09-13 17:55:26 -0700       257)            BUG("unknown filter_situation: %d", filter_situation);
> 77d7a65d502     (Matthew DeVore 2018-09-13 17:55:26 -0700       438)            BUG("invalid list-objects filter choice: %d",
> list-objects.c
> f447a499dbb     (Matthew DeVore 2018-08-13 11:14:28 -0700       197)                    ctx->show_object(obj, base->buf, ctx->show_data);
> ll-merge.c
> d64324cb60e     (Torsten Bögershausen   2018-09-12 21:32:02 +0200       379)                    marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
> midx.c
> 56ee7ff1565     (Derrick Stolee 2018-09-13 11:02:13 -0700       949)            return 0;
> cc6af73c029     (Derrick Stolee 2018-09-13 11:02:25 -0700       990)                    midx_report(_("failed to load pack-index for packfile %s"),
> cc6af73c029     (Derrick Stolee 2018-09-13 11:02:25 -0700       991)                                e.p->pack_name);
> cc6af73c029     (Derrick Stolee 2018-09-13 11:02:25 -0700       992)                    break;
> remote-curl.c
> c3b9bc94b9b     (Elijah Newren  2018-09-05 10:03:07 -0700       181)            options.filter = xstrdup(value);
> submodule.c
> df255b8cac7     (Brandon Williams       2018-08-08 15:33:22 -0700       1738)           die(_("could not create directory '%s'"), new_gitdir.buf);

I updated this output, but then forgot that I had a "commentary" of the 
old diff below it. Please ignore that portion of the cover letter.

-Stolee


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/1] contrib: add coverage-diff script
  2018-09-21 15:15     ` [PATCH v3 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
@ 2018-09-25 18:36       ` Junio C Hamano
  0 siblings, 0 replies; 22+ messages in thread
From: Junio C Hamano @ 2018-09-25 18:36 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget; +Cc: git, peff, Derrick Stolee

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +files=$(git diff --name-only $V1 $V2 -- *.c)

You'd want to quote that *.c from the shell, i.e. either one of
these

	files=$(git diff --name-only $V1 $V2 -- \*.c)
	files=$(git diff --name-only $V1 $V2 -- '*.c')

otherwise you'd lose things like "builtin/am.c", I'd think.

> +
> +for file in $files
> +do

I know this is only for running in _our_ source tree, and we do not
have a source with $IFS in it, so I'd declare that this is OK.  It
would be good to document that assumption in red capital letters at
the beginning of this loop, though ;-)

	# Note: this script is only for our codebase and we rely on
	# the fact that the pathnames of our source files do not
	# have any funny characters---letting the shell split $files
	# list at $IFS boundary is very much intentional, and not
	# quoting "$file" in the code below also is.  Don't imitate
	# this in scripts that are meant to handle random end-user
	# repositories!
	for file in $files
	do
		...

> +	git diff $V1 $V2 -- $file \
> +		| diff_lines \
> +		| sort >new_lines.txt

I do not see a strong reason why we would want to limit $V1 and $V2
to branch names and raw commit object names, and quoting them in dq
pair is a cheap fix to allow things like

	$ contrib/coverage-diff.sh master 'pu^{/^### match next}'

so let's do so.

Could you cut lines _after_ typing a pipe and omit backslashes, i.e.

	git diff "$V1" "$V2" -- $file |
	diff_lines |
	sort >new_lines.txt

It seems to be personal taste whether to indent the second and
subsequent lines; I do not care if you indent or if you align too
much either way (but I have moderate perference to align).  

But I do not want to see people type unnecessary backslashes.  This
is not limited to just this pipeline but elsewhere in the script.

> +	if ! test -s new_lines.txt
> +	then
> +		continue
> +	fi
> +
> +	hash_file=$(echo $file | sed "s/\//\#/")
> +	sed -ne '/#####:/{
> +			s/    #####://
> +			s/:.*//
> +			s/ //g
> +			p
> +		}' "$hash_file.gcov" \
> +		| sort >uncovered_lines.txt
> +
> +	comm -12 uncovered_lines.txt new_lines.txt \
> +		| sed -e 's/$/\)/' \
> +		| sed -e 's/^/\t/' \

Do you need two sed invocations for this, or would

	sed -e 's/$/\)/' -e '/^/	/'

work as well?  By the way """The meaning of an unescaped <backslash>
immediately followed by any character other than '&', <backslash>, a
digit, <newline>, or the delimiter character used for this command,
is unspecified."""[*1*] so '\t' on the replacement side is a no-no
in the portability department.

	Reference. *1*
	http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html


> +		>uncovered_new_lines.txt
> +
> +	grep -q '[^[:space:]]' < uncovered_new_lines.txt && \

Lose the backslash at the end.  The shell knows that you haven't
finished your sentence when it sees a line that ends with &&, || or
a pipe |, so there is no need to tell it redundantly that you have
more things to say with the backslash.

> +		echo $file && \
> +		git blame -c $file \
> +			| grep -f uncovered_new_lines.txt
> +
> +	rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt
> +done

Near the begininng (like just before the "for file in $files" loop),
you can probably have a trap to make sure these are removed upon
exit, e.g.

    trap 'rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt' 0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 0/1] contrib: Add script to show uncovered "new" lines
  2018-09-21 15:15   ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
  2018-09-21 15:15     ` [PATCH v3 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
  2018-09-21 15:20     ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee
@ 2018-10-08 14:52     ` " Derrick Stolee via GitGitGadget
  2018-10-08 14:52       ` [PATCH v4 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
  2018-10-12  3:01       ` [PATCH v4 0/1] contrib: Add script to show uncovered "new" lines Junio C Hamano
  2 siblings, 2 replies; 22+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2018-10-08 14:52 UTC (permalink / raw)
  To: git; +Cc: peff, Junio C Hamano

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

make coverage-test
make coverage-report

This leaves the repo in a state where every X.c file that was covered has an
X.c.gcov file containing the coverage counts for every line, and "#####" at
every uncovered line.

There have been a few bugs in recent patches what would have been caught if
the test suite covered those blocks (including a few of mine). I want to
work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks in error
cases do not need to be covered.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by
the test suite. For example, I ran this against the 'next' branch (e82ca0)
versus 'master' (f84b9b) and got the following output:

builtin/commit.c
76f2f5c1e3 builtin/commit.c 1657) write_commit_graph_reachable(get_object_directory(), 0, 0);

builtin/fsck.c
66ec0390e7 builtin/fsck.c 862) midx_argv[2] = "--object-dir";
66ec0390e7 builtin/fsck.c 863) midx_argv[3] = alt->path;
66ec0390e7 builtin/fsck.c 864) if (run_command(&midx_verify))
66ec0390e7 builtin/fsck.c 865) errors_found |= ERROR_COMMIT_GRAPH;

fsck.c
fb8952077d  214) die_errno("Could not read '%s'", path);

midx.c
56ee7ff156  949) return 0;
cc6af73c02  990) midx_report(_("failed to load pack-index for packfile %s"),
cc6af73c02  991)     e.p->pack_name);
cc6af73c02  992) break;

Commits introducing uncovered code:
Derrick Stolee      56ee7ff15: multi-pack-index: add 'verify' verb
Derrick Stolee      66ec0390e: fsck: verify multi-pack-index
Derrick Stolee      cc6af73c0: multi-pack-index: verify object offsets
Junio C Hamano      76f2f5c1e: Merge branch 'ab/commit-graph-progress' into next
René Scharfe      fb8952077: fsck: use strbuf_getline() to read skiplist file

Thanks, -Stolee

CHANGES IN V3: I took Junio's perl script verbatim, which speeds up the
performance greatly. Some of the other sed commands needed some massaging,
but also added extra cleanup. Thanks for the help!

CHANGES IN V4: I reduced the blame output using -s which decreases the
width. I include a summary of the commit authors at the end to help people
see the lines they wrote. This version is also copied into a build
definition in the public Git project on Azure Pipelines [1]. I'll use this
build definition to generate the coverage report after each "What's Cooking"
email.

[1] https://git.visualstudio.com/git/_build?definitionId=5

Derrick Stolee (1):
  contrib: add coverage-diff script

 contrib/coverage-diff.sh | 108 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100755 contrib/coverage-diff.sh


base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-40%2Fderrickstolee%2Fcoverage-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-40/derrickstolee/coverage-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/40

Range-diff vs v3:

 1:  21214cc321 ! 1:  6daf310a43 contrib: add coverage-diff script
     @@ -26,10 +26,10 @@
              contrib/coverage-diff.sh base topic
      
          to see the lines added between 'base' and 'topic' that are not covered by the
     -    test suite. The output uses 'git blame -c' format so you can find the commits
     -    responsible and view the line numbers for quick access to the context.
     +    test suite. The output uses 'git blame -s' format so you can find the commits
     +    responsible and view the line numbers for quick access to the context, but
     +    trims leading tabs in the file contents to reduce output width.
      
     -    Helped-by: Junio C Hamano <gister@pobox.com>
          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
      
      diff --git a/contrib/coverage-diff.sh b/contrib/coverage-diff.sh
     @@ -81,13 +81,16 @@
      +	'
      +}
      +
     -+files=$(git diff --name-only $V1 $V2 -- *.c)
     ++files=$(git diff --name-only "$V1" "$V2" -- \*.c)
     ++
     ++# create empty file
     ++>coverage-data.txt
      +
      +for file in $files
      +do
     -+	git diff $V1 $V2 -- $file \
     -+		| diff_lines \
     -+		| sort >new_lines.txt
     ++	git diff "$V1" "$V2" -- "$file" |
     ++	diff_lines |
     ++	sort >new_lines.txt
      +
      +	if ! test -s new_lines.txt
      +	then
     @@ -95,24 +98,50 @@
      +	fi
      +
      +	hash_file=$(echo $file | sed "s/\//\#/")
     ++
     ++	if ! test -s "$hash_file.gcov"
     ++	then
     ++		continue
     ++	fi
     ++
      +	sed -ne '/#####:/{
      +			s/    #####://
      +			s/:.*//
      +			s/ //g
      +			p
     -+		}' "$hash_file.gcov" \
     -+		| sort >uncovered_lines.txt
     ++		}' "$hash_file.gcov" |
     ++	sort >uncovered_lines.txt
      +
     -+	comm -12 uncovered_lines.txt new_lines.txt \
     -+		| sed -e 's/$/\)/' \
     -+		| sed -e 's/^/\t/' \
     -+		>uncovered_new_lines.txt
     ++	comm -12 uncovered_lines.txt new_lines.txt |
     ++	sed -e 's/$/\)/' |
     ++	sed -e 's/^/ /' >uncovered_new_lines.txt
      +
     -+	grep -q '[^[:space:]]' < uncovered_new_lines.txt && \
     -+		echo $file && \
     -+		git blame -c $file \
     -+			| grep -f uncovered_new_lines.txt
     ++	grep -q '[^[:space:]]' <uncovered_new_lines.txt &&
     ++	echo $file >>coverage-data.txt &&
     ++	git blame -s "$V2" -- "$file" |
     ++	sed 's/\t//g' |
     ++	grep -f uncovered_new_lines.txt >>coverage-data.txt &&
     ++	echo >>coverage-data.txt
      +
      +	rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt
      +done
      +
     ++cat coverage-data.txt
     ++
     ++echo "Commits introducing uncovered code:"
     ++
     ++commit_list=$(cat coverage-data.txt |
     ++	grep -E '^[0-9a-f]{7,} ' |
     ++	awk '{print $1;}' |
     ++	sort |
     ++	uniq)
     ++
     ++(
     ++	for commit in $commit_list
     ++	do
     ++		git log --no-decorate --pretty=format:'%an      %h: %s' -1 $commit
     ++		echo
     ++	done
     ++) | sort
     ++
     ++rm coverage-data.txt

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 1/1] contrib: add coverage-diff script
  2018-10-08 14:52     ` [PATCH v4 " Derrick Stolee via GitGitGadget
@ 2018-10-08 14:52       ` Derrick Stolee via GitGitGadget
  2018-10-12  3:01       ` [PATCH v4 0/1] contrib: Add script to show uncovered "new" lines Junio C Hamano
  1 sibling, 0 replies; 22+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2018-10-08 14:52 UTC (permalink / raw)
  To: git; +Cc: peff, Junio C Hamano, Derrick Stolee

From: Derrick Stolee <dstolee@microsoft.com>

We have coverage targets in our Makefile for using gcov to display line
coverage based on our test suite. The way I like to do it is to run:

    make coverage-test
    make coverage-report

This leaves the repo in a state where every X.c file that was covered has
an X.c.gcov file containing the coverage counts for every line, and "#####"
at every uncovered line.

There have been a few bugs in recent patches what would have been caught
if the test suite covered those blocks (including a few of mine). I want
to work towards a "sensible" amount of coverage on new topics. In my opinion,
this means that any logic should be covered, but the 'die()' blocks covering
very unlikely (or near-impossible) situations may not warrant coverage.

It is important to not measure the coverage of the codebase by what old code
is not covered. To help, I created the 'contrib/coverage-diff.sh' script.
After creating the coverage statistics at a version (say, 'topic') you can
then run

    contrib/coverage-diff.sh base topic

to see the lines added between 'base' and 'topic' that are not covered by the
test suite. The output uses 'git blame -s' format so you can find the commits
responsible and view the line numbers for quick access to the context, but
trims leading tabs in the file contents to reduce output width.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 contrib/coverage-diff.sh | 108 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100755 contrib/coverage-diff.sh

diff --git a/contrib/coverage-diff.sh b/contrib/coverage-diff.sh
new file mode 100755
index 0000000000..4ec419f900
--- /dev/null
+++ b/contrib/coverage-diff.sh
@@ -0,0 +1,108 @@
+#!/bin/sh
+
+# Usage: Run 'contrib/coverage-diff.sh <version1> <version2>' from source-root
+# after running
+#
+#     make coverage-test
+#     make coverage-report
+#
+# while checked out at <version2>. This script combines the *.gcov files
+# generated by the 'make' commands above with 'git diff <version1> <version2>'
+# to report new lines that are not covered by the test suite.
+
+V1=$1
+V2=$2
+
+diff_lines () {
+	perl -e '
+		my $line_num;
+		while (<>) {
+			# Hunk header?  Grab the beginning in postimage.
+			if (/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/) {
+				$line_num = $1;
+				next;
+			}
+
+			# Have we seen a hunk?  Ignore "diff --git" etc.
+			next unless defined $line_num;
+
+			# Deleted line? Ignore.
+			if (/^-/) {
+				next;
+			}
+
+			# Show only the line number of added lines.
+			if (/^\+/) {
+				print "$line_num\n";
+			}
+			# Either common context or added line appear in
+			# the postimage.  Count it.
+			$line_num++;
+		}
+	'
+}
+
+files=$(git diff --name-only "$V1" "$V2" -- \*.c)
+
+# create empty file
+>coverage-data.txt
+
+for file in $files
+do
+	git diff "$V1" "$V2" -- "$file" |
+	diff_lines |
+	sort >new_lines.txt
+
+	if ! test -s new_lines.txt
+	then
+		continue
+	fi
+
+	hash_file=$(echo $file | sed "s/\//\#/")
+
+	if ! test -s "$hash_file.gcov"
+	then
+		continue
+	fi
+
+	sed -ne '/#####:/{
+			s/    #####://
+			s/:.*//
+			s/ //g
+			p
+		}' "$hash_file.gcov" |
+	sort >uncovered_lines.txt
+
+	comm -12 uncovered_lines.txt new_lines.txt |
+	sed -e 's/$/\)/' |
+	sed -e 's/^/ /' >uncovered_new_lines.txt
+
+	grep -q '[^[:space:]]' <uncovered_new_lines.txt &&
+	echo $file >>coverage-data.txt &&
+	git blame -s "$V2" -- "$file" |
+	sed 's/\t//g' |
+	grep -f uncovered_new_lines.txt >>coverage-data.txt &&
+	echo >>coverage-data.txt
+
+	rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt
+done
+
+cat coverage-data.txt
+
+echo "Commits introducing uncovered code:"
+
+commit_list=$(cat coverage-data.txt |
+	grep -E '^[0-9a-f]{7,} ' |
+	awk '{print $1;}' |
+	sort |
+	uniq)
+
+(
+	for commit in $commit_list
+	do
+		git log --no-decorate --pretty=format:'%an      %h: %s' -1 $commit
+		echo
+	done
+) | sort
+
+rm coverage-data.txt
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 0/1] contrib: Add script to show uncovered "new" lines
  2018-10-08 14:52     ` [PATCH v4 " Derrick Stolee via GitGitGadget
  2018-10-08 14:52       ` [PATCH v4 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
@ 2018-10-12  3:01       ` Junio C Hamano
  2018-10-12 12:09         ` Derrick Stolee
  1 sibling, 1 reply; 22+ messages in thread
From: Junio C Hamano @ 2018-10-12  3:01 UTC (permalink / raw)
  To: Derrick Stolee via GitGitGadget; +Cc: git, peff

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> CHANGES IN V4: I reduced the blame output using -s which decreases the
> width. I include a summary of the commit authors at the end to help people
> see the lines they wrote. This version is also copied into a build
> definition in the public Git project on Azure Pipelines [1]. I'll use this
> build definition to generate the coverage report after each "What's Cooking"
> email.
>
> [1] https://git.visualstudio.com/git/_build?definitionId=5

Thanks.  Let's move this forward by merging it down to 'next'.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 0/1] contrib: Add script to show uncovered "new" lines
  2018-10-12  3:01       ` [PATCH v4 0/1] contrib: Add script to show uncovered "new" lines Junio C Hamano
@ 2018-10-12 12:09         ` Derrick Stolee
  0 siblings, 0 replies; 22+ messages in thread
From: Derrick Stolee @ 2018-10-12 12:09 UTC (permalink / raw)
  To: Junio C Hamano, Derrick Stolee via GitGitGadget; +Cc: git, peff

On 10/11/2018 11:01 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> CHANGES IN V4: I reduced the blame output using -s which decreases the
>> width. I include a summary of the commit authors at the end to help people
>> see the lines they wrote. This version is also copied into a build
>> definition in the public Git project on Azure Pipelines [1]. I'll use this
>> build definition to generate the coverage report after each "What's Cooking"
>> email.
>>
>> [1] https://git.visualstudio.com/git/_build?definitionId=5
> Thanks.  Let's move this forward by merging it down to 'next'.
Sounds good. When it moves into 'master' I can update my build to call 
the script from source.

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, back to index

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-12 16:45 [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
2018-09-12 16:45 ` [PATCH 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
2018-09-12 22:13   ` Junio C Hamano
2018-09-12 22:54     ` Junio C Hamano
2018-09-13 12:21       ` Derrick Stolee
2018-09-13 14:59         ` Junio C Hamano
2018-09-12 19:14 ` [PATCH 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee
2018-09-12 19:33 ` Ben Peart
2018-09-12 19:53 ` Junio C Hamano
2018-09-13 14:56 ` [PATCH v2 " Derrick Stolee via GitGitGadget
2018-09-13 14:56   ` [PATCH v2 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
2018-09-13 17:40     ` Junio C Hamano
2018-09-13 18:15       ` Derrick Stolee
2018-09-13 17:54     ` Eric Sunshine
2018-09-21 15:15   ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee via GitGitGadget
2018-09-21 15:15     ` [PATCH v3 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
2018-09-25 18:36       ` Junio C Hamano
2018-09-21 15:20     ` [PATCH v3 0/1] contrib: Add script to show uncovered "new" lines Derrick Stolee
2018-10-08 14:52     ` [PATCH v4 " Derrick Stolee via GitGitGadget
2018-10-08 14:52       ` [PATCH v4 1/1] contrib: add coverage-diff script Derrick Stolee via GitGitGadget
2018-10-12  3:01       ` [PATCH v4 0/1] contrib: Add script to show uncovered "new" lines Junio C Hamano
2018-10-12 12:09         ` Derrick Stolee

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox