Re: Reduce the number of special cases to build contrib modules on windows
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2020-11-10T22:01:57Z
Lists: pgsql-hackers
Attachments
- reduce_contrib_build_special_cases_on_windows_v3.patch (text/plain) patch v3
- vcxproj_diffs.patch.txt (text/plain)
On Tue, 10 Nov 2020 at 03:07, Alvaro Herrera <alvherre@alvh.no-ip.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
Commits
-
Remove some special cases from MSVC build scripts
- 76ad24400d73 15.0 landed
-
Adjust MSVC build scripts to parse Makefiles for defines
- 245de48455da 15.0 landed
-
Don't duplicate references and libraries in MSVC scripts
- 15f16ec6511c 15.0 landed
-
Make the includes field an array in MSVC build scripts
- 33d74c5d00e4 15.0 landed
-
Use the AddFile function consistently in MSVC build scripts
- ed1884a2fede 15.0 landed
-
Remove seemingly unneeded include directory in MSVC scripts
- 4b763ff642e1 15.0 landed