David Rowley <dgrowle...@gmail.com> writes:
> On Thu, 15 Jul 2021 at 04:01, vignesh C <vignes...@gmail.com> wrote:
>> The patch does not apply on Head anymore, could you rebase and post a
>> patch. I'm changing the status to "Waiting for Author".
>
> I've rebased this patch and broken it down into 6 individual patches.
I don't know anything about the MSVC build process, but I figured I
could do a general Perl code review.
> --- 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 [+:]?
> --- 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(…).
> --- 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.
> --- 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;
> +}
- ilmari
