Re: PGDLLEXPORTing all GUCs?

On 05/08/2014 10:53 AM, Tom Lane wrote:
> Craig Ringer <craig(at)2ndquadrant(dot)com> writes:
>> On 05/08/2014 12:21 AM, Tom Lane wrote:
>>> If Craig has a concrete argument why all GUCs should be accessible
>>> to external modules, then let's see it
>
>> As for just GUCs: I suggested GUCs because GUCs are what's been coming
>> up repeatedly as an actual practical issue.
>
> Meh. A quick look through the commit logs says that GUC variables are not
> more than 50% of what we've had to PGDLLIMPORT'ify in the past year or
> two. Maybe that's different from 2ndQuadrant's internal experience,
> but then you've not showed us the use-case driving your changes.

That's because the use case isn't that interesting, really, only the
hiccups it's caused. I'd just as happily mark all extern vars
PGDLLIMPORT, and only suggested focusing on GUCs because I didn't expect
to get far with what I really thought was best.

Re your concerns with exposing GUCs that should by rights be internal to
the wider world, it'd be interesting to mark functions and vars as
something like PGINTERNAL, to expand to:
__attribute__((visibility ("hidden"))
on gcc, so they're just not available for linkage in extensions. That's
a weaker form of using -fvisibility=hidden, where we explicitly say
"this is private" rather than treating everything as private unless
explicitly marked public, which has already been rejected.

Right now they're already exported for !windows, and while it's IMO a
bug to have that difference for windows, it doesn't mean the correct
answer is to export for all. If we're confident it won't break anything
interesting it'd be OK to instead say "unexport on !windows too".

In terms of ugliness, would you be happier using a pg_extern instead of
extern, where we:

#ifdef WIN32
#define pg_extern extern PGDLLIMPORT
#else
#define pg_extern extern
#endif

?

That makes it easier to pretend that there's nothing windows-y going on
- and despite appearances, I'm also pretty keen not to have to think
about that platform's linkage horrors when I don't have to.

However, it also makes backpatching ickier.

>> I'd be quite happy to
>> PGDLLEXPORT all extern vars, but I was confident that'd be rejected for
>> aesthetic reasons, and thought that exporting all GUCs would be a
>> reasonable compromise.
>
> From the aesthetic standpoint, what I'd like is to not have to blanket
> our source code with Windows-isms. But I guess I can't have that.

I'd rather prefer that as well, but without the ability to go knocking
at Redmond and introduce them to ELF and sane linkage, I don't think any
of us are going to get it.

If it weren't for backbranches etc the first thing I'd do to make it
less ugly personally would be to rename PGDLLIMPORT as PGEXPORT and
BUILDING_DLL as BUILDING_POSTGRES . The current names are unfortunate.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services