Thread: PGDLLIMPORT: patch or not to patch

PGDLLIMPORT: patch or not to patch

From
George Tarasov
Date:
Dear all!

(comment: my question relates only to the development area; so, please, 
re-post to pgsql-hackers if it is allowed).

I use PostgreSQL under Windows quiet often and make my own builds in 
msys2/mingw64 environment. Also I often experiments with the different 
third-party extensions from the community. And as a result I often faces 
with a PGDLLIMPORT macro to link all libraries and extensions with the 
core postgres.exe.

For some extensions it is required to patch core and these extensions 
insert missing PGDLLIMPORTs by itself. Other extensions are not ported 
under Windows and I prepare my own patches (insert PGDLLIMPORTs) to 
adopt its under my build environment and link shared libraries correctly.

My last case was the "extern ProcessingMode Mode;" variable in the 
miscadmin.h which i patched by PGDLLIMPORT macro. I have discovered the 
same modifications in some other extensions.

So, my questions are there any rules / descriptions / agreements inside 
the PostgreSQL Project that define which global variables inside a core 
code should by specified by a PGDLLIMPORT and which should not?? Or 
there is freedom; you need this variable in the extension (under 
Windows), make patch for it yourself! Or there is plan in the community 
that all global non-static variables should be PGDLLIMPORT-ed by default 
in the future?? What the right way to propose the PGDLLIMPORT patch to 
the master and back-ported PostgreSQL code in order to avoid dup patches 
in the extensions?

Thank you!

Regards,
George Tarasov




Re: PGDLLIMPORT: patch or not to patch

From
Tom Lane
Date:
George Tarasov <george.v.tarasov@gmail.com> writes:
> So, my questions are there any rules / descriptions / agreements inside 
> the PostgreSQL Project that define which global variables inside a core 
> code should by specified by a PGDLLIMPORT and which should not?? Or 
> there is freedom; you need this variable in the extension (under 
> Windows), make patch for it yourself! Or there is plan in the community 
> that all global non-static variables should be PGDLLIMPORT-ed by default 
> in the future?? What the right way to propose the PGDLLIMPORT patch to 
> the master and back-ported PostgreSQL code in order to avoid dup patches 
> in the extensions?

Our policy so far has been to add PGDLLIMPORT to variables for which
someone makes a case that an extension would have a reasonable use
for it.  The bar's not terribly high, but it does exist.  The idea of
just doing a blanket s/extern/extern PGDLLIMPORT/g has been discussed
and rejected, because we don't want to commit to supporting absolutely
every global variable as something that's okay for extensions to touch.

So if you've got specific proposals (such as "Mode"), bring them up
on pgsql-hackers.

            regards, tom lane



Re: PGDLLIMPORT: patch or not to patch

From
Craig Ringer
Date:
On Wed, 30 Jun 2021 at 04:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:
George Tarasov <george.v.tarasov@gmail.com> writes:
> So, my questions are there any rules / descriptions / agreements inside
> the PostgreSQL Project that define which global variables inside a core
> code should by specified by a PGDLLIMPORT and which should not?? Or
> there is freedom; you need this variable in the extension (under
> Windows), make patch for it yourself! Or there is plan in the community
> that all global non-static variables should be PGDLLIMPORT-ed by default
> in the future?? What the right way to propose the PGDLLIMPORT patch to
> the master and back-ported PostgreSQL code in order to avoid dup patches
> in the extensions?

Our policy so far has been to add PGDLLIMPORT to variables for which
someone makes a case that an extension would have a reasonable use
for it.  The bar's not terribly high, but it does exist.  The idea of
just doing a blanket s/extern/extern PGDLLIMPORT/g has been discussed
and rejected, because we don't want to commit to supporting absolutely
every global variable as something that's okay for extensions to touch.

I agree that it doesn't make sense to mark all of them as a blanket rule.

I'd like to explicitly tag *non*-exported externs as __attribute__(("hidden")) on GCC-alike ELF systems to ensure that extension authors don't rely on them then later find they cannot be used on Windows. Obviously wrapped in some PG_NO_EXPORT or PG_DLL_HIDDEN macro.

I'm updating a patch at the moment that makes all GUC storage and most variables computed from GUCs during hook execution PGDLLIMPORT. It might make sense to follow that up with a patch to make non-export vars hidden. But I vaguely recall raising this before and some folks not being a fan of the extra noise on each line?