From: Junio C Hamano <gitster@pobox.com>
To: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, peff@peff.net,
Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH 1/1] contrib: add coverage-diff script
Date: Wed, 12 Sep 2018 15:13:30 -0700 [thread overview]
Message-ID: <xmqqa7omjred.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <e4124471e5494b737d99eceed25fb03e787d0b96.1536770746.git.gitgitgadget@gmail.com> (Derrick Stolee via GitGitGadget's message of "Wed, 12 Sep 2018 09:45:49 -0700 (PDT)")
"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
next prev parent reply other threads:[~2018-09-12 22:13 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqa7omjred.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).