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

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Reduce the number of special cases to build contrib modules on windows
Date: 2020-11-10 22:01:57
Message-ID: CAApHDvokVHhOP1x2A1dpBaWP+9UDUmL4fKY_k-u-Bu8+NyBOHg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 10 Nov 2020 at 03:07, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2020-Nov-06, David Rowley wrote:
>
> > +# Handle makefile rules for when file to be added to the project
> > +# does not exist. Returns 1 when the original file add should be
> > +# skipped.
> > +sub FindAndAddAdditionalFiles
> > +{
> > + my $self = shift;
> > + my $fname = shift;
> > + my ($ext) = $fname =~ /(\.[^.]+)$/;
> > +
> > + # For .c files, check if a .l file of the same name exists and add that
> > + # too.
> > + if ($ext eq ".c")
> > + {
> > + my $filenoext = $fname;
> > + $filenoext =~ s{\.[^.]+$}{};
>
> I think you can make this simpler by capturing both the basename and the
> extension in one go. For example,
>
> $fname =~ /(.*)(\.[^.]+)$/;
> $filenoext = $1;
> $ext = $2;
>
> so you avoid the second =~ statement.

Thanks. That's neater.

> > + if (-e "$filenoext.l")
> > + {
> > + AddFileConditional($self, "$filenoext.l");
> > + return 1;
> > + }
> > + if (-e "$filenoext.y")
> > + {
> > + AddFileConditional($self, "$filenoext.y");
>
> Maybe DRY like
>
> for my $ext (".l", ".y") {
> my $file = $filenoext . $ext;
> AddFileConditional($self, $file) if -f $file;
> return 1;
> }

I did adapt that part of the code, but not exactly to what's above.
The return there would cause us to return from the function after the
first iteration.

> Note: comment says "check if a .l file" and then checks both .l and .y.
> Probably want to update the comment ...

Updated.

I've attached the v3 patch.

I'm still working through some small differences in some of the
.vcxproj files. I've been comparing these by copying *.vcxproj out to
another directory with patched and unpatched then diffing the
directory. See attached txt file with those diffs. Here's a summary of
some of them:

1. There are a few places that libpq gets linked where it previously did not.
2. REFINT_VERBOSE gets defined in a few more places than it did
previously. This makes it closer to what happens on Linux anyway, if
you look at the Make output from contrib/spi/Makefile you'll see
-DREFINT_VERBOSE in there for autoinc
3. LOWER_NODE gets defined in ltree now where it wasn't before. It's
defined on Linux. Unsure why it wasn't before on Windows.

David

Attachment Content-Type Size
reduce_contrib_build_special_cases_on_windows_v3.patch text/plain 5.5 KB
vcxproj_diffs.patch.txt text/plain 22.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2020-11-10 22:28:14 Re: Support for NSS as a libpq TLS backend
Previous Message David G. Johnston 2020-11-10 21:58:26 Re: Additional Chapter for Tutorial