Re: Reduce the number of special cases to build contrib modules on windows - Mailing list pgsql-hackers

From David Rowley
Subject Re: Reduce the number of special cases to build contrib modules on windows
Date
Msg-id CAApHDvrYvE0ecA-+VMktgripCRmoeGKDcvdCKWPmgHw-Og78aA@mail.gmail.com
Whole thread Raw
In response to Re: Reduce the number of special cases to build contrib modules on windows  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Reduce the number of special cases to build contrib modules on windows
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Let's start using setenv()
Next
From: David Rowley
Date:
Subject: Re: Reduce the number of special cases to build contrib modules on windows