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