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

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, 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-12-29 21:03:34
Message-ID: CAApHDvrYvE0ecA-+VMktgripCRmoeGKDcvdCKWPmgHw-Og78aA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 23 Dec 2020 at 18:46, Michael Paquier <michael(at)paquier(dot)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 Content-Type Size
reduce_contrib_build_special_cases_on_windows_v5.patch text/plain 1.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2020-12-29 21:07:29 Re: Reduce the number of special cases to build contrib modules on windows
Previous Message Thomas Munro 2020-12-29 20:34:59 Re: Let's start using setenv()