Обсуждение: dynloader.h missing in prebuilt package for Windows?
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
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
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
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 +
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
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 +
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?
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 +
Вложения
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
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 +
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 +
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
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
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 +
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
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 +
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 +
CurrentExtensionObject was Re: dynloader.h missing in prebuilt package for Windows?
От
Chapman Flack
Дата:
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
Re: CurrentExtensionObject was Re: dynloader.h missing in prebuilt package for Windows?
От
Chapman Flack
Дата:
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
Re: CurrentExtensionObject was Re: dynloader.h missing in prebuilt package for Windows?
От
Bruce Momjian
Дата:
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 +
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
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
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 +
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
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 +
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
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
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
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
Вложения
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
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
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
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
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
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
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 +
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
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