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: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2020-12-29T21:03:34Z
Lists: pgsql-hackers

Attachments

On Wed, 23 Dec 2020 at 18:46, Michael Paquier <michael@paquier.xyz> wrote:
> I have begun a new thread about this point as that's a separate
> topic.  I did not see other places in need of a similar cleanup:
> https://www.postgresql.org/message-id/X+LQpfLyk7jgzUki@paquier.xyz

Thanks. I'll look at that shortly.

> > I didn't look in detail, but it looks like if we define LOWER_NODE on
> > Windows that it might break pg_upgrade.  I guess you could say it's
> > partially broken now as the behaviour there will depend on if you
> > build using Visual Studio or cygwin.  We'd define LOWER_NODE on cygwin
> > but not on VS.  Looks like a pg_upgrade might be problematic there
> > today.
> >
> > It feels a bit annoying to add some special case to the script to
> > maintain the status quo there.  An alternative to that would be to
> > modify the .c code at #ifdef LOWER_NODE to also check we're not
> > building on VS. Neither option seems nice.
>
> Hmm.  It seems that you are right here.  This influences lquery
> parsing so it may be nasty and this exists since ltree is present in
> the tree (2002).  I think that I would choose the update in the C code
> and remove LOWER_NODE while keeping the scripts clean, and documenting
> directly in the code why this compatibility issue exists.
> REFINT_VERBOSE is no problem, fortunately.

I ended up modifying each place in the C code where we check
LOWER_NODE.  I found 2 places, one in crc32.c and another in ltree.h.
I added the same comment to both to explain why there's a check for
!defined(_MSC_VER) there. I'm not particularly happy about this code,
but I don't really see what else to do right now.

> I have tested your patch, and this is causing compilation failures for
> hstore_plpython, jsonb_plpython and ltree_plpython.  So
> AddTransformModule is missing something here when compiling with
> Python.

Oh thanks for finding that.  That was due to some incorrect Perl code
I'd written to add the includes from one project into another. Fixed
by:

-       $p->AddIncludeDir(join(";", $pl_proj->{includes}));
+       foreach my $inc (keys %{ $pl_proj->{includes} } )
+       {
+               $p->AddIncludeDir($inc);
+       }
+

David

Commits

  1. Remove some special cases from MSVC build scripts

  2. Adjust MSVC build scripts to parse Makefiles for defines

  3. Don't duplicate references and libraries in MSVC scripts

  4. Make the includes field an array in MSVC build scripts

  5. Use the AddFile function consistently in MSVC build scripts

  6. Remove seemingly unneeded include directory in MSVC scripts