Re: install libpq.dll in bin directory on Windows / Cygwin

Jeff Janes asked me about this, and Bruce just tripped up on it. Usually
on Windows it's necessary to have libpq.dll/cygpq.dll either in the PATH
or in the same directory as client .exe files. The buildfarm client has
for many years simply copied this dll from the installation lib to the
installation bin directory after running "make install". But I can't
really see why we don't do that as part of "make install" anyway. I
haven't tested but I think something like this patch would achieve this
goal - it would fix something that's tripped a lot of people up over the
years.

Comments? If we do this, should it be backported?

cheers

andrew

Andrew Dunstan wrote:
> Jeff Janes asked me about this, and Bruce just tripped up on it.
> Usually on Windows it's necessary to have libpq.dll/cygpq.dll either
> in the PATH or in the same directory as client .exe files. The
> buildfarm client has for many years simply copied this dll from the
> installation lib to the installation bin directory after running
> "make install". But I can't really see why we don't do that as part
> of "make install" anyway. I haven't tested but I think something
> like this patch would achieve this goal - it would fix something
> that's tripped a lot of people up over the years.

Seems a reasonable workaround for a silly platform bug. Do you need to
patch the MSVC stuff as well?

> Comments? If we do this, should it be backported?

To 9.3, sure, but not further back, as there's probably little point.

> +ifneq (,$findstring($(PORTNAME), win32 cygwin))
> + $(INSTALL_DATA) $(shlib) '$(DESTDIR)$(bindir)/$(shlib)'
> +endif

I wonder if someday we will create a win64 $(PORTNAME) value, or
something like that, and then we'll have to update the port list on
which these hacks are applied all over the place. Not complaining
about this patch in particular, just an idle thought.

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

On 07/25/2013 05:02 PM, Alvaro Herrera wrote:
> Andrew Dunstan wrote:
>> Jeff Janes asked me about this, and Bruce just tripped up on it.
>> Usually on Windows it's necessary to have libpq.dll/cygpq.dll either
>> in the PATH or in the same directory as client .exe files. The
>> buildfarm client has for many years simply copied this dll from the
>> installation lib to the installation bin directory after running
>> "make install". But I can't really see why we don't do that as part
>> of "make install" anyway. I haven't tested but I think something
>> like this patch would achieve this goal - it would fix something
>> that's tripped a lot of people up over the years.
> Seems a reasonable workaround for a silly platform bug. Do you need to
> patch the MSVC stuff as well?

MSVC already does it - see src/tools/msvc/Install.pm:

lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');

cheers

andrew

Andrew Dunstan wrote:
>
> On 07/25/2013 05:02 PM, Alvaro Herrera wrote:
> >Andrew Dunstan wrote:

> >>Usually on Windows it's necessary to have libpq.dll/cygpq.dll either
> >>in the PATH or in the same directory as client .exe files.
> >
> >Seems a reasonable workaround for a silly platform bug. Do you need to
> >patch the MSVC stuff as well?
>
> MSVC already does it - see src/tools/msvc/Install.pm:
>
> lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');

Oh, so your patch is just a bug fix and we should backpatch it all the
way, no?

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

From: "Andrew Dunstan" <andrew(dot)dunstan(at)pgexperts(dot)com>
> on Windows it's necessary to have libpq.dll/cygpq.dll either in the PATH
> or in the same directory as client .exe files. The buildfarm client has
> for many years simply copied this dll from the installation lib to the
> installation bin directory after running "make install". But I can't
> really see why we don't do that as part of "make install" anyway. I
> haven't tested but I think something like this patch would achieve this
> goal - it would fix something that's tripped a lot of people up over the
> years.
>
> Comments? If we do this, should it be backported?

I was just about to propose something related to this.

On native Windows (not on Cygwin or MinGW), DLLs are in general placed in
(1) system directory (e.g. c:\windows\system32), (2) somewhere in PATH, or
(3) the same directory as EXEs which automatically load the DLL at
invocation. It's no problem that most DLLs in PostgreSQL's lib directory,
because they are loaded at runtime by postgres.exe by calling LoadLibrary()
specifying an absolute or a relative path. However, the below files are
misplaced:

libecpg.dll
libecpg_compat.dll
libpgtypes.dll

These should be placed in bin directory. If done so, when running SQL
embedded C applications, users can just add bin in PATH, which is usually
done already. Otherwise, users have to add both bin and lib in PATH.
Usually, lib is not added in PATH in many software.

Could you please place the above files in bin and remove them from lib?

BTW, why is libpq.dll in lib necessary? For the above files? If so, we can
remove libpq.dll from lib. Or, libpqwalreceiver.dll needs it?

Regards
MauMau

Il 7/25/2013 11:02 PM, Alvaro Herrera ha scritto:
> Andrew Dunstan wrote:
>> Jeff Janes asked me about this, and Bruce just tripped up on it.
>> Usually on Windows it's necessary to have libpq.dll/cygpq.dll either
>> in the PATH or in the same directory as client .exe files. The
>> buildfarm client has for many years simply copied this dll from the
>> installation lib to the installation bin directory after running
>> "make install". But I can't really see why we don't do that as part
>> of "make install" anyway. I haven't tested but I think something
>> like this patch would achieve this goal - it would fix something
>> that's tripped a lot of people up over the years.
>
> Seems a reasonable workaround for a silly platform bug. Do you need to
> patch the MSVC stuff as well?
>
>> Comments? If we do this, should it be backported?
>
> To 9.3, sure, but not further back, as there's probably little point.

on cygwin no need to go before 9.3 .
We already moved them during the package build as workaround.

>
>> +ifneq (,$findstring($(PORTNAME), win32 cygwin))
>> + $(INSTALL_DATA) $(shlib) '$(DESTDIR)$(bindir)/$(shlib)'
>> +endif
>
> I wonder if someday we will create a win64 $(PORTNAME) value, or
> something like that, and then we'll have to update the port list on
> which these hacks are applied all over the place. Not complaining
> about this patch in particular, just an idle thought.
>

Andrew,
are you planning to include also

http://www.postgresql.org/message-id/E1UqoYt-0007Qc-9E@wrigleys.postgresql.org
http://www.postgresql.org/message-id/51B59794.3000500@gmail.com

to get rid of dllwrap ?

Regards
Marco

On 07/25/2013 06:27 PM, MauMau wrote:
> From: "Andrew Dunstan" <andrew(dot)dunstan(at)pgexperts(dot)com>
>> on Windows it's necessary to have libpq.dll/cygpq.dll either in the PATH
>> or in the same directory as client .exe files. The buildfarm client has
>> for many years simply copied this dll from the installation lib to the
>> installation bin directory after running "make install". But I can't
>> really see why we don't do that as part of "make install" anyway. I
>> haven't tested but I think something like this patch would achieve this
>> goal - it would fix something that's tripped a lot of people up over the
>> years.
>>
>> Comments? If we do this, should it be backported?
>
> I was just about to propose something related to this.
>
> On native Windows (not on Cygwin or MinGW), DLLs are in general placed
> in (1) system directory (e.g. c:\windows\system32), (2) somewhere in
> PATH, or (3) the same directory as EXEs which automatically load the
> DLL at invocation. It's no problem that most DLLs in PostgreSQL's lib
> directory, because they are loaded at runtime by postgres.exe by
> calling LoadLibrary() specifying an absolute or a relative path.
> However, the below files are misplaced:
>
> libecpg.dll
> libecpg_compat.dll
> libpgtypes.dll
>
> These should be placed in bin directory. If done so, when running SQL
> embedded C applications, users can just add bin in PATH, which is
> usually done already. Otherwise, users have to add both bin and lib
> in PATH. Usually, lib is not added in PATH in many software.
>
> Could you please place the above files in bin and remove them from lib?

I don't have a problem adding them to the bin directory. I'd be very
slightly wary of removing them from the lib directory, for legacy
reasons. Maybe these changes should be done for git tip and not
backported (or maybe just to 9.3). Or we could just decide to clean this
mess in one fell swoop.

>
> BTW, why is libpq.dll in lib necessary? For the above files? If so,
> we can remove libpq.dll from lib. Or, libpqwalreceiver.dll needs it?

Not sure. Perhaps you could experiment and see if anything bad happens
if libpq is just installed in the bin directory and not in lib.

cheers

andrew

On 2013-07-28 15:37:49 -0400, Andrew Dunstan wrote:
> >BTW, why is libpq.dll in lib necessary? For the above files? If so, we
> >can remove libpq.dll from lib. Or, libpqwalreceiver.dll needs it?
>
> Not sure. Perhaps you could experiment and see if anything bad happens if
> libpq is just installed in the bin directory and not in lib.

Yes, it does need it.

Andres

From: "Andrew Dunstan" <andrew(at)dunslane(dot)net>
> I don't have a problem adding them to the bin directory. I'd be very
> slightly wary of removing them from the lib directory, for legacy reasons.
> Maybe these changes should be done for git tip and not backported (or
> maybe just to 9.3). Or we could just decide to clean this mess in one fell
> swoop.

I think just adding them to the bin is enough for convenience. I don't
think backporting to 9.2 and before is necessary, if we consider this change
as an improvement. I'd appreciate it if you could do that for 9.3 and
later.

From: "Andres Freund" <andres(at)anarazel(dot)de>
> On 2013-07-28 15:37:49 -0400, Andrew Dunstan wrote:
>> >BTW, why is libpq.dll in lib necessary? For the above files? If so, we
>> >can remove libpq.dll from lib. Or, libpqwalreceiver.dll needs it?
>>
>> Not sure. Perhaps you could experiment and see if anything bad happens if
>> libpq is just installed in the bin directory and not in lib.
>
> Yes, it does need it.

Just out of curiosity, what needs libpq.dll in lib?

Regards
MauMau

On Thu, Jul 25, 2013 at 04:53:45PM -0400, Andrew Dunstan wrote:
> Jeff Janes asked me about this, and Bruce just tripped up on it.
> Usually on Windows it's necessary to have libpq.dll/cygpq.dll either
> in the PATH or in the same directory as client .exe files. The
> buildfarm client has for many years simply copied this dll from the
> installation lib to the installation bin directory after running
> "make install". But I can't really see why we don't do that as part
> of "make install" anyway. I haven't tested but I think something
> like this patch would achieve this goal - it would fix something
> that's tripped a lot of people up over the years.
>
> Comments? If we do this, should it be backported?

Andrew, where are we on this?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On 01/31/2014 12:25 PM, Bruce Momjian wrote:
> On Thu, Jul 25, 2013 at 04:53:45PM -0400, Andrew Dunstan wrote:
>> Jeff Janes asked me about this, and Bruce just tripped up on it.
>> Usually on Windows it's necessary to have libpq.dll/cygpq.dll either
>> in the PATH or in the same directory as client .exe files. The
>> buildfarm client has for many years simply copied this dll from the
>> installation lib to the installation bin directory after running
>> "make install". But I can't really see why we don't do that as part
>> of "make install" anyway. I haven't tested but I think something
>> like this patch would achieve this goal - it would fix something
>> that's tripped a lot of people up over the years.
>>
>> Comments? If we do this, should it be backported?
> Andrew, where are we on this?
>

Hmm, looks like it got dropped. I think my original patch is probably
about the the right thing to do, and given that it's already done by
the MSVC build it's a bug and should be backported, as Alvaro said in
the original discussion.

I'll get on that shortly - since the patch was untested I'll need to do
a test first.

cheers

andrew

On 1/31/14, 12:47 PM, Andrew Dunstan wrote:
> Hmm, looks like it got dropped. I think my original patch is probably
> about the the right thing to do, and given that it's already done by
> the MSVC build it's a bug and should be backported, as Alvaro said in
> the original discussion.
>
> I'll get on that shortly - since the patch was untested I'll need to do
> a test first.

I think this change is reasonable to make, but it should be in
Makefile.shlib, not in libpq/Makefile. Makefile.shlib already knows
about the difference between a link-time and a run-time dynamic library
(not to speak of Windows vs. others), so the right change there will
make it work for libpq, ecpg, and whatever else someone might come up
with automatically.

On 02/01/2014 08:01 AM, Peter Eisentraut wrote:
> On 1/31/14, 12:47 PM, Andrew Dunstan wrote:
>> Hmm, looks like it got dropped. I think my original patch is probably
>> about the the right thing to do, and given that it's already done by
>> the MSVC build it's a bug and should be backported, as Alvaro said in
>> the original discussion.
>>
>> I'll get on that shortly - since the patch was untested I'll need to do
>> a test first.
> I think this change is reasonable to make, but it should be in
> Makefile.shlib, not in libpq/Makefile. Makefile.shlib already knows
> about the difference between a link-time and a run-time dynamic library
> (not to speak of Windows vs. others), so the right change there will
> make it work for libpq, ecpg, and whatever else someone might come up
> with automatically.
>
>

In the end I went with the way I had suggested, because that's what the
MSVC system does - it doesn't copy any other DLLs to the bin directory.
So doing that seemed sane for backpatching, to bring the two build
systems into sync.

If you want to propose a better arrangement for the future, to include,
say, ecpg DLLs, and including changes to the MSVC system, we can discuss
that separately.

cheers

andrew

On 2/1/14, 3:22 PM, Andrew Dunstan wrote:
> In the end I went with the way I had suggested, because that's what the
> MSVC system does - it doesn't copy any other DLLs to the bin directory.
> So doing that seemed sane for backpatching, to bring the two build
> systems into sync.
>
> If you want to propose a better arrangement for the future, to include,
> say, ecpg DLLs, and including changes to the MSVC system, we can discuss
> that separately.

See attached patch.

There is also the commit fest item
https://commitfest.postgresql.org/action/patch_view?id=1330 that
requests the MSVC builds to install the epcg libraries in the bin directory.

On Wed, Feb 26, 2014 at 6:11 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> On 2/1/14, 3:22 PM, Andrew Dunstan wrote:
>> In the end I went with the way I had suggested, because that's what the
>> MSVC system does - it doesn't copy any other DLLs to the bin directory.
>> So doing that seemed sane for backpatching, to bring the two build
>> systems into sync.
>>
>> If you want to propose a better arrangement for the future, to include,
>> say, ecpg DLLs, and including changes to the MSVC system, we can discuss
>> that separately.
>
> See attached patch.
>
> There is also the commit fest item
> https://commitfest.postgresql.org/action/patch_view?id=1330 that
> requests the MSVC builds to install the epcg libraries in the bin directory.

Looking finally at this patch. In short, it moves to bin/
libpgtypes.dll, libecpg.dll and libecpg_compat.dll for cygwin and
MinGW build using some additional processing in Makefile.shlib,
removing at the same time the win32 stuff in libpq/Makefile.

IMO, it would be more readable to replace this part with a separate if
block for readability. So changing that:
- '$(DESTDIR)$(libdir)/$(shlib)' \
+ '$(DESTDIR)$(libdir)/$(shlib)' $(if $(findstring
$(PORTNAME),win32 cygwin),'$(DESTDIR)$(bindir)/$(shlib)') \
For that:
ifneq(blah)
blah2
endif

The MSVC portion of this fix got completely lost in the void:
https://commitfest.postgresql.org/action/patch_view?id=1330

Peter, could it be possible to merge this patch with its MSVC portion
for simplicity? I think that it would more readable to do all the
changes at the same time once and for all. Also, that's still a bug,
so are we still considering a backpatch? I wouldn't mind putting some
time into a patch to get that fixed..
Regards,
--
Michael

On Mon, Dec 22, 2014 at 2:05 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> Peter, could it be possible to merge this patch with its MSVC portion
> for simplicity? I think that it would more readable to do all the
> changes at the same time once and for all. Also, that's still a bug,
> so are we still considering a backpatch? I wouldn't mind putting some
> time into a patch to get that fixed..

Attached are two patches, one for MinGW/cygwin, a slightly modified
version from Peter and the second implementing the same thing but for
the MSVC scripts. The method for MSVC is similar to what is done in
Peter's patch: roughly it checks if SO_MAJOR_VERSION is present in the
Makefile of a given library, the path of Makefile is found by looking
at the location of the .rc in the vcproj file (could be better but I
could not come up with a better method). TBH, it would be good to be
completely consistent in the way we build things on Windows, and we
may as well consider a backpatch to fix this long-standing bug. The
MSVC patch removes of course the hack copying libpq.dll from lib/ to
bin/.

I mentioned the fix for MSVC scripts as well here:
http://www.postgresql.org/message-id/CAB7nPqQiUePzPhd3Mmk+Q7_cQQRKK_V1FVxKNyMRi660Z4dPzg@mail.gmail.com
Regards,
--
Michael

On Wed, Dec 24, 2014 at 3:08 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> Attached are two patches, one for MinGW/cygwin, a slightly modified
> version from Peter and the second implementing the same thing but for
> the MSVC scripts. The method for MSVC is similar to what is done in
> Peter's patch: roughly it checks if SO_MAJOR_VERSION is present in the
> Makefile of a given library, the path of Makefile is found by looking
> at the location of the .rc in the vcproj file (could be better but I
> could not come up with a better method). TBH, it would be good to be
> completely consistent in the way we build things on Windows, and we
> may as well consider a backpatch to fix this long-standing bug. The
> MSVC patch removes of course the hack copying libpq.dll from lib/ to
> bin/.
>
> I mentioned the fix for MSVC scripts as well here:
> http://www.postgresql.org/message-id/CAB7nPqQiUePzPhd3Mmk+Q7_cQQRKK_V1FVxKNyMRi660Z4dPzg@mail.gmail.com
Peter, this patch is waiting for input for a couple of weeks. IMO, it
would be good to finally get a fix for this bug, and we have patches
for both MSVC (the patch I sent) and mingw (your stuff).
--
Michael

On 1/15/15 2:37 AM, Michael Paquier wrote:
> On Wed, Dec 24, 2014 at 3:08 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> Attached are two patches, one for MinGW/cygwin, a slightly modified
>> version from Peter and the second implementing the same thing but for
>> the MSVC scripts. The method for MSVC is similar to what is done in
>> Peter's patch: roughly it checks if SO_MAJOR_VERSION is present in the
>> Makefile of a given library, the path of Makefile is found by looking
>> at the location of the .rc in the vcproj file (could be better but I
>> could not come up with a better method). TBH, it would be good to be
>> completely consistent in the way we build things on Windows, and we
>> may as well consider a backpatch to fix this long-standing bug. The
>> MSVC patch removes of course the hack copying libpq.dll from lib/ to
>> bin/.
>>
>> I mentioned the fix for MSVC scripts as well here:
>> http://www.postgresql.org/message-id/CAB7nPqQiUePzPhd3Mmk+Q7_cQQRKK_V1FVxKNyMRi660Z4dPzg@mail.gmail.com
> Peter, this patch is waiting for input for a couple of weeks. IMO, it
> would be good to finally get a fix for this bug, and we have patches
> for both MSVC (the patch I sent) and mingw (your stuff).

I have committed my mingw portion, but I cannot take on the MSVC part.

On Mon, Jan 19, 2015 at 12:41 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> I have committed my mingw portion, but I cannot take on the MSVC part.
OK, no problem. Perhaps I should add a new entry in the next CF for
the MSVC portion?
--
Michael