Re: Reduce the number of special cases to build contrib modules on windows

David Rowley <dgrowleyml@gmail.com> writes:

> On Thu, 15 Jul 2021 at 04:01, vignesh C <vignesh21@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