Re: Mark all GUC variable as PGDLLIMPORT

Hi,

Sorry for taking so much time to answer here.  I definitely wanted to work on
that but I was under the impression that although we now had an agreement to
apply PGDLLIMPORT globally a patch itself wasn't really rushed, so I spent the
last two days trying to setup a new Windows environment as my previous one
isn't working anymore.  That's taking much longer than I planned.

On Tue, Feb 15, 2022 at 08:06:58AM -0800, Andres Freund wrote:
> Hi,
> 
> On 2022-02-15 08:58:05 -0500, Robert Haas wrote:
> > On Mon, Feb 14, 2022 at 8:53 PM Andres Freund <andres@anarazel.de> wrote:
> > > > An alternative rule which would dodge that particular issue would be
> > > > to just slap PGDLLIMPORT on every global variable in every header
> > > > file. That would arguably be a simpler rule, though it means even more
> > > > PGDLLIMPORT declarations floating around.
> > >
> > > That would have the advantage of being comparatively easy to check in an
> > > automated way. Might even be cheap enough to just make it part of the build.
> > 
> > I wasn't able to quickly write something that was smart enough to use
> > as part of the build, but I wrote something dumb that I think works
> > well enough to use with a little bit of human intelligence alongside.
> > See attached.
> 
> Cool.

+1

> > > But it seems like it'd be a fair amount of work and cause a lot of patch
> > > rebasing pain? If we end up going that way, we should schedule this to happen
> > > just after the feature freeze, I think.
> > 
> > We could do that. I'd sort of rather get it done. We still have two
> > weeks before the last CommitFest officially starts, and it's not as if
> > there won't be tons of uncommitted patches floating around after that.
> 
> Breaking close to every patch 6-7 weeks before freeze doesn't seem
> great. Particularly because this is a mostly automatically generated patch, so
> I don't really see a forcing function to do this now, rather than at a more
> opportune moment.

Agreed, especially if we can do some cleanup in gendef.pl.

> The more I think about it the more I'm convinced that if we want to do this,
> we should do it for variables and functions.
> 
> 
> > > If we consider doing this for all extern variables, we should think about
> > > doing this for headers *and* functions. That'd allow us to get rid of the
> > > fairly complicated logic to generate the .def file for the postgres binary on
> > > windows (src/tools/gendef.pl). And maybe also the related thing on AIX
> > > (src/backend/port/aix/mkldexport.sh)
> > 
> > I don't know what you mean by this.
> 
> We only need gendef.pl because we don't mark functions with visibility
> information. If we attach PGDLLIMPORT to functions and variables we can a fair
> bit of complexity.  And I think not just for windows, but also AIX (there's a
> whole section in src/backend/Makefile about AIX oddity), but I'm not sure
> about it.

I will try to have a look at it once my build environment is ready.