Re: Random PGDLLIMPORTing

On Fri, Nov 25, 2016 at 9:54 AM, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:

> On 25 November 2016 at 14:16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Craig Ringer <craig(at)2ndquadrant(dot)com> writes:
> >> PGDLLIMPORT is free, so the question should be "is there a reason not
> >> to add it here?".
> >
> > TBH, my basic complaint about it is that I do not like Microsoft's tool
> > chain assuming that it's entitled to demand that people sprinkle
> > Microsoft-specific droppings throughout what would otherwise be platform
> > independent source code.
>
> Yeah, I know how you feel. I'm not a fan myself, I just tolerate it as
> one of those annoying things we must put up with, like Perl 5.8 for
> ancient systems, not using C99, and !Linux/FBSD support. Which,
> actually, we _do_ actively work for - take for example the atomics
> work, which went to considerable lengths not to break weird niche
> platforms.
>
> > However, Victor Wagner's observation upthread is quite troubling:
> >
> >>> It worth checking actual variable definitions, not just declarations.
> >>> I've found recently, that at least in MSVC build system, only
> >>> initialized variables are included into postgres.def file, and so are
> >>> actually exported from the backend binary.
> >
> > If this is correct (don't ask me, I don't do Windows) then the issue is
> > not whether "PGDLLIMPORT is free". This puts two separate source-code
> > demands on variables that we want to make available to extensions,
> neither
> > of which is practically checkable on non-Windows platforms.
>
> I agree that, if true, that's a real concern and something that needs
> addressing to avoid a growing maintenance burden from Windows.
>

It would probably not be very hard to create a test that just scans the
sourcecode for PGDLLIMPORT:ed variables and generates a C file that links
to them all, tries to build that and sees if it blows up. If I understand
the issue correctly, that should fail in these cases. So we could make that
a platform specific test that runs, and have the buildfarm run it so we'd
at least detect such problems early. Or am I missing something?

> > I think that basically it's going to be on the heads of people who
> > want to work on Windows to make sure that things work on that platform.
> > That is the contract that every other platform under the sun understands,
> > but it seems like Windows people think it's on the rest of us to make
> > their platform work. I'm done with that.
>
> Well, I'm trying to be one of those "Windows people" to the degree of
> spotting and addressing issues. Like this one. But it's not worth
> arguing this one if it's more than a trivial "meh, done" fix, since
> it's likely to only upset code that's testing assertions (like BDR or
> pglogical).
>
>
So when we *do* identify these places, through projects like that or just
general complaints, do we see any actual risk in backpatching such
additions? As long as the linking is done by name and not by number, it
should be fully safe, right?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/