Hi, Anel! On Oct 30, Anel Husakovic wrote:

revision-id: f112c4a07475 (mariadb-10.2.31-541-gf112c4a07475) parent(s): 784473b98662 author: Anel Husakovic <anel@mariadb.org> committer: Anel Husakovic <anel@mariadb.org> timestamp: 2020-10-26 14:25:41 +0100 message:

MDEV-20787: Script dgcov.pl does not work

- Patch is changing CMakeList with `--coverage` flag as an alias for `-fprofile-arcs -ftest-coverage -lgcov` in addition.

- About the usage of `dgcov`: When the server is compiled with `-DENABLE_GCOV=ON`, from object files are generated `.gcno` and `.gcda` files. `./mtr --gcov is_check_constraint` is invoking the following script calls: - `./dgcov.pl --purge`, `./mtr is_check_constraint` and - `./dgcov.pl --generate>/var/last_changes.dgcov`. The `purge` flag is clearing `.gcda` files (and others extensions), while running the test (which follows after the purge) new `.gcda` files are generated. With dgcov `generate` flag, `gcov -i` (`intermediate format`) is called on generated `<source-files-name>.gcda` files (`dbug.c.gcda` e.g.). The patch is tested on `gcov 6.3` and `gcov 7.4` versions and can be seen that resulting `.gcov` file for `6.3` creates `<full path>.gcov` (`dbug.c.gcda.gcov` e.g.) file, where `gcov 7.4+` is still creating `source-file-names.gcov`(`dbug.c.gcov`) files as `gcov` in general is doing. Results are stored in `./mysql-test/var/last_changes.dgcov`. Note: for the newer versions of `gcov` intermediate format is deprecated `-i` and should be used `--json-format`.

does it mean that eventually we'll have to rewrite dgcov to support json format?

- This patch doesn't take gcov versions in account since is extracting filename of a `.gcov` file, which is generated with `gcov -i <source-file>.gcda` during `dgcov.pl --generate`.

- There is a check if file exists. Reason for that is because some generated `gcov` files are outliers from above rule (that a file generated in intermediate format(gcov) has the same source name). Example 1: `aestables.cpp.gcda` is not generating `aestables.cpp.gcov`, but only the files used in #include `modes.hpp.gcov runtime.hpp.gcov` Example 2: `my_sha256.cc.gcda` is generating `my_sha.ic.gcov`

- Gcov with out-of-source is not working, this patch will solve that (probably?) Make sure to test `MTR_BINDIR` which gets set during out-of-source build and generate an error when running `./mysql-test/mtr --gcov alias`.

diff --git a/mysql-test/dgcov.pl b/mysql-test/dgcov.pl index fbc5540e6973..f2bbdad4d20e 100755 --- a/mysql-test/dgcov.pl +++ b/mysql-test/dgcov.pl @@ -161,8 +161,14 @@ sub gcov_one_file { system($cmd)==0 or die "system($cmd): $? $!"; }

I don't see any changes since the last review. Is it the correct commit?

- # now, read the generated file - open FH, '<', "$_.gcov" or die "open(<$_.gcov): $!"; + (my $filename = $_)=~ s/\.[^.]+$//; # remove extension

I think this contradicts your explanation in the commit comment. You wrote that the file name can be either dbug.c.gcda.gcov or dbug.c.gcov. Here $_ is dbug.c.gcda, you unconditionally remove .gcda and then below you open $filename.gcov, that is dbug.c.gcov Which means this won't work with an older gcov. Is this your intention? I think it's fine not to support old gcov. But this should be a conscious decision, e.g. you can write in the commit comment "Now dgcov.pl only supports gcov 7.4+" - if that's indeed what you want. Also, if you do it this way, then there's no need to use s///, it's simpler to do, like sub gcov_one_file { return unless /\.gcda$/; + my $filename= $`; unless ($opt_skip_gcov) { or, may be even sub gcov_one_file { return unless /\.gcda$/; + my $gcov_file_path= "$File::Find::dir$`.gcov"; unless ($opt_skip_gcov) {

+ my $gcov_file_path= $File::Find::dir."/$filename.gcov";

still an extra slash

+ if (! -f $gcov_file_path) + { + return; + }

still not return unless -f $gcov_file_path;

+ open FH, '<', "$gcov_file_path" or die "open(<$gcov_file_path): $!";

still extra quotes

+ my $fname; while (<FH>) { chomp; diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index 900ef736a455..2189529d82a7 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -1791,7 +1791,7 @@ sub command_line_setup { # -------------------------------------------------------------------------- # Gcov flag # -------------------------------------------------------------------------- - if ( ($opt_gcov or $opt_gprof) and ! $source_dist ) + if ( ($opt_gcov or $opt_gprof) and (! $source_dist or -d $ENV{MTR_BINDIR})) { mtr_error("Coverage test needs the source - please use source dist"); }

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org