On 3/30/22 14:37, Robert Haas wrote:
> On Fri, Feb 18, 2022 at 7:02 PM Andres Freund <andres@anarazel.de> wrote:
>> On 2022-02-15 08:06:58 -0800, Andres Freund wrote:
>>> 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.
>> Btw, if we were to do this, we should just use -fvisibility=hidden everywhere
>> and would see the same set of failures on unixoid systems as on windows. Of
>> course only in in-core extensions, but it'd still be better than nothing.
> Let's be less ambitious for this release, and just get the variables
> marked with PGDLLIMPORT. We seem to have consensus to create parity
> between Windows and non-Windows builds, which means precisely applying
> PGDLLIMPORT to variables marked in header files, and nothing more. The
> merits of -fvisibility=hidden or PGDLLIMPORT on functions are a
> separate question that can be debated on its own merits, but I don't
> want that larger discussion to bog down this effort. Here are updated
> patches for that.
>
> @RMT: Andres proposed upthread that we should plan to do this just
> after feature freeze. Accordingly I propose to commit at least 0002
> and perhaps 0001 if people want it just after feature freeze. I
> therefore ask that the RMT either (a) regard this change as not being
> a feature (and thus not subject to the freeze) or (b) give it a 1-day
> extension. The reason for committing it just after freeze is to
> minimize the number of conflicts that it creates for other patches.
> The reason why that's probably an OK thing to do is that applying
> PGDLLIMPORT markings is low-risk.
>
> Thanks,
>
WFM. I think Tom also has an item he wants to do right at the end of
feature freeze.
The script looks fine, needs a copyright notice and a comment at the top
describing what it does. It seems like something we might need to do
from time to time, as it will be easy to forget to mark variables and we
should periodically run this as a check.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
