Skip to content

Commit 5186f73

Browse files
authored
Improved baseline/current/diff error checking. (#397)
Signed-off-by: Henry Cox <[email protected]>
1 parent 4bed883 commit 5186f73

File tree

2 files changed

+81
-79
lines changed

2 files changed

+81
-79
lines changed

Diff for: bin/genhtml

+67-69
Original file line numberDiff line numberDiff line change
@@ -3479,7 +3479,7 @@ sub new
34793479
{
34803480
my $class = shift;
34813481
my $self = [{}, # linemap
3482-
{}, # filemap
3482+
{}, # filemap: new_filename->old_filename
34833483
{}, # baseline: filename -> baseline lineno -> text
34843484
undef, # diff filename
34853485
[{}, {}], # def location
@@ -3497,10 +3497,11 @@ sub load
34973497
my ($self, $path, $info, $buildDirs) = @_;
34983498
$self->_read_udiff($path);
34993499

3500+
# find list of soft links in [buildDirs] which may point to files in $info
3501+
# we need to keep all the aliases as there may be files in baseline
3502+
# which are not in current
35003503
if (@$buildDirs) {
3501-
# find list of soft links in [buildDirs] which point to files in $info
35023504
my @stack = @$buildDirs;
3503-
my %alias;
35043505
while (@stack) {
35053506
my $dir = pop(@stack);
35063507
die("unexpected non-directory '$dir'") unless -d $dir;
@@ -3515,7 +3516,10 @@ sub load
35153516
push(@stack, $path);
35163517
} elsif (-l $path) {
35173518
my $l = Cwd::realpath($path);
3518-
$alias{$path} = $l if (-f $l);
3519+
next if (!-e $l || TraceFile::skipCurrentFile($l));
3520+
# may need to turn $l into relative path??
3521+
$self->[ALIASES]->{$path} = $l if (-f $l);
3522+
#lcovutil::info("add alias '$path' -> '$l'\n");
35193523
# really, this should be a file...
35203524
die("unexpected soft link $path to directory")
35213525
unless -f $l;
@@ -3524,17 +3528,6 @@ sub load
35243528
}
35253529
closedir($dh);
35263530
}
3527-
# now look through list of filenames in the current .info file, and see
3528-
# if any match the soft links found above
3529-
foreach my $f ($info->files()) {
3530-
my $path = $f;
3531-
$path = File::Spec->catfile($main::cwd, $f)
3532-
unless File::Spec->file_name_is_absolute($f);
3533-
if (exists($alias{$path})) {
3534-
#lcovutil::info("found alias $alias{$path} of $f\n");
3535-
$self->[ALIASES]->{$path} = $alias{$path};
3536-
}
3537-
}
35383531
}
35393532
return $self;
35403533
}
@@ -3772,20 +3765,23 @@ sub check_path_consistency
37723765
# That is: files whose version differs should appear in the diff data
37733766
$self->check_version_match($baseline, $current);
37743767

3775-
my %diffMap;
3776-
my %diffBaseMap;
3768+
my %diffMap; # current_filename -> where_used
3769+
my %diffBaseMap; # current_directory -> []
3770+
# for every 'current' filename in the udiff file
37773771
foreach my $f ($self->files()) {
3778-
$diffMap{$f} = 0;
3772+
$diffMap{$f} = 0; # this file not used in baseline or current (yet)
37793773
my $b = File::Basename::basename($f);
37803774
$diffBaseMap{$b} = [[], {}]
37813775
unless exists($diffBaseMap{$b});
37823776
push(@{$diffBaseMap{$b}->[0]}, $f);
37833777
}
3778+
# foreach unchanged file in udiff data
37843779
foreach my $f (keys %{$self->[UNCHANGED]}) {
37853780
# unchanged in baseline and current
37863781
$diffMap{$f} = 3;
37873782
}
37883783
my %missed;
3784+
# for each file in 'current' info:
37893785
foreach my $curr ($current->files()) {
37903786
my $b = File::Basename::basename($curr);
37913787
if ($self->containsFile($curr)) {
@@ -3796,10 +3792,12 @@ sub check_path_consistency
37963792
$diffBaseMap{$b}->[1]->{$alias} = 0
37973793
unless exists($diffBaseMap{$b}->[1]->{$alias});
37983794
++$diffBaseMap{$b}->[1]->{$alias};
3799-
} else {
3795+
} elsif (!exists($self->[UNCHANGED]->{$self->findName($curr)})) {
38003796
$missed{$curr} = 1; # in current but not in diff
3797+
#lcovutil::info("missed curr: $curr\n");
38013798
}
38023799
}
3800+
# for each file in 'baseline' info:
38033801
foreach my $base ($baseline->files()) {
38043802
my $b = File::Basename::basename($base);
38053803
if ($self->containsFile($base)) {
@@ -3810,8 +3808,9 @@ sub check_path_consistency
38103808
$diffBaseMap{$b}->[1]->{$alias} = 0
38113809
unless exists($diffBaseMap{$b}->[1]->{$alias});
38123810
++$diffBaseMap{$b}->[1]->{$alias};
3813-
} else {
3811+
} elsif (!exists($self->[UNCHANGED]->{$self->findName($base)})) {
38143812
# in baseline but not in diff
3813+
#lcovutil::info("missed base: $base\n");
38153814
if (exists($missed{$base})) {
38163815
$missed{$base} |= 2;
38173816
} else {
@@ -3822,45 +3821,46 @@ sub check_path_consistency
38223821
my $ok = 1;
38233822
foreach my $f (sort keys(%missed)) {
38243823
my $b = File::Basename::basename($f);
3825-
if (exists($diffBaseMap{$b})) {
3826-
# this basename is in the diff file and didn't match any other
3827-
# trace filename entry (i.e., same filename in more than one
3828-
# source code directory) - then warn about possible pathname issue
3829-
my ($diffFiles, $sourceFiles) = @{$diffBaseMap{$b}};
3830-
# find the files which appear in the 'diff' list which have the
3831-
# same basename and were not matched - those might be candidates
3832-
my @unused;
3833-
for my $d (@$diffFiles) {
3834-
# my $location = $self->[DIFF_FILENAME] . ':' . $self->[LOCATION]->[NEW]->{$d};
3835-
push(@unused, $d)
3836-
unless exists($sourceFiles->{$d});
3837-
}
3838-
if (scalar(@unused)) {
3839-
my $type;
3840-
3841-
# my $baseData = $baseline->data($f);
3842-
# my $baseLocation = join(":", ${$baseData->location()});
3843-
# my $currData = $current->data($f);
3844-
# my $currLocation = join(":", ${$currData->location()});
3845-
3846-
if (2 == $missed{$f}) {
3847-
$type = "baseline";
3848-
} elsif (1 == $missed{$f}) {
3849-
$type = "current";
3850-
} else {
3851-
$type = "both baseline and current";
3852-
}
3853-
my $single = 1 == scalar(@unused);
3854-
# @todo could print line numbers in baseline, current .info files and
3855-
# in diff file ..
3856-
if (lcovutil::warn_once($lcovutil::ERROR_MISMATCH, $f)) {
3857-
my $suffix =
3858-
$main::elide_path_mismatch ? ' (elided)' :
3859-
lcovutil::explain_once(
3860-
'elide-path-mismatch',
3861-
' Perhaps see "--elide-path-mismatch", "--substitute" and "--build-directory" options in \'man '
3862-
. $lcovutil::tool_name . '\'');
3863-
lcovutil::ignorable_warning(
3824+
next unless exists($diffBaseMap{$b});
3825+
3826+
# this basename is in the diff file and didn't match any other
3827+
# trace filename entry (i.e., same filename in more than one
3828+
# source code directory) - then warn about possible pathname issue
3829+
my ($diffFiles, $sourceFiles) = @{$diffBaseMap{$b}};
3830+
# find the files which appear in the 'diff' list which have the
3831+
# same basename and were not matched - those might be candidates
3832+
my @unused;
3833+
for my $d (@$diffFiles) {
3834+
# my $location = $self->[DIFF_FILENAME] . ':' . $self->[LOCATION]->[NEW]->{$d};
3835+
push(@unused, $d)
3836+
unless exists($sourceFiles->{$d});
3837+
}
3838+
next unless @unused;
3839+
3840+
# my $baseData = $baseline->data($f);
3841+
# my $baseLocation = join(":", ${$baseData->location()});
3842+
# my $currData = $current->data($f);
3843+
# my $currLocation = join(":", ${$currData->location()});
3844+
3845+
my $type;
3846+
if (2 == $missed{$f}) {
3847+
$type = "baseline";
3848+
} elsif (1 == $missed{$f}) {
3849+
$type = "current";
3850+
} else {
3851+
$type = "both baseline and current";
3852+
}
3853+
my $single = 1 == scalar(@unused);
3854+
# @todo could print line numbers in baseline, current .info files and
3855+
# in diff file ..
3856+
if (lcovutil::warn_once($lcovutil::ERROR_MISMATCH, $f)) {
3857+
my $suffix =
3858+
$main::elide_path_mismatch ? ' (elided)' :
3859+
lcovutil::explain_once(
3860+
'elide-path-mismatch',
3861+
' Perhaps see "--elide-path-mismatch", "--substitute" and "--build-directory" options in \'man '
3862+
. $lcovutil::tool_name . '\'');
3863+
lcovutil::ignorable_warning(
38643864
$lcovutil::ERROR_MISMATCH,
38653865
"source file '$f' (in $type .info file" .
38663866
($missed{$f} == 3 ? "s" : "") .
@@ -3874,16 +3874,14 @@ sub check_path_consistency
38743874
($single ? " " : "\n\t") .
38753875
'Possible pathname mismatch?' .
38763876
$suffix);
3877-
}
3878-
if ($main::elide_path_mismatch &&
3879-
$missed{$f} == 3 &&
3880-
$single) {
3881-
$self->[FILEMAP]->{$f} = $f;
3882-
$self->[LINEMAP]->{$f} = $self->[LINEMAP]->{$unused[0]};
3883-
} else {
3884-
$ok = 0;
3885-
}
3886-
}
3877+
}
3878+
if ($main::elide_path_mismatch &&
3879+
$missed{$f} == 3 &&
3880+
$single) {
3881+
$self->[FILEMAP]->{$f} = $f;
3882+
$self->[LINEMAP]->{$f} = $self->[LINEMAP]->{$unused[0]};
3883+
} else {
3884+
$ok = 0;
38873885
}
38883886
}
38893887
return $ok;

Diff for: scripts/gitdiff

+14-10
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use Getopt::Long;
44
use strict;
5+
use Cwd /realpath/;
56

67
my $verbose = 0;
78

@@ -39,18 +40,20 @@ my @include_patterns;
3940
my $suppress_unchanged;
4041
my $prefix = '';
4142
my $ignore_whitespace;
43+
my $repo = '.'; # cwd
4244

4345
if (!GetOptions("exclude=s" => \@exclude_patterns,
4446
"include=s" => \@include_patterns,
4547
'no-unchanged' => \$suppress_unchanged,
4648
'prefix=s' => \$prefix,
4749
'b|blank' => \$ignore_whitespace,
50+
'repo=s' => \$repo,
4851
'verbose+' => \$verbose) ||
4952
(2 != scalar(@ARGV) &&
5053
3 != scalar(@ARGV))
5154
) {
5255
print(STDERR
53-
"usage: [(--exclude|include) regexp[,regexp]] [-b] [dir] base_changelist current_changelist\n'exclude' wins if both exclude and include would match.\n"
56+
"usage: [(--exclude|include) regexp[,regexp]] [--repo repo] [-b] [dir] base_changelist current_changelist\n'exclude' wins if both exclude and include would match.\n"
5457
);
5558
exit(1);
5659
}
@@ -71,15 +74,16 @@ if ($verbose) {
7174
}
7275

7376
$prefix .= '/' if $prefix;
74-
my $cmd;
7577
if (3 == scalar(@ARGV)) {
7678
my $dir = shift @ARGV;
7779
push(@include_patterns, "$dir");
7880
}
7981
my $base_sha = shift @ARGV;
8082
my $current_sha = shift @ARGV;
8183

82-
$cmd = "git diff $ignore_whitespace $base_sha $current_sha";
84+
$repo = Cwd::realpath($repo);
85+
86+
my $cmd = "cd $repo ; git diff $ignore_whitespace $base_sha $current_sha";
8387

8488
open(HANDLE, "-|", $cmd) or
8589
die("failed to exec git diff: $!");
@@ -104,8 +108,10 @@ while (<HANDLE>) {
104108
$includeCurrentFile =
105109
(include_me($fileA, \@include_patterns, \@exclude_patterns) ||
106110
include_me($fileB, \@include_patterns, \@exclude_patterns));
107-
$allFiles{$fileB} = $fileA
108-
if ($includeCurrentFile);
111+
if ($includeCurrentFile) {
112+
$allFiles{$fileB} = $fileA;
113+
$line =~ s/($fileA|$fileB)/$1/g;
114+
}
109115
}
110116

111117
printf("%s\n", $line)
@@ -119,19 +125,17 @@ if (0 != $?) {
119125

120126
exit 0 if defined($suppress_unchanged);
121127

122-
my $p = `git rev-parse --show-prefix`;
123-
chomp $p;
124-
125128
# now find the list of files in the current SHA - we can process that
126129
# to find the list of all current files which were unchanged since the
127130
# baseline SHA
128131
die("failed to exec git ls-tree")
129-
unless (open(HANDLE, "-|", "git ls-tree -r --name-only $current_sha"));
132+
unless (
133+
open(HANDLE, "-|", "cd $repo ; git ls-tree -r --name-only $current_sha"));
130134

131135
while (<HANDLE>) {
132136
chomp($_);
133137
s/\r//g;
134-
my $filename = $p . $_;
138+
my $filename = $repo . '/' . $_;
135139
my $path = $prefix . $filename;
136140
if (!exists($allFiles{$path}) &&
137141
include_me($path, \@include_patterns, \@exclude_patterns)) {

0 commit comments

Comments
 (0)