Extensions makefiles - coverage

Lists: pgsql-hackers
From: Ronan Dunklau <rdunklau(at)gmail(dot)com>
To: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Extensions makefiles - coverage
Date: 2013-07-25 15:07:31
Message-ID: CAJWq4=Z+1P8ZWPK72j5FOOuCH7kOZnUmsY3u7aOFHFeTS-YQ5Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hello.

I was having trouble figuring how to use the coverage targets when
using an extension.
I am using approximatively the layout that was proposed here:
http://www.postgresql.org/message-id/51BB1B6E.2070705@dunslane.net
It looks like everything is hard-coded to take the source and the
gcda, gcno files in the base directory, but these files lay in a src
directory with the proposed layout.

It may be better to base the .gcda file discovery on the OBJS
variables when using PGXS.

Please find attached a small patch that implements this. There is
probably a better way than the redundant rm $(gcda_files) / rm *.gcda
to cleanup the generated files.

With the attached patch, the following targets seem to have the same
behaviour as on the current HEAD, both on the whole tree and on
individual contrib modules:

- coverage-html
- clean
- coverage-clean
- clean-coverage

I noticed that make clean leaves gcda and gcov files on the current
HEAD, and this is no different with the given patch.

I also tested it against several pgxn extensions, and it seems to work fine.

Attachment Content-Type Size
coverage_pgxs.patch application/octet-stream 948 bytes

From: Ronan Dunklau <rdunklau(at)gmail(dot)com>
To: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extensions makefiles - coverage
Date: 2013-07-25 15:08:55
Message-ID: CAJWq4=YknFvaV-a7OL7SGsu2zN4AN7ujcceeRjahLBSZjwuNdA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Please ignore this comment:

> I noticed that make clean leaves gcda and gcov files on the current
> HEAD, and this is no different with the given patch.


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Ronan Dunklau <rdunklau(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extensions makefiles - coverage
Date: 2013-07-26 13:06:51
Message-ID: CA+TgmoZAsHV8B2RtWAZA=9ShryfemWXJ0=2LjD-wGBLF4vNFLw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Jul 25, 2013 at 11:07 AM, Ronan Dunklau <rdunklau(at)gmail(dot)com> wrote:
> Hello.
>
> I was having trouble figuring how to use the coverage targets when
> using an extension.
> I am using approximatively the layout that was proposed here:
> http://www.postgresql.org/message-id/51BB1B6E.2070705@dunslane.net
> It looks like everything is hard-coded to take the source and the
> gcda, gcno files in the base directory, but these files lay in a src
> directory with the proposed layout.
>
> It may be better to base the .gcda file discovery on the OBJS
> variables when using PGXS.
>
> Please find attached a small patch that implements this. There is
> probably a better way than the redundant rm $(gcda_files) / rm *.gcda
> to cleanup the generated files.
>
> With the attached patch, the following targets seem to have the same
> behaviour as on the current HEAD, both on the whole tree and on
> individual contrib modules:
>
> - coverage-html
> - clean
> - coverage-clean
> - clean-coverage
>
> I noticed that make clean leaves gcda and gcov files on the current
> HEAD, and this is no different with the given patch.
>
> I also tested it against several pgxn extensions, and it seems to work fine.

You can ensure that your patch doesn't get forgotten about by adding it here:

https://commitfest.postgresql.org/action/commitfest_view/open

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


From: Ronan Dunklau <rdunklau(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extensions makefiles - coverage
Date: 2013-07-26 13:30:16
Message-ID: CAJWq4=bUxd7RXDzi9iDyoa8Gujwmsa9D4PrQcLKm1AY=GrN+Xw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Thank you for the tip, its done.

2013/7/26 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Thu, Jul 25, 2013 at 11:07 AM, Ronan Dunklau <rdunklau(at)gmail(dot)com> wrote:
>> Hello.
>>
>> I was having trouble figuring how to use the coverage targets when
>> using an extension.
>> I am using approximatively the layout that was proposed here:
>> http://www.postgresql.org/message-id/51BB1B6E.2070705@dunslane.net
>> It looks like everything is hard-coded to take the source and the
>> gcda, gcno files in the base directory, but these files lay in a src
>> directory with the proposed layout.
>>
>> It may be better to base the .gcda file discovery on the OBJS
>> variables when using PGXS.
>>
>> Please find attached a small patch that implements this. There is
>> probably a better way than the redundant rm $(gcda_files) / rm *.gcda
>> to cleanup the generated files.
>>
>> With the attached patch, the following targets seem to have the same
>> behaviour as on the current HEAD, both on the whole tree and on
>> individual contrib modules:
>>
>> - coverage-html
>> - clean
>> - coverage-clean
>> - clean-coverage
>>
>> I noticed that make clean leaves gcda and gcov files on the current
>> HEAD, and this is no different with the given patch.
>>
>> I also tested it against several pgxn extensions, and it seems to work fine.
>
> You can ensure that your patch doesn't get forgotten about by adding it here:
>
> https://commitfest.postgresql.org/action/commitfest_view/open
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Ronan Dunklau <rdunklau(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extensions makefiles - coverage
Date: 2013-09-22 05:34:53
Message-ID: 1379828093.5400.2.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, 2013-07-25 at 17:07 +0200, Ronan Dunklau wrote:
> I am using approximatively the layout that was proposed here:
> http://www.postgresql.org/message-id/51BB1B6E.2070705@dunslane.net
> It looks like everything is hard-coded to take the source and the
> gcda, gcno files in the base directory, but these files lay in a src
> directory with the proposed layout.

The PostgreSQL build system isn't going to work very well if you build
files outside of the current directory. If you want to put your source
files into a src/ subdirectory, then your top-level makefile should to a
$(MAKE) -C src, and you need to have a second makefile in the src
directory. If you do that, then the existing coverage targets will work
alright, I think.


From: Ronan Dunklau <rdunklau(at)gmail(dot)com>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Extensions makefiles - coverage
Date: 2013-09-24 06:44:42
Message-ID: 1526670.BGjqRqbRiH@ronan_laptop
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sunday 22 September 2013 01:34:53 Peter Eisentraut wrote:
> On Thu, 2013-07-25 at 17:07 +0200, Ronan Dunklau wrote:
> > I am using approximatively the layout that was proposed here:
> > http://www.postgresql.org/message-id/51BB1B6E.2070705@dunslane.net
> > It looks like everything is hard-coded to take the source and the
> > gcda, gcno files in the base directory, but these files lay in a src
> > directory with the proposed layout.
>
> The PostgreSQL build system isn't going to work very well if you build
> files outside of the current directory. If you want to put your source
> files into a src/ subdirectory, then your top-level makefile should to a
> $(MAKE) -C src, and you need to have a second makefile in the src
> directory. If you do that, then the existing coverage targets will work
> alright, I think.

The PGXS build system allows for the definition of an OBJS variable, which
works fine with almost every other make target.

Maybe we need to take a step back, and think about what kind of extension
layouts we want to support ?

At the time of this writing, the HOW TO on http://manager.pgxn.org/howto
"strongly encourage" to put all C-files in an src directory.

As a result, many extensions on pgxn use this layout. It would be great not to
have to change them to measure code coverage.