Re: [HACKERS] coverage analysis improvements

2017-10-16 Thread Peter Eisentraut
On 9/27/17 01:52, Michael Paquier wrote:
> I am marking the full set of patches as ready for committer.

All these patches have now been committed.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-09-26 Thread Michael Paquier
On Tue, Sep 26, 2017 at 3:45 PM, Michael Paquier
 wrote:
> Except for the plperl patch, I don't have more comments to offer about
> this patch set. It would be nice to make configure a bit smarter for
> lcov and gcov detection by not hard-failing if gcov can be found but
> not lcov. It is after all possible to run coverage without lcov, like
> on ArchLinux. Still that's a different request than what this patch
> set is doing, so this is not a blocker for this patch set.

-SPI.c: SPI.xs plperl_helpers.h
+%.c: %.xs
@if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch
--with-perl was not specified."; exit 1; fi
-   $(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap
$(perl_privlibexp)/ExtUtils/typemap $< >$@
-
-Util.c: Util.xs plperl_helpers.h
-   @if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch
--with-perl was not specified."; exit 1; fi
-   $(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap
$(perl_privlibexp)/ExtUtils/typemap $< >$@
+   $(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap
$(perl_privlibexp)/ExtUtils/typemap -output $@ $<
I just looked at the plperl portion of this patch, and I think that
01d83ffd has done some unnecessary things here. Contrary to
perlchunks.h and plperl_opmask.h which are generated during the build,
plperl_helpers.h is part of the source code so there is no meaning in
having a dependency with it. The .c files do not need this header
anyway, but their .o files would, still there is no need for that
either as plperl_helpers.h will not go away.

I am marking the full set of patches as ready for committer.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-09-25 Thread Michael Paquier
On Fri, Sep 22, 2017 at 11:34 PM, Peter Eisentraut
 wrote:
> Apparently, rmgr.c doesn't contain any instrumentable code.  I don't see
> this warning, but it might depend on tool versions and compiler options.

Even on HEAD I am seeing the same problem, this is not outlined just
because the quiet mode of lcov is not used, but the warnings are there
like in this report:
https://www.postgresql.org/message-id/ec92eb95-26de-6da8-9862-ded3ff678c5c%40BlueTreble.com
So please let me discard my concerns about this portion of the patch.

Except for the plperl patch, I don't have more comments to offer about
this patch set. It would be nice to make configure a bit smarter for
lcov and gcov detection by not hard-failing if gcov can be found but
not lcov. It is after all possible to run coverage without lcov, like
on ArchLinux. Still that's a different request than what this patch
set is doing, so this is not a blocker for this patch set.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-09-22 Thread Peter Eisentraut
On 9/21/17 03:42, Michael Paquier wrote:
> -SPI.c: SPI.xs plperl_helpers.h
> +%.c: %.xs
> @if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch
> --with-perl was not specified."; exit 1; fi
> -   $(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap
> $(perl_privlibexp)/ExtUtils/typemap $< >$@
> Doing coverage on plperl with this patch applied, those do not seem
> necessary. But I don't know enough this code to give a clear opinion.

That patch is necessary for doing make coverage in vpath builds.
Otherwise it makes no difference.

> Running coverage-html with all the patches, I am seeing the following
> warnings with a fresh build on my macos laptop 10.11:
> geninfo: WARNING: gcov did not create any files for
> /Users/mpaquier/git/postgres/src/backend/access/transam/rmgr.gcda!
> I don't think that this is normal.

Apparently, rmgr.c doesn't contain any instrumentable code.  I don't see
this warning, but it might depend on tool versions and compiler options.

Note that rmgr.c doesn't show up here either:
https://coverage.postgresql.org/src/backend/access/transam/index.html

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-09-21 Thread Dagfinn Ilmari Mannsåker
Peter Eisentraut  writes:

> On 9/20/17 13:13, Dagfinn Ilmari Mannsåker wrote:
>> I have no opinion on the bulk of this patch set, but skimming it out of
>> curiosity I noticed that the plperl change seems to have lost the
>> dependency on plperl_helpers.h from the xsubpp targets:
>
> Those commands don't actually require plperl_helpers.h, do they?

No, but the .xs files #include it.  I guess it's SPI.o and Util.o that
should depend on the header..

- ilmari
-- 
"I use RMS as a guide in the same way that a boat captain would use
 a lighthouse.  It's good to know where it is, but you generally
 don't want to find yourself in the same spot." - Tollef Fog Heen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-09-21 Thread Michael Paquier
On Thu, Sep 21, 2017 at 1:48 AM, Peter Eisentraut
 wrote:
> OK, I was not aware that people are using it that way.

At least one.

> So updated patch
> set there, which separates coverage and coverage-html into two
> independent targets.

Thanks for the new versions.

Patches 4 and 5 could be merged. They step on each other's code paths.
Some other things could be merged as well.

 coverage-html-stamp: lcov_base.info lcov_test.info
rm -rf coverage
-   $(GENHTML) $(GENHTML_FLAGS) -o coverage --title='$(GENHTML_TITLE)'
--num-spaces=4 --prefix='$(abs_top_srcdir)' $^
+   $(GENHTML) $(GENHTML_FLAGS) -o coverage --title='$(GENHTML_TITLE)'
--num-spaces=4 $^
touch $@
Actually this is very nice. I have been always enforcing
abs_top_srcdir when using coverage stuff on things out of the main
tree.

-SPI.c: SPI.xs plperl_helpers.h
+%.c: %.xs
@if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch
--with-perl was not specified."; exit 1; fi
-   $(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap
$(perl_privlibexp)/ExtUtils/typemap $< >$@
Doing coverage on plperl with this patch applied, those do not seem
necessary. But I don't know enough this code to give a clear opinion.

Running coverage-html with all the patches, I am seeing the following
warnings with a fresh build on my macos laptop 10.11:
geninfo: WARNING: gcov did not create any files for
/Users/mpaquier/git/postgres/src/backend/access/transam/rmgr.gcda!
I don't think that this is normal.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-09-20 Thread Peter Eisentraut
On 9/20/17 13:13, Dagfinn Ilmari Mannsåker wrote:
> I have no opinion on the bulk of this patch set, but skimming it out of
> curiosity I noticed that the plperl change seems to have lost the
> dependency on plperl_helpers.h from the xsubpp targets:

Those commands don't actually require plperl_helpers.h, do they?

>> diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
>> index 191f74067a..66a2c3d4c9 100644
>> --- a/src/pl/plperl/GNUmakefile
>> +++ b/src/pl/plperl/GNUmakefile
>> @@ -81,13 +81,9 @@ perlchunks.h: $(PERLCHUNKS)
>>  
>>  all: all-lib
>>  
>> -SPI.c: SPI.xs plperl_helpers.h
>> +%.c: %.xs
>>  @if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch 
>> --with-perl was not specified."; exit 1; fi
>> -$(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap 
>> $(perl_privlibexp)/ExtUtils/typemap $< >$@
>> -
>> -Util.c: Util.xs plperl_helpers.h
>> -@if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch 
>> --with-perl was not specified."; exit 1; fi
>> -$(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap 
>> $(perl_privlibexp)/ExtUtils/typemap $< >$@
>> +$(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap 
>> $(perl_privlibexp)/ExtUtils/typemap -output $@ $<

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-09-20 Thread Dagfinn Ilmari Mannsåker
Hi Peter,

Peter Eisentraut  writes:

> OK, I was not aware that people are using it that way.  So updated patch
> set there, which separates coverage and coverage-html into two
> independent targets.

I have no opinion on the bulk of this patch set, but skimming it out of
curiosity I noticed that the plperl change seems to have lost the
dependency on plperl_helpers.h from the xsubpp targets:

> diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
> index 191f74067a..66a2c3d4c9 100644
> --- a/src/pl/plperl/GNUmakefile
> +++ b/src/pl/plperl/GNUmakefile
> @@ -81,13 +81,9 @@ perlchunks.h: $(PERLCHUNKS)
>  
>  all: all-lib
>  
> -SPI.c: SPI.xs plperl_helpers.h
> +%.c: %.xs
>   @if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch 
> --with-perl was not specified."; exit 1; fi
> - $(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap 
> $(perl_privlibexp)/ExtUtils/typemap $< >$@
> -
> -Util.c: Util.xs plperl_helpers.h
> - @if [ x"$(perl_privlibexp)" = x"" ]; then echo "configure switch 
> --with-perl was not specified."; exit 1; fi
> - $(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap 
> $(perl_privlibexp)/ExtUtils/typemap $< >$@
> + $(PERL) $(XSUBPPDIR)/ExtUtils/xsubpp -typemap 
> $(perl_privlibexp)/ExtUtils/typemap -output $@ $<
>  
>  
>  install: all install-lib install-data

-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.   - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.  - Calle Dybedahl


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-09-20 Thread Peter Eisentraut
On 8/24/17 04:12, Michael Paquier wrote:
> Patch 0001 removes the .gcov files, which offer a text representation
> of the coverage. Sometimes I use that with a terminal... Not sure for
> the others, but that's my status on the matter. This also removes the
> target coverage. Please note that on some distributions, like, err...
> ArchLinux, lcov is not packaged in the core packages and it is
> necessary to make use of external sources (AUR). It would be nice to
> keep the original gcov calls as well, and keep the split between
> coverage-html and coverage. I think as well that html generate should
> be done only if lcov is found, and that text generation should be done
> only if gcov is used. It is annoying to see --enable-coverage fail
> because lcov only is missing, but it is not mandatory for coverage.

OK, I was not aware that people are using it that way.  So updated patch
set there, which separates coverage and coverage-html into two
independent targets.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 39969d232e9d137aadae55f67a45fa2306f43df9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 10 Aug 2017 23:33:47 -0400
Subject: [PATCH v3 1/9] Run only top-level recursive lcov

This is the way lcov was intended to be used.  It is much faster and
more robust and makes the makefiles simpler than running it in each
subdirectory.

The previous coding ran gcov before lcov, but that is useless because
lcov/geninfo call gcov internally and use that information.  Moreover,
this led to complications and failures during parallel make.  This
separates the two targets:  You either use "make coverage" to get
textual output from gcov or "make coverage-html" to get an HTML report
via lcov.  (Using both is still problematic because they write the same
output files.)
---
 .gitignore|  1 +
 doc/src/sgml/regress.sgml | 13 +
 src/Makefile.global.in| 28 
 3 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/.gitignore b/.gitignore
index 4976fd9119..94e2c582f5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -23,6 +23,7 @@ objfiles.txt
 *.gcov.out
 lcov.info
 coverage/
+coverage-html-stamp
 *.vcproj
 *.vcxproj
 win32ver.rc
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 7c2b1029c2..14747e5f3b 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -706,6 +706,19 @@ Test Coverage Examination
 The make commands also work in subdirectories.

 
+   
+If you don't have lcov or prefer text output over an
+HTML report, you can also run
+
+make coverage
+
+instead of make coverage-html, which will
+produce .gcov output files for each source file
+relevant to the test.  (make coverage and make
+coverage-html will overwrite each other's files, so mixing them
+might be confusing.)
+   
+

 To reset the execution counts between test runs, run:
 
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index fae8068150..f352ba20e2 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -874,25 +874,29 @@ endif # enable_nls
 
 ifeq ($(enable_coverage), yes)
 
-# There is a strange interaction between lcov and existing .gcov
-# output files.  Hence the rm command and the ordering dependency.
+# make coverage -- text output
 
-gcda_files := $(wildcard *.gcda)
+local_gcda_files = $(wildcard *.gcda)
 
-lcov.info: $(gcda_files)
-   rm -f *.gcov .*.gcov
-   $(if $^,$(LCOV) -d . -c -o $@ $(LCOVFLAGS) --gcov-tool $(GCOV))
+coverage: $(local_gcda_files:.gcda=.c.gcov)
 
-%.c.gcov: %.gcda | lcov.info
+%.c.gcov: %.gcda
$(GCOV) -b -f -p -o . $(GCOVFLAGS) $*.c >$*.c.gcov.out
 
-coverage: $(gcda_files:.gcda=.c.gcov) lcov.info
+# make coverage-html -- HTML output via lcov
 
 .PHONY: coverage-html
-coverage-html: coverage
+coverage-html: coverage-html-stamp
+
+coverage-html-stamp: lcov.info
rm -rf coverage
-   mkdir coverage
-   $(GENHTML) --show-details --legend --output-directory=coverage 
--title=PostgreSQL --num-spaces=4 --prefix=$(abs_top_srcdir) `find . -name 
lcov.info -print`
+   $(GENHTML) --show-details --legend --output-directory=coverage 
--title=PostgreSQL --num-spaces=4 --prefix=$(abs_top_srcdir) $<
+   touch $@
+
+all_gcda_files = $(shell find . -name '*.gcda' -print)
+
+lcov.info: $(all_gcda_files)
+   $(LCOV) -d . -c -o $@ $(LCOVFLAGS) --gcov-tool $(GCOV)
 
 
 # hook for clean-up
@@ -900,7 +904,7 @@ clean distclean maintainer-clean: clean-coverage
 
 .PHONY: clean-coverage
 clean-coverage:
-   rm -rf coverage
+   rm -rf coverage coverage-html-stamp
rm -f *.gcda *.gcno lcov.info *.gcov .*.gcov *.gcov.out
 
 

base-commit: d42294fc00da4b97d04ddb4401b76295e8d86816
-- 
2.14.1

From 48c874997f2714268ef2dbdf44d65ebca2a2d5a3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 10 Aug 2017 23:33:47 -0400
Subject: [PA

Re: [HACKERS] coverage analysis improvements

2017-08-24 Thread Michael Paquier
On Wed, Aug 23, 2017 at 3:47 AM, Peter Eisentraut
 wrote:
> On 8/21/17 01:23, Michael Paquier wrote:
>> Patch 0001 fails to apply as of c629324.
>
> Updated patches attached.
>
>> Which versions of lcov and gcov did you use for your tests?
>
> LCOV version 1.13, and gcc-7 and gcc-6

LCOV can be compiled from here (I have for example just changed PREFIX
in the main makefile):
https://github.com/linux-test-project/lcov.gi
And testing down to 1.11 I am not seeing issues with your patches. I
have used gcc 7.1.1.

Patch 0001 removes the .gcov files, which offer a text representation
of the coverage. Sometimes I use that with a terminal... Not sure for
the others, but that's my status on the matter. This also removes the
target coverage. Please note that on some distributions, like, err...
ArchLinux, lcov is not packaged in the core packages and it is
necessary to make use of external sources (AUR). It would be nice to
keep the original gcov calls as well, and keep the split between
coverage-html and coverage. I think as well that html generate should
be done only if lcov is found, and that text generation should be done
only if gcov is used. It is annoying to see --enable-coverage fail
because lcov only is missing, but it is not mandatory for coverage.

Patches 2, 4, 5, 6 and 9 are good independent ideas.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] coverage analysis improvements

2017-08-22 Thread Peter Eisentraut
On 8/21/17 01:23, Michael Paquier wrote:
> Patch 0001 fails to apply as of c629324.

Updated patches attached.

> Which versions of lcov and gcov did you use for your tests?

LCOV version 1.13, and gcc-7 and gcc-6

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 7b2f5087a2d0710e6a5c7ffa946dcfabd163f987 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 10 Aug 2017 23:33:47 -0400
Subject: [PATCH v2 1/9] Run only top-level recursive lcov

This is the way lcov was intended to be used.  It is much faster and
more robust and makes the makefiles simpler than running it in each
subdirectory.

This also removes the direct gcov calls and lets lcov do it instead.
The direct gcov calls are useless because lcov/geninfo call gcov
internally and use that information.
---
 .gitignore|  3 +--
 GNUmakefile.in|  2 +-
 doc/src/sgml/regress.sgml |  2 +-
 src/Makefile.global.in| 33 +++--
 4 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/.gitignore b/.gitignore
index 4976fd9119..2052f719d1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,10 +19,9 @@ objfiles.txt
 .deps/
 *.gcno
 *.gcda
-*.gcov
-*.gcov.out
 lcov.info
 coverage/
+coverage-stamp
 *.vcproj
 *.vcxproj
 win32ver.rc
diff --git a/GNUmakefile.in b/GNUmakefile.in
index dc76a5d11d..8d77b01eea 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -41,7 +41,7 @@ install-world-contrib-recurse: install-world-src-recurse
 
 $(call recurse,installdirs uninstall init-po update-po,doc src config)
 
-$(call recurse,distprep coverage,doc src config contrib)
+$(call recurse,distprep,doc src config contrib)
 
 # clean, distclean, etc should apply to contrib too, even though
 # it's not built by default
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 7c2b1029c2..796cdc26ff 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -699,7 +699,7 @@ Test Coverage Examination
 ./configure --enable-coverage ... OTHER OPTIONS ...
 make
 make check # or other test suite
-make coverage-html
+make coverage
 
 Then point your HTML browser
 to coverage/index.html.
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index e8b3a519cb..e39ed884e7 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -19,7 +19,7 @@
 #
 # Meta configuration
 
-standard_targets = all install installdirs uninstall distprep clean distclean 
maintainer-clean coverage check installcheck init-po update-po
+standard_targets = all install installdirs uninstall distprep clean distclean 
maintainer-clean check installcheck init-po update-po
 # these targets should recurse even into subdirectories not being built:
 standard_always_targets = distprep clean distclean maintainer-clean
 
@@ -863,34 +863,23 @@ endif # enable_nls
 #  (by gcc -ftest-coverage)
 #   foo.gcda   gcov data file, created when the program is run (for
 #  programs compiled with gcc -fprofile-arcs)
-#   foo.c.gcov gcov output file with coverage information, created by
-#  gcov from foo.gcda (by "make coverage")
-#   foo.c.gcov.out  stdout captured when foo.c.gcov is created, mildly
-#  interesting
 #   lcov.info  lcov tracefile, built from gcda files in one directory,
 #  later collected by "make coverage-html"
 
 ifeq ($(enable_coverage), yes)
 
-# There is a strange interaction between lcov and existing .gcov
-# output files.  Hence the rm command and the ordering dependency.
-
-gcda_files := $(wildcard *.gcda)
+gcda_files = $(shell find . -name '*.gcda' -print)
 
 lcov.info: $(gcda_files)
-   rm -f *.gcov .*.gcov
-   $(if $^,$(LCOV) -d . -c -o $@ $(LCOVFLAGS) --gcov-tool $(GCOV))
-
-%.c.gcov: %.gcda | lcov.info
-   $(GCOV) -b -f -p -o . $(GCOVFLAGS) $*.c >$*.c.gcov.out
-
-coverage: $(gcda_files:.gcda=.c.gcov) lcov.info
+   $(LCOV) -d . -c -o $@ $(LCOVFLAGS) --gcov-tool $(GCOV)
 
-.PHONY: coverage-html
-coverage-html: coverage
+coverage-stamp: lcov.info
rm -rf coverage
-   mkdir coverage
-   $(GENHTML) --show-details --legend --output-directory=coverage 
--title=PostgreSQL --num-spaces=4 --prefix=$(abs_top_srcdir) `find . -name 
lcov.info -print`
+   $(GENHTML) --show-details --legend --output-directory=coverage 
--title=PostgreSQL --num-spaces=4 --prefix=$(abs_top_srcdir) $<
+   touch $@
+
+.PHONY: coverage coverage-html
+coverage coverage-html: coverage-stamp
 
 
 # hook for clean-up
@@ -898,8 +887,8 @@ clean distclean maintainer-clean: clean-coverage
 
 .PHONY: clean-coverage
 clean-coverage:
-   rm -rf coverage
-   rm -f *.gcda *.gcno lcov.info *.gcov .*.gcov *.gcov.out
+   rm -rf coverage/ coverage-stamp
+   rm -f *.gcda *.gcno lcov.info
 
 
 # User-callable target to reset counts between test runs

base-commit: 2bfd1b1ee562c4e4fd065c7f7d1beaa9b9852070
-- 
2.14.1

From 0bbf137972e22923f650a38ff46c0e13

Re: [HACKERS] coverage analysis improvements

2017-08-20 Thread Michael Paquier
On Fri, Aug 11, 2017 at 1:05 PM, Peter Eisentraut
 wrote:
> Here is a patch series with some significant reworking and adjusting of
> how the coverage analysis tools are run.  The result should be that the
> "make coverage" runs are faster and more robust, the results are more
> accurate, and the code is simpler.  Please see the individual patches
> for details.
>
> I have replaced "make coverage-html" with "make coverage" (but kept the
> previous name for compatibility), since the old meaning of the latter
> has gone away and was never very useful.  Other than that, the usage of
> everything stays the same.
>
> I will add it to the commit fest.  Testing with different versions of
> tools would be especially valuable.

Patch 0001 fails to apply as of c629324. Which versions of lcov and
gcov did you use for your tests?
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] coverage analysis improvements

2017-08-10 Thread Peter Eisentraut
Here is a patch series with some significant reworking and adjusting of
how the coverage analysis tools are run.  The result should be that the
"make coverage" runs are faster and more robust, the results are more
accurate, and the code is simpler.  Please see the individual patches
for details.

I have replaced "make coverage-html" with "make coverage" (but kept the
previous name for compatibility), since the old meaning of the latter
has gone away and was never very useful.  Other than that, the usage of
everything stays the same.

I will add it to the commit fest.  Testing with different versions of
tools would be especially valuable.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 3c8a2c6afb3a8873a70c61c427128bb4d1cade2b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 10 Aug 2017 23:33:47 -0400
Subject: [PATCH 1/9] Run only top-level recursive lcov

This is the way lcov was intended to be used.  It is much faster and
more robust and makes the makefiles simpler than running it in each
subdirectory.

This also removes the direct gcov calls and lets lcov do it instead.
The direct gcov calls are useless because lcov/geninfo call gcov
internally and use that information.
---
 .gitignore|  3 +--
 GNUmakefile.in|  2 +-
 doc/src/sgml/regress.sgml |  2 +-
 src/Makefile.global.in| 33 +++--
 4 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/.gitignore b/.gitignore
index 4976fd9119..2052f719d1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,10 +19,9 @@ objfiles.txt
 .deps/
 *.gcno
 *.gcda
-*.gcov
-*.gcov.out
 lcov.info
 coverage/
+coverage-stamp
 *.vcproj
 *.vcxproj
 win32ver.rc
diff --git a/GNUmakefile.in b/GNUmakefile.in
index dc76a5d11d..8d77b01eea 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -41,7 +41,7 @@ install-world-contrib-recurse: install-world-src-recurse
 
 $(call recurse,installdirs uninstall init-po update-po,doc src config)
 
-$(call recurse,distprep coverage,doc src config contrib)
+$(call recurse,distprep,doc src config contrib)
 
 # clean, distclean, etc should apply to contrib too, even though
 # it's not built by default
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 7c2b1029c2..796cdc26ff 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -699,7 +699,7 @@ Test Coverage Examination
 ./configure --enable-coverage ... OTHER OPTIONS ...
 make
 make check # or other test suite
-make coverage-html
+make coverage
 
 Then point your HTML browser
 to coverage/index.html.
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 0d3f8ca950..b01e578238 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -19,7 +19,7 @@
 #
 # Meta configuration
 
-standard_targets = all install installdirs uninstall distprep clean distclean 
maintainer-clean coverage check installcheck init-po update-po
+standard_targets = all install installdirs uninstall distprep clean distclean 
maintainer-clean check installcheck init-po update-po
 # these targets should recurse even into subdirectories not being built:
 standard_always_targets = distprep clean distclean maintainer-clean
 
@@ -863,34 +863,23 @@ endif # enable_nls
 #  (by gcc -ftest-coverage)
 #   foo.gcda   gcov data file, created when the program is run (for
 #  programs compiled with gcc -fprofile-arcs)
-#   foo.c.gcov gcov output file with coverage information, created by
-#  gcov from foo.gcda (by "make coverage")
-#   foo.c.gcov.out  stdout captured when foo.c.gcov is created, mildly
-#  interesting
 #   lcov.info  lcov tracefile, built from gcda files in one directory,
 #  later collected by "make coverage-html"
 
 ifeq ($(enable_coverage), yes)
 
-# There is a strange interaction between lcov and existing .gcov
-# output files.  Hence the rm command and the ordering dependency.
-
-gcda_files := $(wildcard *.gcda)
+gcda_files = $(shell find . -name '*.gcda' -print)
 
 lcov.info: $(gcda_files)
-   rm -f *.gcov
-   $(if $^,$(LCOV) -d . -c -o $@ $(LCOVFLAGS) --gcov-tool $(GCOV))
-
-%.c.gcov: %.gcda | lcov.info
-   $(GCOV) -b -f -p -o . $(GCOVFLAGS) $*.c >$*.c.gcov.out
-
-coverage: $(gcda_files:.gcda=.c.gcov) lcov.info
+   $(LCOV) -d . -c -o $@ $(LCOVFLAGS) --gcov-tool $(GCOV)
 
-.PHONY: coverage-html
-coverage-html: coverage
+coverage-stamp: lcov.info
rm -rf coverage
-   mkdir coverage
-   $(GENHTML) --show-details --legend --output-directory=coverage 
--title=PostgreSQL --num-spaces=4 --prefix=$(abs_top_srcdir) `find . -name 
lcov.info -print`
+   $(GENHTML) --show-details --legend --output-directory=coverage 
--title=PostgreSQL --num-spaces=4 --prefix=$(abs_top_srcdir) $<
+   touch $@
+
+.PHONY: coverage coverage-html
+coverage coverage-html: coverage-stamp
 
 
 # hook for clean-up
@@ -898,8 +887,8 @@ cle