On Wed, 28 Jul 2021 at 01:44, Dagfinn Ilmari Mannsåker <ilm...@ilmari.org> wrote: > I don't know anything about the MSVC build process, but I figured I > could do a general Perl code review.
Thanks for looking at this. Perl review is very useful as it's certainly not my native tongue, as you might have noticed. > > --- a/src/tools/msvc/Mkvcbuild.pm > > +++ b/src/tools/msvc/Mkvcbuild.pm > […] > > + # Process custom compiler flags > > + if ($mf =~ /^PG_CPPFLAGS\s*=\s*(.*)$/mg || $mf =~ > > /^override\s*CPPFLAGS\s*(?:[\+\:])?=\s*(.*)$/mg) > > ^^^^^^^^^^^ > This is a very convoluted way of writing [+:]? I've replaced the (?:[\+\:])? with [+:]? It's a bit of a blind adjustment. I see that the resulting vcxproj files have not changed as a result of that. > > --- a/src/tools/msvc/Project.pm > > +++ b/src/tools/msvc/Project.pm > > @@ -58,7 +58,7 @@ sub AddFiles > > > > while (my $f = shift) > > { > > - $self->{files}->{ $dir . "/" . $f } = 1; > > + AddFile($self, $dir . "/" . $f, 1); > > AddFile is a method, so should be called as $self->AddFile(…). Adjusted thanks. > > --- a/src/tools/msvc/Mkvcbuild.pm > > +++ b/src/tools/msvc/Mkvcbuild.pm > > @@ -36,16 +36,12 @@ my @unlink_on_exit; > […] > > + elsif ($flag =~ /^-I(.*)$/) > > + { > > + foreach my $proj (@projects) > > + { > > + if ($1 eq '$(libpq_srcdir)') > > + { > > + > > $proj->AddIncludeDir('src\interfaces\libpq'); > > + $proj->AddReference($libpq); > > + } > > + } > > + } > > It would be better to do the if check outside the for loop. Agreed. > > --- a/src/tools/msvc/Project.pm > > +++ b/src/tools/msvc/Project.pm > > @@ -51,6 +51,16 @@ sub AddFile > > return; > > } > > > > +sub AddFileAndAdditionalFiles > > +{ > > + my ($self, $filename) = @_; > > + if (FindAndAddAdditionalFiles($self, $filename) != 1) > > Again, FindAndAddAdditionalFiles is a method and should be called as > $self->FindAndAddAdditionalFiles($filename). > > > + { > > + $self->{files}->{$filename} = 1; > > + } > > + return; > > +} Adjusted. I'll send updated patches once I look at the other reviews. David