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 |
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() |