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

dynloader.h missing in prebuilt package for Windows?

From
Chapman Flack
Date:
Hi,

This is my first -hackers message; I've recently been putting some effort
into PL/Java since this summer (my employer published a restated IP policy
that seems much friendlier toward FOSS contributions on my own time, so
my PL/Java contributions will be seen to have ticked up since then).

Ken Olson has helped me greatly by testing on Windows, and he noticed
something odd: #include <server/dynloader.h> fails on Windows when building
an extension out-of-tree, simply because that file isn't there. He tells me
that a lot of sites using PG on Windows will have obtained it from an EDB
binary distribution, so I am not sure whether that file is missing because
of PG's Windows build tooling, or because of something in the way EDB makes
their packages.

As far as either of us can tell, the <dynloader.h> file distributed for
any given platform is one of the templates in backend/port/dynloader,
and (on platforms that use configure), the proper one is chosen by
configure, and ends up supplied as dynloader.h in postgresql-devel
sorts of packages, so it can be seen when building extensions out-of-tree.

There is a win32.h in backend/port/dynloader, and Ken got compilation to
succeed by duplicating the contents of that file in place of the #include,
so it seems that is the file that *should* become <dynloader.h> in a
Windows package. I do notice there is a tools/msvc/Mkvcbuild.pm with code
in it to make use of the backend/port/dynloader/win32.c file, but it
makes no mention of the .h file.

Am I right in thinking some version of <server/dynloader.h> is intended
to be present on every platform, and its absence on Windows is simply
an oversight in building/packaging? The compiler seems happy with

#ifndef _MSC_VER
#include <server/dynloader.h>
#else
... pasted copy of win32.h from the source tree ...
#endif

but I assume it's preferable to have the same code work on Windows as on
other platforms when possible.

Regards,
Chapman Flack



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

From
Tom Lane
Date:
Chapman Flack <chap@anastigmatix.net> writes:
> Ken Olson has helped me greatly by testing on Windows, and he noticed
> something odd: #include <server/dynloader.h> fails on Windows when building
> an extension out-of-tree, simply because that file isn't there.

While it may indeed be a packaging bug that that file isn't installed,
the reason why nobody noticed before is that there doesn't seem to be
any good reason for anything except dfmgr.c to include it.  What's the
context?
        regards, tom lane



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

From
Chapman Flack
Date:
On 11/14/15 18:18, Tom Lane wrote:
> While it may indeed be a packaging bug that that file isn't installed,
> the reason why nobody noticed before is that there doesn't seem to be
> any good reason for anything except dfmgr.c to include it.  What's the
> context?

One of the most long-standing sources of frustration for people newly
trying out PL/Java is its dependence on a libjvm.so, which is usually
found several directories deep under whatever location somebody chose
to install the JRE for the platform in question, and it usually isn't
on the platform's normal dynamic loader search path.

Pretty much *everything else* about PL/Java can be configured via its
custom GUCs, which makes for *almost* a one-stop shop for anything that
has to be configured to make the thing work. To get the pljava.so itself
to load successfully, all you need to do is put it in $libdir, or set
dynamic_library_path to include where it lives, all easy things that any
PGer already finds familiar.

But historically, pljava.so has been built to simply contain a dependency
on libjvm.so, just as it would to any of its other library dependencies.
That's great, but all those other library dependencies are usually on the
OS's dyld search path, and libjvm usually isn't. Because that dependency
is baked into the pljava.so, the OS loader attempts to resolve it
*before* we can have any say in where to look; the search pays no attention
to dynamic_library_path or anything else configured in PG, and when it
fails, there is usually a disorienting error message suggesting that
pljava.so itself was what couldn't be loaded (yes, technically, it couldn't,
but that's not the part of the story that helps anyone).

That one inconvenient fact is the entire reason why PL/Java's install
instructions do not stop after "set these GUCs and you're done", but
instead sprawl on for several more paragraphs of platform-specific ick
about LD_LIBRARY_PATH or linking with DT_RPATH or running ldconfig (which
has systemwide effects!) or whatever other hoops you can jump through on
your platform to make this one library findable.

In the proof-of-concept branch I'm working on (which works great in
Linux already, and Ken is testing on Windows), the build process is
changed so that pljava.so does *not* contain a baked-in dependency on
libjvm. That allows PG to successfully load it, no sweat, and in _PG_init
then it goes out and finds the libjvm *using a pljava.libjvm_location GUC*
and now you do have a one-stop shop where there isn't anything you need
to know to install it besides how to set GUCs. If the GUC is set wrong
and it can't load the library, you get a helpful ereport telling you that's
what went wrong, you change the value, using normal PG mechanisms, and then
it succeeds and you're off to the races.

I know I'm saying it myself, but it makes the initial setup experience
a *lot* less irritating. Ken also thinks it'll greatly simplify
installations some of his users have struggled with.

I use the PG dynloader because, hey, you guys have already done the work
of abstracting up from 12 different platforms' variations on dlopen, and
it seems smarter to stand on your shoulders and not reinvent that. The
one minor quirk is that the declaration of pg_dlsym is tailored to return
a PGFunction specifically, which of course is not the type of the one
function JNI_CreateJavaVM that I need to look up in libjvm. But adding
a cast is no trouble. I am not expecting any platform's wrapped dl*
functions to actually fail if the symbol hasn't got that exact type, right?

Regards,
-Chap



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

From
Bruce Momjian
Date:
On Sat, Nov 14, 2015 at 07:11:32PM -0500, Chapman Flack wrote:
> I use the PG dynloader because, hey, you guys have already done the work
> of abstracting up from 12 different platforms' variations on dlopen, and
> it seems smarter to stand on your shoulders and not reinvent that. The
> one minor quirk is that the declaration of pg_dlsym is tailored to return
> a PGFunction specifically, which of course is not the type of the one
> function JNI_CreateJavaVM that I need to look up in libjvm. But adding
> a cast is no trouble. I am not expecting any platform's wrapped dl*
> functions to actually fail if the symbol hasn't got that exact type, right?

OK, good logical reason to install dynloader.h on Windows.  Also, I am
very glad you are working on PL/Java.  :-)

What do we need to do to close this item?  What versions of Windows
installers are missing dynloader.h?  Mingw?  MSVC?  EDB's?  OpenSCG? 
Postgres Pro (Russian)?  

Is this a change we need to make on the server end and then all the
installers will automatically install the file?  It is present in all
Unix-like installs?

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Chapman Flack
Date:
On 12/04/15 12:56, Bruce Momjian wrote:
> 
> OK, good logical reason to install dynloader.h on Windows.

Ah!  Thanks.  I was starting to wonder whether I had said something wrong
and been sent to the bit bucket within my first two -hackers posts. :)

> What do we need to do to close this item?  What versions of Windows
> installers are missing dynloader.h?  Mingw?  MSVC?  EDB's?  OpenSCG?
> Postgres Pro (Russian)?

I am too new around here to be able to supply good answers myself to
those questions.  I keep learning though.  For example, I have just
learned that OpenSCG and Postgres Pro (Russian) are things, and (by
inference) that they run on Windows. :)

In working on PL/Java, as a non-Windows dev myself, I have been
blessed to find Ken Olson willing to build my WIP branches in MSVC
and tell me what I've busted. I think he's building against an EDB
installation of PostgreSQL, so that's the combination I am least
ignorant about. I know that mingw has also been used, but I haven't
yet made a connection with someone who can tell me what I'm breaking
for that build....

> Is this a change we need to make on the server end and then all the
> installers will automatically install the file?  It is present in all
> Unix-like installs?

AFAICS, it must at one time have been envisioned that sometimes
extension code might like a nice dynloader; the whole way that
backend/port/dynloader contains a version for every platform, and
the configure script links the appropriate one into src/include with
all the other .h files that would go into a -devel package, makes me
_think_ that it'd be present in any install that used configure in
the build. Anyone building a -devel package would have to go out
of their way to exclude it but still package the other .h files.

So I'd guess that Windows builds that don't use configure are probably
the odd players out. But I don't have any contacts or name recognition
to approach { EDB, OpenSCG, Postgres Pro } and find out how their
internal build processes work, or whether they all end up lacking
the file, or whether there is any single change that could be made
in the PG source tree so that their builds would then all include it.

>  Also, I am very glad you are working on PL/Java.  :-)

Thanks!  It has been interesting trying to get up to speed, both on
how it technically works, and also on the development history, why it
lives out-of-tree, and so on. (That question had puzzled me for a long
time, and when I finally found the long 2006 -hackers thread,
http://www.postgresql.org/message-id/44B3952B.2080401@tada.se
I was like your genealogy-obsessed aunt finding a trunk in the attic. :)

I can see that a lot of the considerations raised in that thread, both
ways, are still pertinent today, plus with nine more years behind us
to see how things have actually played out. Tom Lane was concerned about
what would happen if Thomas's time for maintenance became scarce, and
what seems to happen is someone like Johann Oskarsson, or yours truly,
will step up, flounder a bit to get over the learning curve, and then
carry the ball some distance. There is a bit of a barrier to entry,
because PostgreSQL and Java are each large and deep and PL/Java has
both, and for the future vigor of the project it seems important to
find the ways to minimize that barrier. (I know I'm getting OT from
dynloader here, but this other stuff has been on my mind a while.)

That doesn't require reopening the in-tree question necessarily
(I saw that Tom Lane was concerned about a buildfarm going all red
because of a problem too few people could easily fix), but it would
probably be very helpful to at least have some kind of _associated
buildfarm_ so the main board might not go all red, but it would still
be easy to see right away if a change was going to affect important
out-of-tree components.

That seems to have been a worthwhile idea nine years ago (I read that
Andrew Dunstan _almost_ found the time to set it up then), and
still today something like that would be very helpful. PL/Java still needs
work on an easily-automatable suite of tests (that much, I'm working on),
and once that's in place, the next obvious and valuable step would be to get
it building continuously on the several major platforms, so it can turn red
on some board that the team can easily see. I might ask for some help or
suggestions on what are the currently most-favored ways to do that.

I think that with more automated testing and CI, the barrier to entry on
PL/Java contributions will be a lot lower, because it is much less
intimidating to start poking at something when it is easy to see what
happens.

But that's all food for future thought ... this thread's about dynloader ...

-Chap



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

From
Bruce Momjian
Date:
On Fri, Dec  4, 2015 at 07:09:03PM -0500, Chapman Flack wrote:
> On 12/04/15 12:56, Bruce Momjian wrote:
> > 
> > OK, good logical reason to install dynloader.h on Windows.
> 
> Ah!  Thanks.  I was starting to wonder whether I had said something wrong
> and been sent to the bit bucket within my first two -hackers posts. :)

No, sometimes people just don't know how to respond, particuarly related
to technology we don't use regularly.

> > What do we need to do to close this item?  What versions of Windows
> > installers are missing dynloader.h?  Mingw?  MSVC?  EDB's?  OpenSCG?
> > Postgres Pro (Russian)?
> 
> I am too new around here to be able to supply good answers myself to
> those questions.  I keep learning though.  For example, I have just
> learned that OpenSCG and Postgres Pro (Russian) are things, and (by
> inference) that they run on Windows. :)

Yes, there are several binary Postgres distributions.  However, in
researching, I think there is a central way to fix them all.

> In working on PL/Java, as a non-Windows dev myself, I have been
> blessed to find Ken Olson willing to build my WIP branches in MSVC
> and tell me what I've busted. I think he's building against an EDB
> installation of PostgreSQL, so that's the combination I am least
> ignorant about. I know that mingw has also been used, but I haven't
> yet made a connection with someone who can tell me what I'm breaking
> for that build....

OK, that sounds good.  I assume he is using MSVC to build PL/Java, and
then using the EDB server binaries, which should work fine.

> > Is this a change we need to make on the server end and then all the
> > installers will automatically install the file?  It is present in all
> > Unix-like installs?
> 
> AFAICS, it must at one time have been envisioned that sometimes
> extension code might like a nice dynloader; the whole way that
> backend/port/dynloader contains a version for every platform, and
> the configure script links the appropriate one into src/include with
> all the other .h files that would go into a -devel package, makes me
> _think_ that it'd be present in any install that used configure in
> the build. Anyone building a -devel package would have to go out
> of their way to exclude it but still package the other .h files.

Yes, it should.  Looking at the 'install' Makefile rule in
include/Makefile I see:
       cp $(srcdir)/*.h '$(DESTDIR)$(includedir_server)'/ || exit; \       chmod $(INSTALL_DATA_MODE)
'$(DESTDIR)$(includedir_server)'/*.h || exit; \       for dir in $(SUBDIRS); do \         cp $(srcdir)/$$dir/*.h
'$(DESTDIR)$(includedir_server)'/$$dir/|| exit; \         chmod $(INSTALL_DATA_MODE)
'$(DESTDIR)$(includedir_server)'/$$dir/*.h || exit; \       done
 

This copies all the *.h files in src/include during install.  If I look
at my Debian source install, I see the dynloader.h installed as
include/server/dynloader.h, which is what you want.  In fact, there are
many include files installed in include/server:
c.h            pg_config_ext.h     pgtime.h     postgres_ext.hdynloader.h    pg_config.h         pg_trace.h
postgres_fe.hfmgr.h        pg_config_manual.h  plperl.h     postgres.hfuncapi.h      pg_config_os.h      plpgsql.h
ppport.hgetaddrinfo.h pg_getopt.h         plpython.h   rusagestub.hgetopt_long.h  pgstat.h            plpy_util.h
windowapi.hmiscadmin.h   pgtar.h             port.h
 

> So I'd guess that Windows builds that don't use configure are probably
> the odd players out. But I don't have any contacts or name recognition
> to approach { EDB, OpenSCG, Postgres Pro } and find out how their
> internal build processes work, or whether they all end up lacking
> the file, or whether there is any single change that could be made
> in the PG source tree so that their builds would then all include it.

In fact, there is a single file that affects all the MSVC-based Windows
installers.  All the MSVC build stuff happens in src/tools/msvc, and it
is mostly written in Perl.  This is the corresponding line in the MSVC Perl 
file Install.pm:
CopySetOfFiles(        '',        [ glob("src\\include\\*.h") ],        $target . '/include/server/');

So, for me, the big question is why dynloader.h isn't getting copied
into /include/server.  (There is a mention the 'Makefile' about vpath
builds needing to copy dynloader.h manually --- is vpath being used for
the Windows installers somehow?)

Can you show me what files you have in /include/server, without your
copying the dynloader.h file manually?  Where did you get that Windows
installer?

> >  Also, I am very glad you are working on PL/Java.  :-)
> 
> Thanks!  It has been interesting trying to get up to speed, both on
> how it technically works, and also on the development history, why it
> lives out-of-tree, and so on. (That question had puzzled me for a long
> time, and when I finally found the long 2006 -hackers thread,
> http://www.postgresql.org/message-id/44B3952B.2080401@tada.se
> I was like your genealogy-obsessed aunt finding a trunk in the attic. :)

Yes, it took us a long time to work out the logic of what should be
external.

> I can see that a lot of the considerations raised in that thread, both
> ways, are still pertinent today, plus with nine more years behind us
> to see how things have actually played out. Tom Lane was concerned about
> what would happen if Thomas's time for maintenance became scarce, and
> what seems to happen is someone like Johann Oskarsson, or yours truly,
> will step up, flounder a bit to get over the learning curve, and then
> carry the ball some distance. There is a bit of a barrier to entry,
> because PostgreSQL and Java are each large and deep and PL/Java has
> both, and for the future vigor of the project it seems important to
> find the ways to minimize that barrier. (I know I'm getting OT from
> dynloader here, but this other stuff has been on my mind a while.)
> 
> That doesn't require reopening the in-tree question necessarily
> (I saw that Tom Lane was concerned about a buildfarm going all red
> because of a problem too few people could easily fix), but it would
> probably be very helpful to at least have some kind of _associated
> buildfarm_ so the main board might not go all red, but it would still
> be easy to see right away if a change was going to affect important
> out-of-tree components.
>
> That seems to have been a worthwhile idea nine years ago (I read that
> Andrew Dunstan _almost_ found the time to set it up then), and
> still today something like that would be very helpful. PL/Java still needs
> work on an easily-automatable suite of tests (that much, I'm working on),
> and once that's in place, the next obvious and valuable step would be to get
> it building continuously on the several major platforms, so it can turn red
> on some board that the team can easily see. I might ask for some help or
> suggestions on what are the currently most-favored ways to do that.

Yes, you can either add a buildfarm member of your own, or get someone
who is already on the buildfarm to build PL/Java as part of their build
process:
http://buildfarm.postgresql.org/

> I think that with more automated testing and CI, the barrier to entry on
> PL/Java contributions will be a lot lower, because it is much less
> intimidating to start poking at something when it is easy to see what
> happens.

Agreed.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
"Olson, Ken"
Date:
I have downloaded a fresh copy of the Win x64 installer (postgresql-9.4.5-2-windows-x64.exe) from
http://www.enterprisedb.com/products-services-training/pgdownload.The output of pg_config and assodicated directory
listingof include/server: 

BINDIR = C:/PROGRA~1/POSTGR~1/9.4/bin
DOCDIR = C:/PROGRA~1/POSTGR~1/9.4/doc
HTMLDIR = C:/PROGRA~1/POSTGR~1/9.4/doc
INCLUDEDIR = C:/PROGRA~1/POSTGR~1/9.4/include
PKGINCLUDEDIR = C:/PROGRA~1/POSTGR~1/9.4/include
INCLUDEDIR-SERVER = C:/PROGRA~1/POSTGR~1/9.4/include/server
LIBDIR = C:/PROGRA~1/POSTGR~1/9.4/lib
PKGLIBDIR = C:/PROGRA~1/POSTGR~1/9.4/lib
LOCALEDIR = C:/PROGRA~1/POSTGR~1/9.4/share/locale
MANDIR = C:/Program Files/PostgreSQL/9.4/man
SHAREDIR = C:/PROGRA~1/POSTGR~1/9.4/share
SYSCONFDIR = C:/Program Files/PostgreSQL/9.4/etc
PGXS = C:/Program Files/PostgreSQL/9.4/lib/pgxs/src/makefiles/pgxs.mk
CONFIGURE = --enable-thread-safety --enable-integer-datetimes --enable-nls --with-ldap --with-ossp-uuid --with-libxml
--with-libxslt--with-tcl --with-perl --with-python 
VERSION = PostgreSQL 9.4.5Volume in drive C has no label.Volume Serial Number is 78D5-D08D
Directory of C:\PROGRA~1\POSTGR~1\9.4\include\server

12/06/2015  01:58 PM    <DIR>          .
12/06/2015  01:58 PM    <DIR>          ..
12/06/2015  01:58 PM    <DIR>          access
12/06/2015  01:58 PM    <DIR>          bootstrap
12/06/2015  01:59 PM    <DIR>          catalog
12/06/2015  01:58 PM    <DIR>          commands
12/06/2015  01:58 PM    <DIR>          common
12/06/2015  01:58 PM    <DIR>          datatype
12/06/2015  01:58 PM    <DIR>          executor
12/06/2015  01:58 PM    <DIR>          foreign
12/06/2015  01:58 PM    <DIR>          lib
12/06/2015  01:58 PM    <DIR>          libpq
12/06/2015  01:58 PM    <DIR>          mb
12/06/2015  01:58 PM    <DIR>          nodes
12/06/2015  01:58 PM    <DIR>          optimizer
12/06/2015  01:58 PM    <DIR>          parser
12/06/2015  01:58 PM    <DIR>          port
12/06/2015  01:58 PM    <DIR>          portability
12/06/2015  01:58 PM    <DIR>          postmaster
12/06/2015  01:58 PM    <DIR>          regex
12/06/2015  01:58 PM    <DIR>          replication
12/06/2015  01:58 PM    <DIR>          rewrite
12/06/2015  01:58 PM    <DIR>          snowball
12/06/2015  01:58 PM    <DIR>          storage
12/06/2015  01:58 PM    <DIR>          tcop
12/06/2015  01:58 PM    <DIR>          tsearch
12/06/2015  01:58 PM    <DIR>          utils
11/19/2015  12:19 AM            30,882 c.h
11/19/2015  12:19 AM            30,626 fmgr.h
11/19/2015  12:19 AM            10,711 funcapi.h
11/19/2015  12:19 AM             3,986 getaddrinfo.h
11/19/2015  12:19 AM               660 getopt_long.h
11/19/2015  12:19 AM            15,482 miscadmin.h
11/19/2015  12:45 AM            21,702 pg_config.h
11/19/2015  12:45 AM               253 pg_config_ext.h
11/19/2015  12:19 AM            10,875 pg_config_manual.h
11/19/2015  12:45 AM            13,392 pg_config_os.h
11/19/2015  12:19 AM             1,084 pg_getopt.h
11/19/2015  12:19 AM               316 pg_trace.h
11/19/2015  12:19 AM            27,166 pgstat.h
11/19/2015  12:19 AM               606 pgtar.h
11/19/2015  12:19 AM             2,309 pgtime.h
11/19/2015  12:19 AM            27,489 plpgsql.h
11/19/2015  12:19 AM            14,096 port.h
11/19/2015  12:19 AM            21,398 postgres.h
11/19/2015  12:19 AM             2,109 postgres_ext.h
11/19/2015  12:19 AM               763 postgres_fe.h
11/19/2015  12:19 AM               843 rusagestub.h
11/19/2015  12:19 AM             2,379 windowapi.h             22 File(s)        239,127 bytes             27 Dir(s)
26,142,257,152bytes free 

Ken

-----Original Message-----
From: Chapman Flack [mailto:chap@anastigmatix.net]
Sent: Saturday, December 05, 2015 4:07 PM
To: Olson, Ken
Subject: Re: [HACKERS] dynloader.h missing in prebuilt package for Windows?

Ken,

Do you have a moment to respond to Bruce's questions here, about what files
*are* put into $INCLUDEDIR_SERVER by the Windows PG installer you've used, and just what installer that is (supplied by
EDB,right?)? 

Thanks,
-Chap


On 12/05/15 15:35, Bruce Momjian wrote:
> So, for me, the big question is why dynloader.h isn't getting copied
> into /include/server.  (There is a mention the 'Makefile' about vpath
> builds needing to copy dynloader.h manually --- is vpath being used
> for the Windows installers somehow?)
>
> Can you show me what files you have in /include/server, without your
> copying the dynloader.h file manually?  Where did you get that Windows
> installer?



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

From
Bruce Momjian
Date:
On Sun, Dec  6, 2015 at 07:16:24PM +0000, Olson, Ken wrote:
> I have downloaded a fresh copy of the Win x64 installer (postgresql-9.4.5-2-windows-x64.exe) from
http://www.enterprisedb.com/products-services-training/pgdownload.The output of pg_config and assodicated directory
listingof include/server: 
...
>  Directory of C:\PROGRA~1\POSTGR~1\9.4\include\server
>
> 11/19/2015  12:19 AM            30,882 c.h
> 11/19/2015  12:19 AM            30,626 fmgr.h
> 11/19/2015  12:19 AM            10,711 funcapi.h
> 11/19/2015  12:19 AM             3,986 getaddrinfo.h
> 11/19/2015  12:19 AM               660 getopt_long.h
> 11/19/2015  12:19 AM            15,482 miscadmin.h
> 11/19/2015  12:45 AM            21,702 pg_config.h
> 11/19/2015  12:45 AM               253 pg_config_ext.h
> 11/19/2015  12:19 AM            10,875 pg_config_manual.h
> 11/19/2015  12:45 AM            13,392 pg_config_os.h
> 11/19/2015  12:19 AM             1,084 pg_getopt.h
> 11/19/2015  12:19 AM               316 pg_trace.h
> 11/19/2015  12:19 AM            27,166 pgstat.h
> 11/19/2015  12:19 AM               606 pgtar.h
> 11/19/2015  12:19 AM             2,309 pgtime.h
> 11/19/2015  12:19 AM            27,489 plpgsql.h
> 11/19/2015  12:19 AM            14,096 port.h
> 11/19/2015  12:19 AM            21,398 postgres.h
> 11/19/2015  12:19 AM             2,109 postgres_ext.h
> 11/19/2015  12:19 AM               763 postgres_fe.h
> 11/19/2015  12:19 AM               843 rusagestub.h
> 11/19/2015  12:19 AM             2,379 windowapi.h

Sorry, I am just getting to this.  This list helps.  Looking at the
'configure' run on Debian, I see at the bottom:

  config.status: linking src/backend/port/tas/dummy.s to src/backend/port/tas.s
  config.status: linking src/backend/port/dynloader/linux.c to src/backend/port/dynloader.c
  config.status: linking src/backend/port/sysv_sema.c to src/backend/port/pg_sema.c
  config.status: linking src/backend/port/sysv_shmem.c to src/backend/port/pg_shmem.c
  config.status: linking src/backend/port/unix_latch.c to src/backend/port/pg_latch.c
1 config.status: linking src/backend/port/dynloader/linux.h to src/include/dynloader.h
2 config.status: linking src/include/port/linux.h to src/include/pg_config_os.h
  config.status: linking src/makefiles/Makefile.linux to src/Makefile.port

Line #2 copies pg_config_os.h to src/include, and later all
src/include/*.h files are copied to the install directory by
src/include/Makefile.  The same happens for dynloader.h on line #1.

In the Windows MSVC build, we handle pg_config_os.h in the Perl scripts,
but not dynloader.h.  The attached patch copies the process used for
pg_config_os.h to handle dynloader.h.  I believe this is the only *.h
file that has this problem.

This should fix the PL/Java Windows build problem.  I don't think I will
get this patch into 9.5.0 but will put it in after 9.5.0 is released and
it will appear in all the next minor version releases, including 9.5.1,
which should happen in the next 60 days.

Thanks.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +

Attachment

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

From
Chapman Flack
Date:
On 12/29/15 14:08, Bruce Momjian wrote:

> This should fix the PL/Java Windows build problem.  I don't think I will
> get this patch into 9.5.0 but will put it in after 9.5.0 is released and
> it will appear in all the next minor version releases, including 9.5.1,
> which should happen in the next 60 days.

Thanks!  What I ended up doing in PL/Java was just to create a 'fallback'
directory with a copy of dynloader.h in it, and adding it at the very
end of the include path when building on Windows, so I think the build
will now work either way, using the real file if it is present, or the
frozen-in-time fallback copy if it isn't.  Then the fallback directory
can be axed as soon as 9.5.1 becomes the oldest thing in PL/Java's
rearview support mirror.

-Chap



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

From
Bruce Momjian
Date:
On Tue, Dec 29, 2015 at 03:01:55PM -0500, Chapman Flack wrote:
> On 12/29/15 14:08, Bruce Momjian wrote:
> 
> > This should fix the PL/Java Windows build problem.  I don't think I will
> > get this patch into 9.5.0 but will put it in after 9.5.0 is released and
> > it will appear in all the next minor version releases, including 9.5.1,
> > which should happen in the next 60 days.
> 
> Thanks!  What I ended up doing in PL/Java was just to create a 'fallback'
> directory with a copy of dynloader.h in it, and adding it at the very
> end of the include path when building on Windows, so I think the build
> will now work either way, using the real file if it is present, or the
> frozen-in-time fallback copy if it isn't.  Then the fallback directory
> can be axed as soon as 9.5.1 becomes the oldest thing in PL/Java's
> rearview support mirror.

Yes, that is a good plan.  I will apply this to all supported major
versions, so a simple minor version upgrade will allow it to work
without the fallback.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Bruce Momjian
Date:
On Tue, Dec 29, 2015 at 02:08:19PM -0500, Bruce Momjian wrote:
> In the Windows MSVC build, we handle pg_config_os.h in the Perl scripts,
> but not dynloader.h.  The attached patch copies the process used for
> pg_config_os.h to handle dynloader.h.  I believe this is the only *.h
> file that has this problem.
> 
> This should fix the PL/Java Windows build problem.  I don't think I will
> get this patch into 9.5.0 but will put it in after 9.5.0 is released and
> it will appear in all the next minor version releases, including 9.5.1,
> which should happen in the next 60 days.

Oops.  Once this patch is applied, it is only going to take effect when
someone _installs_ Postgres.  Even an initdb will not add the file. 
This means that upgrading to the next minor release will _not_ fix this.

This suggests that we perhaps should consider this for 9.5.0, and that
your hack will have to be active until 9.4 gets to end-of-life, or
perhaps 9.5 if we can't get this into 9.5.0.  People who are using 9.5
Beta or RC will still not have the file, meaning 9.5 end-of-life might
still be a requirement.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Chapman Flack
Date:
On 12/30/15 20:40, Bruce Momjian wrote:

> your hack will have to be active until 9.4 gets to end-of-life, or
> perhaps 9.5 if we can't get this into 9.5.0.  People who are using 9.5
> Beta or RC will still not have the file, meaning 9.5 end-of-life might
> still be a requirement.

... by which time PL/Java will be sporting the javax.ai.selfaware
features of Java SE 11 or 12, and no one will even have to tell it to
drop the fallback directory.

Well, so it's one directory and one file and a few extra lines in pom.xml.
There are already more onerous backward compatibility hacks in PL/Java
than that. :)

-Chap



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

From
Alvaro Herrera
Date:
Bruce Momjian wrote:
> On Tue, Dec 29, 2015 at 02:08:19PM -0500, Bruce Momjian wrote:
> > In the Windows MSVC build, we handle pg_config_os.h in the Perl scripts,
> > but not dynloader.h.  The attached patch copies the process used for
> > pg_config_os.h to handle dynloader.h.  I believe this is the only *.h
> > file that has this problem.
> > 
> > This should fix the PL/Java Windows build problem.  I don't think I will
> > get this patch into 9.5.0 but will put it in after 9.5.0 is released and
> > it will appear in all the next minor version releases, including 9.5.1,
> > which should happen in the next 60 days.
> 
> Oops.  Once this patch is applied, it is only going to take effect when
> someone _installs_ Postgres.  Even an initdb will not add the file. 
> This means that upgrading to the next minor release will _not_ fix this.

They will, unless they build from source -- which most people don't.

> This suggests that we perhaps should consider this for 9.5.0, and that
> your hack will have to be active until 9.4 gets to end-of-life, or
> perhaps 9.5 if we can't get this into 9.5.0.  People who are using 9.5
> Beta or RC will still not have the file, meaning 9.5 end-of-life might
> still be a requirement.

I think this should be fixed in 9.5.0 since you already have the patch --
what's the point of waiting for 9.5.1?

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



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

From
Bruce Momjian
Date:
On Wed, Dec 30, 2015 at 11:18:45PM -0300, Alvaro Herrera wrote:
> > This suggests that we perhaps should consider this for 9.5.0, and that
> > your hack will have to be active until 9.4 gets to end-of-life, or
> > perhaps 9.5 if we can't get this into 9.5.0.  People who are using 9.5
> > Beta or RC will still not have the file, meaning 9.5 end-of-life might
> > still be a requirement.
> 
> I think this should be fixed in 9.5.0 since you already have the patch --
> what's the point of waiting for 9.5.1?

True.  We are in RC mode now though, and this is a Windows build issue
which I cannot test.  I would need someone else to apply it after
testing.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Oops.  Once this patch is applied, it is only going to take effect when
> someone _installs_ Postgres.  Even an initdb will not add the file. 
> This means that upgrading to the next minor release will _not_ fix this.

Uh, what?  Surely an upgrade to a new package *would* fix it, because
that is a reinstall at the filesystem level.  This patch has nothing
to do with system catalog contents, which is what initdb would be
concerned with.

I would not be terribly worried about you pushing it into 9.5.0, but
I also think that it could wait for 9.5.1.
        regards, tom lane



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

From
Bruce Momjian
Date:
On Wed, Dec 30, 2015 at 11:57:45PM -0500, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Oops.  Once this patch is applied, it is only going to take effect when
> > someone _installs_ Postgres.  Even an initdb will not add the file. 
> > This means that upgrading to the next minor release will _not_ fix this.
> 
> Uh, what?  Surely an upgrade to a new package *would* fix it, because
> that is a reinstall at the filesystem level.  This patch has nothing
> to do with system catalog contents, which is what initdb would be
> concerned with.

Uh, do we install hew lib files and stuff in a minor upgrade?  I guess
we do, yeah.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Bruce Momjian
Date:
On Thu, Dec 31, 2015 at 12:50:13AM -0500, Bruce Momjian wrote:
> On Wed, Dec 30, 2015 at 11:57:45PM -0500, Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > Oops.  Once this patch is applied, it is only going to take effect when
> > > someone _installs_ Postgres.  Even an initdb will not add the file. 
> > > This means that upgrading to the next minor release will _not_ fix this.
> > 
> > Uh, what?  Surely an upgrade to a new package *would* fix it, because
> > that is a reinstall at the filesystem level.  This patch has nothing
> > to do with system catalog contents, which is what initdb would be
> > concerned with.
> 
> Uh, do we install hew lib files and stuff in a minor upgrade?  I guess
> we do, yeah.

Let's hold this for 9.5.1 and all minor releases will get it at the same
time.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



While on the subject of things that could make it or not into 9.5.?,
I see that 9.5.0 already adds PGDLLIMPORT on the global variable
creating_extension, but CurrentExtensionObject on the very next
line of extension.h still doesn't have it.

The simplest way I've come up with in Windows to identify the extension
being created is to create some temporary object, call
getExtensionOfObject() on it, then drop it. A bit circuitous when on
any other platform I can just look at CurrentExtensionObject....

-Chap



Chapman Flack <chap@anastigmatix.net> writes:
> I see that 9.5.0 already adds PGDLLIMPORT on the global variable
> creating_extension, but CurrentExtensionObject on the very next
> line of extension.h still doesn't have it.

Why would you need to access that?
        regards, tom lane



On 12/31/15 16:13, Tom Lane wrote:
>> I see that 9.5.0 already adds PGDLLIMPORT on the global variable
>> creating_extension, but CurrentExtensionObject on the very next
>> line of extension.h still doesn't have it.
> 
> Why would you need to access that?

This returns to the earlier question about extensions whose purpose
is to enable other extensions. I'm thinking a bit ahead. At the moment,
I am only working on nudging PL/Java itself into the extension framework,
so you can install *it* with CREATE EXTENSION. For that, I can get along
without the extension Oid.

Down the road, it would be quite natural for PL/Java users to develop
functionality in Java, package it in a jar file, and want to install that
using CREATE EXTENSION. So they'd distribute their foo.jar file with a
foo.control file (requires = 'pljava'), and a very short foo--1.0.0.sql file SELECT sqlj.install_jar('file:foo.jar',
'foo',true);
 
and most of the right stuff will automagically happen ... the
associated system objects (created by the deployment script inside
the jar, executed by install_jar) will be captured as extension
members. But the jar itself, stashed by install_jar into a PL/Java
managed table that can't participate in pg_depend, somehow still
needs to be associated with the extension too.

I suppose there really won't be a way to do this with reliability
unless someday extensions can hook the dependency infrastructure,
as you mentioned in passing in an earlier message.

That sounds like a longer discussion. But I wondered if it might
not be too hard to put PGDLLIMPORT on CurrentExtensionObject as
a stopgap.

... though perhaps it doesn't matter that much, because I still
have to write a circuitous workaround anyway, and keep it in the
code until 9.1 through 9.4 all vanish from the earth.

-Chap



On Thu, Dec 31, 2015 at 04:41:44PM -0500, Chapman Flack wrote:
> I suppose there really won't be a way to do this with reliability
> unless someday extensions can hook the dependency infrastructure,
> as you mentioned in passing in an earlier message.
> 
> That sounds like a longer discussion. But I wondered if it might
> not be too hard to put PGDLLIMPORT on CurrentExtensionObject as
> a stopgap.
> 
> ... though perhaps it doesn't matter that much, because I still
> have to write a circuitous workaround anyway, and keep it in the
> code until 9.1 through 9.4 all vanish from the earth.

I think we decided that only older _minor_ versions would need your
workaround, so anyone doing a minor upgrade after 9.5.1 and friends
would be fine.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Robert Haas
Date:
On Dec 31, 2015, at 1:04 PM, Bruce Momjian <bruce@momjian.us> wrote:
>> On Thu, Dec 31, 2015 at 12:50:13AM -0500, Bruce Momjian wrote:
>>> On Wed, Dec 30, 2015 at 11:57:45PM -0500, Tom Lane wrote:
>>> Bruce Momjian <bruce@momjian.us> writes:
>>>> Oops.  Once this patch is applied, it is only going to take effect when
>>>> someone _installs_ Postgres.  Even an initdb will not add the file.
>>>> This means that upgrading to the next minor release will _not_ fix this.
>>>
>>> Uh, what?  Surely an upgrade to a new package *would* fix it, because
>>> that is a reinstall at the filesystem level.  This patch has nothing
>>> to do with system catalog contents, which is what initdb would be
>>> concerned with.
>>
>> Uh, do we install hew lib files and stuff in a minor upgrade?  I guess
>> we do, yeah.
>
> Let's hold this for 9.5.1 and all minor releases will get it at the same
> time.

I vote for going ahead with this at once. It seems low risk, and having 9.5.1 install different files from 9.5.0 seems
likean unnecessary annoyance. 

...Robert


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

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> On Dec 31, 2015, at 1:04 PM, Bruce Momjian <bruce@momjian.us> wrote:
>> Let's hold this for 9.5.1 and all minor releases will get it at the same
>> time.

> I vote for going ahead with this at once. It seems low risk, and having 9.5.1 install different files from 9.5.0
seemslike an unnecessary annoyance.
 

If we're willing to allow 9.4.6 to install different files than 9.4.5
does, I don't see why it's a problem for 9.5.1.  But having said that,
I agree that this seems pretty low-risk, and so IMO we might as well
ship it sooner not later.
        regards, tom lane



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

From
Bruce Momjian
Date:
On Sun, Jan  3, 2016 at 12:35:02PM -0500, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Dec 31, 2015, at 1:04 PM, Bruce Momjian <bruce@momjian.us> wrote:
> >> Let's hold this for 9.5.1 and all minor releases will get it at the same
> >> time.
> 
> > I vote for going ahead with this at once. It seems low risk, and having 9.5.1 install different files from 9.5.0
seemslike an unnecessary annoyance.
 
> 
> If we're willing to allow 9.4.6 to install different files than 9.4.5
> does, I don't see why it's a problem for 9.5.1.  But having said that,
> I agree that this seems pretty low-risk, and so IMO we might as well
> ship it sooner not later.

Well, as I said, I can't test the patch, which made me lean toward
9.5.1.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> On Sun, Jan  3, 2016 at 12:35:02PM -0500, Tom Lane wrote:
>> If we're willing to allow 9.4.6 to install different files than 9.4.5
>> does, I don't see why it's a problem for 9.5.1.  But having said that,
>> I agree that this seems pretty low-risk, and so IMO we might as well
>> ship it sooner not later.

> Well, as I said, I can't test the patch, which made me lean toward
> 9.5.1.

That's what the buildfarm is for ... but ...

I would have been fine with you pushing this yesterday, but now it's
too late to get a buildfarm cycle on it.  Please hold for 9.5.1.
        regards, tom lane



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

From
Bruce Momjian
Date:
On Mon, Jan  4, 2016 at 12:59:26PM -0500, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Sun, Jan  3, 2016 at 12:35:02PM -0500, Tom Lane wrote:
> >> If we're willing to allow 9.4.6 to install different files than 9.4.5
> >> does, I don't see why it's a problem for 9.5.1.  But having said that,
> >> I agree that this seems pretty low-risk, and so IMO we might as well
> >> ship it sooner not later.
> 
> > Well, as I said, I can't test the patch, which made me lean toward
> > 9.5.1.
> 
> That's what the buildfarm is for ... but ...
> 
> I would have been fine with you pushing this yesterday, but now it's
> too late to get a buildfarm cycle on it.  Please hold for 9.5.1.

Oh, I forgot about the buildfarm testing.  Good point.  OK, hold for
9.5.1.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Michael Paquier
Date:
On Mon, Jan 4, 2016 at 10:06 AM, Bruce Momjian <bruce@momjian.us> wrote:
> On Mon, Jan  4, 2016 at 12:59:26PM -0500, Tom Lane wrote:
>> Bruce Momjian <bruce@momjian.us> writes:
>> > On Sun, Jan  3, 2016 at 12:35:02PM -0500, Tom Lane wrote:
>> >> If we're willing to allow 9.4.6 to install different files than 9.4.5
>> >> does, I don't see why it's a problem for 9.5.1.  But having said that,
>> >> I agree that this seems pretty low-risk, and so IMO we might as well
>> >> ship it sooner not later.
>>
>> > Well, as I said, I can't test the patch, which made me lean toward
>> > 9.5.1.
>>
>> That's what the buildfarm is for ... but ...
>>
>> I would have been fine with you pushing this yesterday, but now it's
>> too late to get a buildfarm cycle on it.  Please hold for 9.5.1.
>
> Oh, I forgot about the buildfarm testing.  Good point.  OK, hold for
> 9.5.1.

The patch would put the buildfarm in red as it is incomplete anyway,
with MSVC what is used instead of dynloader.h is
port/dynloader/win32.h. 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.
-- 
Michael



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

From
Tom Lane
Date:
Michael Paquier <michael.paquier@gmail.com> writes:
> The patch would put the buildfarm in red as it is incomplete anyway,
> with MSVC what is used instead of dynloader.h is
> port/dynloader/win32.h. 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.

Patch please?
        regards, tom lane



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

From
Chapman Flack
Date:
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.

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.

> 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?

-Chap



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

From
Michael Paquier
Date:
On Tue, Jan 5, 2016 at 2:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> The patch would put the buildfarm in red as it is incomplete anyway,
>> with MSVC what is used instead of dynloader.h is
>> port/dynloader/win32.h. 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.
>
> Patch please?

Sure, here you go. Bruce's patch simply forgot to copy the header file
via Solution.pm, so installation just failed. The change of dfmgr.c is
actually not mandatory but I think that as long as dynloader.h is
copied in include/ we had better change that as well, and it makes the
code cleaner.
--
Michael

Attachment

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

From
Michael Paquier
Date:
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



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

From
Chapman Flack
Date:
On 01/05/2016 12:53 AM, Michael Paquier wrote:

> 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.

Looking at what you changed, all becomes clear.  :)

-Chap




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

From
Chapman Flack
Date:
On 01/05/2016 09:18 AM, Chapman Flack wrote:
> On 01/05/2016 12:53 AM, Michael Paquier wrote:
> 
>> 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.
> 
> Looking at what you changed, all becomes clear.  :)

Out of curiosity, what happens (or what is supposed to happen)
to port/dynloader/<os-specific>.c ?

-Chap




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

From
Michael Paquier
Date:
On Tue, Jan 5, 2016 at 11:24 PM, Chapman Flack <chap@anastigmatix.net> wrote:
> On 01/05/2016 09:18 AM, Chapman Flack wrote:
>> On 01/05/2016 12:53 AM, Michael Paquier wrote:
>>
>>> 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.
>>
>> Looking at what you changed, all becomes clear.  :)
>
> Out of curiosity, what happens (or what is supposed to happen)
> to port/dynloader/<os-specific>.c ?

For other platforms that go through ./configure, this file is
similarly copied as src/include/dynloader.h. MSVC is a special case..
-- 
Michael



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

From
Michael Paquier
Date:
On Tue, Jan 5, 2016 at 2:50 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Jan 5, 2016 at 2:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Michael Paquier <michael.paquier@gmail.com> writes:
>>> The patch would put the buildfarm in red as it is incomplete anyway,
>>> with MSVC what is used instead of dynloader.h is
>>> port/dynloader/win32.h. 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.
>>
>> Patch please?
>
> Sure, here you go. Bruce's patch simply forgot to copy the header file
> via Solution.pm, so installation just failed. The change of dfmgr.c is
> actually not mandatory but I think that as long as dynloader.h is
> copied in include/ we had better change that as well, and it makes the
> code cleaner.

By the way, it is definitely wiser to wait for after the release of
9.5.0 to push that or something similar...
-- 
Michael



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

From
Michael Paquier
Date:
On Wed, Jan 6, 2016 at 7:47 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> By the way, it is definitely wiser to wait for after the release of
> 9.5.0 to push that or something similar...

And I have added an entry in the CF app, let's not lose track of this item:
https://commitfest.postgresql.org/9/473/
-- 
Michael



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

From
Bruce Momjian
Date:
On Mon, Jan  4, 2016 at 09:50:40PM -0800, Michael Paquier wrote:
> On Tue, Jan 5, 2016 at 2:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Michael Paquier <michael.paquier@gmail.com> writes:
> >> The patch would put the buildfarm in red as it is incomplete anyway,
> >> with MSVC what is used instead of dynloader.h is
> >> port/dynloader/win32.h. 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.
> >
> > Patch please?
> 
> Sure, here you go. Bruce's patch simply forgot to copy the header file
> via Solution.pm, so installation just failed. The change of dfmgr.c is
> actually not mandatory but I think that as long as dynloader.h is
> copied in include/ we had better change that as well, and it makes the
> code cleaner.

I have applied this patch all the way back to 9.1.  This means PL/Java
can be cleanly built via MSVC on Windows for all installs after the next
set of minor releases.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +



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

From
Michael Paquier
Date:
On Wed, Jan 20, 2016 at 1:34 PM, Bruce Momjian <bruce@momjian.us> wrote:
> On Mon, Jan  4, 2016 at 09:50:40PM -0800, Michael Paquier wrote:
>> On Tue, Jan 5, 2016 at 2:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> > Michael Paquier <michael.paquier@gmail.com> writes:
>> >> The patch would put the buildfarm in red as it is incomplete anyway,
>> >> with MSVC what is used instead of dynloader.h is
>> >> port/dynloader/win32.h. 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.
>> >
>> > Patch please?
>>
>> Sure, here you go. Bruce's patch simply forgot to copy the header file
>> via Solution.pm, so installation just failed. The change of dfmgr.c is
>> actually not mandatory but I think that as long as dynloader.h is
>> copied in include/ we had better change that as well, and it makes the
>> code cleaner.
>
> I have applied this patch all the way back to 9.1.  This means PL/Java
> can be cleanly built via MSVC on Windows for all installs after the next
> set of minor releases.

Thanks. I'll keep an eye on the buildfarm in case.
-- 
Michael



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

From
Michael Paquier
Date:
On Wed, Jan 20, 2016 at 1:37 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Jan 20, 2016 at 1:34 PM, Bruce Momjian <bruce@momjian.us> wrote:
>> On Mon, Jan  4, 2016 at 09:50:40PM -0800, Michael Paquier wrote:
>>> On Tue, Jan 5, 2016 at 2:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> > Michael Paquier <michael.paquier@gmail.com> writes:
>>> >> The patch would put the buildfarm in red as it is incomplete anyway,
>>> >> with MSVC what is used instead of dynloader.h is
>>> >> port/dynloader/win32.h. 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.
>>> >
>>> > Patch please?
>>>
>>> Sure, here you go. Bruce's patch simply forgot to copy the header file
>>> via Solution.pm, so installation just failed. The change of dfmgr.c is
>>> actually not mandatory but I think that as long as dynloader.h is
>>> copied in include/ we had better change that as well, and it makes the
>>> code cleaner.
>>
>> I have applied this patch all the way back to 9.1.  This means PL/Java
>> can be cleanly built via MSVC on Windows for all installs after the next
>> set of minor releases.
>
> Thanks. I'll keep an eye on the buildfarm in case.

And it did not blow up.
-- 
Michael