Req #52718 [Opn->Csd]: Simplifying Makefile.gcov?

Edit report at https://bugs.php.net/bug.php?id=52718&edit=1 ID: 52718 Updated by: nikic@php.net Reported by: hartmut@php.net Summary: Simplifying Makefile.gcov? -Status: Open +Status: Closed Type: Feature/Change Request Package: Unknown/Other Function Operating System: linux/unix PHP Version: 5.3SVN-2010-08-27 (SVN) -Assigned To: +Assigned To: nikic Block user comment: N Private report: N New Comment: This was implemented in https://github.com/php/php-src/pull/4739. Previous Comments: ------------------------------------------------------------------------ [2010-08-30 19:11:01] johannes@php.net the lcov version on gcov.php.net supports the flag, so this might be ok. ------------------------------------------------------------------------ [2010-08-27 13:59:17] hholzgra@php.net Correct link for "Actual result" is http://php-baustelle.de/lcov_oldhtml ------------------------------------------------------------------------ [2010-08-27 13:33:59] hartmut@php.net Description: ------------ Looks as if the current lcov/gcov implementation works by: * collecting all 'interesting' files and copying/linking them over to a new directory lcov_data * then removing files for which actually no coverage information should be generated, like bundled library sources * finally running lcov and genhtml over the lcov_data copies As far as i can tell the removal of bundled library stuff is the only real reason for the copy/symlink step, or am i missing something else there? Assuming that this is really the only reason i'd like to propose to use 'lcov --remove' for filtering out the bundled library stuff instead and to run lcov and genhtml in place instead of the new lcov_data subdir, so skipping the need for this extra directory and the process needed to populate it. Advantages: * less complex (Makefile.gcov line count shrinks from 64 to 30) * no duplication of information * shows coverage info for a few file that the current implementation misses for some reason unknown to me yet, like * main/internal_functions.c * main/internal_functions_cli.c * ext/standard/url_scanner_ex.re * ext/standard/var_unserializer.re * Zend/zend_language_scanner.l * Zend/zend_ini_scanner.l Disadvantages: * 'lcov --remove' is rather slow, as far as i can tell the copy&remove approach is a tad quicker ... the difference is not relevant when compared with the total time needed for a lcov analysis run though * genhtml still generates HTML files for files filtered out by 'lcov --remove', so taking a bit longer to run, the files do not show up in any of the result overview pages though So all in all i think the advantages justify a change while the disadvantages are not blocking it. The only thing that actually changes would be the 'php_lcov.info' target in 'Makgefile.gcov', the simplified version would look like this: php_lcov.info: lcov-test @echo "Generating $@" @$(LTP) --directory . --capture --base-directory . --output-file $@ @echo "Stripping bundled libraries from $@" @for dir in ext/bcmath/libbcmath ext/fileinfo/libmagic ext/gd/libgd ext/mbstring/libmbfl ext/mbstring/oniguruma ext/pcre/pcrelib ext/pdo_sqlite/libsqlite ext/sqlite/libsqlite ext/sqlite3/libsqlite ext/xmlrpc/libxmlrpc ext/zip/lib; do \ echo "... removing $$dir"; \ $(LTP) --remove $@ $$dir/\* --output-file $@ ; \ done @echo Ideas, comments, any reasons *not* to commit this? Test script: --------------- see Makefile.gcov implementation Expected result: ---------------- LCOV report generated by patched PHP 5.3 "make lcov" see http://php-baustelle.de/lcov_newhtml Actual result: -------------- LCOV report generated by unpatched PHP 5.3 "make lcov" see http://php-baustelle.de/lcov_old65html ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=52718&edit=1