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: | 2021-03-03 09:37:56 |
Message-ID: | CAApHDvq=pXiGWkqPRON8gzK86cCoxOtpUyqPom9qpf-dX8dADg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thank you for having another look at this.
On Tue, 12 Jan 2021 at 20:18, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Dec 30, 2020 at 10:07:29AM +1300, David Rowley wrote:
> > -#ifdef LOWER_NODE
> > +/*
> > + * Below we ignore the fact that LOWER_NODE is defined when compiling with
> > + * MSVC. The reason for this is that earlier versions of the MSVC build
> > + * scripts failed to define LOWER_NODE. More recent version of the MSVC
> > + * build scripts parse makefiles which results in LOWER_NODE now being
> > + * defined. We check for _MSC_VER here so as not to break pg_upgrade when
> > + * upgrading from versions MSVC versions where LOWER_NODE was not defined.
> > + */
> > +#if defined(LOWER_NODE) && !defined(_MSC_VER)
> > #include <ctype.h>
> > #define TOLOWER(x) tolower((unsigned char) (x))
> > #else
>
> While on it, do you think that it would be more readable if we remove
> completely LOWER_NODE and use only a check based on _MSC_VER for those
> two files in ltree? This could also be handled as a separate change.
I'm hesitant to touch that. If anyone is running an instance compiled
with a non-default LOWER_NODE then we might give them some trouble if
they pg_upgrade their database later.
> > + foreach my $line (split /\n/, $mf)
> > + {
> > + if ($line =~ /^[A-Za-z0-9_]*\.o:\s(.*)/)
> > + {
> > + foreach my $file (split /\s+/, $1)
> > + {
> > + foreach my $proj (@projects)
> > + {
> > + $proj->AddFileConditional("$subdir/$n/$file");
> > + }
> > + }
> > + }
> > + }
> Looking closer at this change, I don't think that this is completely
> correct and that could become a trap. This is adding quite a bit of
> complexity to take care of contrib_extrasource getting empty, and it
> actually overlaps with the handling of OBJS done in AddDir(), no?
hmm. I'm not quite sure if I know what you mean by "trap" here.
contrib/cube/Makefile has an example of what this is trying to catch:
# cubescan is compiled as part of cubeparse
cubeparse.o: cubescan.c
I don't really see what other options there are apart from just not
get rid of $contrib_extrasource.
Can you give an example of what sort of scenario you've got in mind
where it'll cause issues?
I've attached a rebased patch.
David
Attachment | Content-Type | Size |
---|---|---|
reduce_contrib_build_special_cases_on_windows_v7.patch | text/plain | 10.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2021-03-03 09:39:55 | Re: PROXY protocol support |
Previous Message | Gilles Darold | 2021-03-03 09:22:16 | Re: [PATCH] proposal for regexp_count, regexp_instr, regexp_substr and regexp_replace |