maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Anel!

On Oct 21, Anel Husakovic wrote:
> revision-id: 8c34a242aa2 (mariadb-10.2.31-478-g8c34a242aa2)
> parent(s): 01ffccd6a4e
> author: Anel Husakovic <anel@xxxxxxxxxxx>
> committer: Anel Husakovic <anel@xxxxxxxxxxx>
> timestamp: 2020-10-06 00:00:58 +0200
> message:
> 
> MDEV-20787: Script dgcov.pl does not work
> 
> - This patch is done after doing test on 00f9df29224ada9cdbb796c99f4
> where difference was found between versions of gcov.

don't refer to commits that are not pushed and are not going to be
pushed. Because if you push this one you'll have a commit forever in the
history that refers to an non-existing commit. It isn't helpful.

> - 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`

> - Make sure to test `MTR_BINDIR` which gets set during out-of-source
> build and generate error when running `./mysql-test/mtr --gcov alias`

What is a problem with out of source builds?
If they don't work, perhaps it would be better to fix dgcov to work with
out of source builds?

> - During the testing:
>   10.1 results are stored in
>   - `<basedir>//mysql-test-gcov.msg` and errors in
>   - `<basedir>//mysql-test-gcov.err`
>   10.2 results should be stored in
>   `./mysql-test/var/last_changes.dgcov`, but only empty results are
>   obtained (tests `1st` and `alias`).

I don't understand this ^^^ paragraph.
And also because your patch is 10.2+ so who cares what it does on 10.1?

> diff --git a/mysql-test/dgcov.pl b/mysql-test/dgcov.pl
> index fbc5540e697..f2bbdad4d20 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): $? $!";
>    }
>  
> -  # now, read the generated file
> -  open FH, '<', "$_.gcov" or die "open(<$_.gcov): $!";
> +  (my $filename = $_)=~ s/\.[^.]+$//; # remove extension

In your previous patch you either removed the extension or not based on
gcov version. Why did you abandon this logic?

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

File::Find documentation seems to be saying that $File::Find::dir ends
with a slash (/) so you don't need to prefix $filename.gcov with it.

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

may be

  return unless -f $gcov_file_path

it's perl after all :)

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

you don't need double quotes around "$gcov_file_path" - they don't serve
any purpose.

> +
>    my $fname;
>    while (<FH>) {
>      chomp;

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx