Re: dynloader.h missing in prebuilt package for Windows?

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Chapman Flack <chap(at)anastigmatix(dot)net>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Olson, Ken" <Ken(dot)Olson(at)pega(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dynloader.h missing in prebuilt package for Windows?
Date: 2016-01-05 05:53:01
Message-ID: CAB7nPqRvg2jGOW7sKAr6_ZVwUJqSkDgy8zSC7BW3ZWfttVr8fw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jan 4, 2016 at 9:37 PM, Chapman Flack <chap(at)anastigmatix(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-01-05 05:53:04 Re: Keyword classifications
Previous Message Michael Paquier 2016-01-05 05:50:40 Re: dynloader.h missing in prebuilt package for Windows?