Re: dynloader.h missing in prebuilt package for Windows? - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: dynloader.h missing in prebuilt package for Windows?
Date
Msg-id CAB7nPqRvg2jGOW7sKAr6_ZVwUJqSkDgy8zSC7BW3ZWfttVr8fw@mail.gmail.com
Whole thread Raw
In response to Re: dynloader.h missing in prebuilt package for Windows?  (Chapman Flack <chap@anastigmatix.net>)
Responses Re: dynloader.h missing in prebuilt package for Windows?  (Chapman Flack <chap@anastigmatix.net>)
List pgsql-hackers
On Mon, Jan 4, 2016 at 9:37 PM, Chapman Flack <chap@anastigmatix.net> wrote:
> On 01/05/16 00:18, Michael Paquier wrote:
>
>> with MSVC what is used instead of dynloader.h is
>> port/dynloader/win32.h.
>
> Seems like a good catch - AFAICS, what happens with port/dynloader
> is that for 12 different OSes, there's an <osname>.h file there to
> be copied _renamed to dynloader.h_ into the build tree, and a .c
> file expecting similar treatment, and that the problem that kicked
> off this whole thread was just that the windows build process (and
> only the windows build process) was neglecting to do that.

Yep, via ./configure.

> And so I was pretty sure the fix was to make the windows build do
> that, and then it would be doing the same thing as the other eleven,
> but I just looked at Bruce's patch more closely and it does seem to
> be doing something else instead.

Bruce's patch was incomplete, it forgot to copy the header file to
include/ to installation actually failed. That's the equivalent of
configure, but for msvc.

>> Instead of this patch I would be incline to
>> remove the #define stuff with dynloader.h that use WIN32_ONLY_COMPILER
>> (see for example dfmgr.c) and just copy the header in include/. This
>> way we use the same header for all platforms.
>
> But this part I'm not sure I follow (and maybe I don't need to follow
> it, you guys know your code better than I do) ... in this whole thread
> haven't we been talking about just making the windows build copy its
> port/dynloader files the same way the other eleven platforms do, because
> it wasn't, and that seemed to be an omission, and worth not continuing
> to omit?

That's not a mandatory fix, but I think we had better do it. As long
as dynloader.h is copied in include/, there is no need to keep the
tweak of dfmgr.c to include the definitions those routines.
-- 
Michael



pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: dynloader.h missing in prebuilt package for Windows?
Next
From: Tom Lane
Date:
Subject: Re: Keyword classifications