Hi, Anel!
On Oct 30, Anel Husakovic wrote:
> revision-id: f112c4a07475 (mariadb-10.2.31-541-gf112c4a07475)
> parent(s): 784473b98662
> author: Anel Husakovic <a...@mariadb.org>
> committer: Anel Husakovic <a...@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 secur...@mariadb.org
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
