[bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

Started by MauMauabout 12 years ago11 messages
#1MauMau
maumau307@gmail.com
1 attachment(s)

Hello,

The attached patch implements the below proposal, and moves libecpg.dll,
libecpg_compat.dll, and libpgtypes.dll from lib to bin folder on Windows,
where they should be.

/messages/by-id/10470B394DB8486F93AC60107CC44C8B@maumau

As Andrew-san said, I don't expect it to be back-ported.

In addition, I'll remove libpq.dll from lib folder unless somebody objects.
Currently, libpq.dll is placed in both bin and lib. I guess libpq.dll was
left in lib because it was considered necessary for ECPG DLLs.

Would removing libpq.dll from lib cause any problem? No. The following
modules in lib need libpq.dll when they are loaded by postgres.exe by
LoadLibrary(). But the necessary libpq.dll is loaded from bin folder.
According to the DLL search order rule, DLLs are loaded from the same folder
as the loading program, which is postgres.exe in our case.

dblink.dll
libpqwalreceiver.dll
postgres_fdw.dll

Regards
MauMau

Attachments:

ECPG_DLL_not_move_libpq.patchapplication/octet-stream; name=ECPG_DLL_not_move_libpq.patchDownload
diff -rpcd a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
*** a/src/tools/msvc/Install.pm	Mon Dec  2 09:17:05 2013
--- b/src/tools/msvc/Install.pm	Tue Dec  3 17:10:43 2013
*************** sub Install
*** 75,80 ****
--- 75,83 ----
  
  	CopySolutionOutput($conf, $target);
  	lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');
+ 	move($target . '/lib/libecpg.dll', $target . '/bin/libecpg.dll');
+ 	move($target . '/lib/libecpg_compat.dll', $target . '/bin/libecpg_compat.dll');
+ 	move($target . '/lib/libpgtypes.dll', $target . '/bin/libpgtypes.dll');
  	my $sample_files = [];
  	File::Find::find(
  		{   wanted => sub {
#2MauMau
maumau307@gmail.com
In reply to: MauMau (#1)
1 attachment(s)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

From: "MauMau" <maumau307@gmail.com>

In addition, I'll remove libpq.dll from lib folder unless somebody
objects.
Currently, libpq.dll is placed in both bin and lib. I guess libpq.dll was
left in lib because it was considered necessary for ECPG DLLs.

The attached patch also removes libpq.dll from lib folder. I don't mind
whether this patch or yesterday's one will be adopted. I'll add this to
2014-1 commitfest this weekend if the patch is not committed until then.

Regards
MauMau

Attachments:

ECPG_DLL.patchapplication/octet-stream; name=ECPG_DLL.patchDownload
diff -rpcd a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
*** a/src/tools/msvc/Install.pm	2013-12-02 09:17:05.000000000 +0900
--- b/src/tools/msvc/Install.pm	2013-12-04 09:27:22.000000000 +0900
*************** sub Install
*** 74,80 ****
  		'symbols',            'share/tsearch_data');
  
  	CopySolutionOutput($conf, $target);
! 	lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');
  	my $sample_files = [];
  	File::Find::find(
  		{   wanted => sub {
--- 74,83 ----
  		'symbols',            'share/tsearch_data');
  
  	CopySolutionOutput($conf, $target);
! 	move($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');
! 	move($target . '/lib/libecpg.dll', $target . '/bin/libecpg.dll');
! 	move($target . '/lib/libecpg_compat.dll', $target . '/bin/libecpg_compat.dll');
! 	move($target . '/lib/libpgtypes.dll', $target . '/bin/libpgtypes.dll');
  	my $sample_files = [];
  	File::Find::find(
  		{   wanted => sub {
#3Asif Naeem
anaeem.it@gmail.com
In reply to: MauMau (#2)
1 attachment(s)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

Hi MauMau,

I have't completed tested all the expects of submitted patch yet. I would
like to share my findings so far. By looking at the patch I do feel that
there is room for improvement in the patch, Instead of moving related dll's
from lib directory to bin directory later in the installation process, copy
these files directly from release/debug directory earlier (PFA patch).

It seems unfortunate that Windows don't have RPATH or similar logic
implemented in OS. Alternate methods seems not appropriate, Only feasible
option seems to be placing related dependency dll in same executable
directory. At first one may think an alternate to create symbolic link for
relative path in bin directory e.g. libpq.dll -> ..\lib\libpq.dll but it is
unfortunate that normal user do require special permissions to create
symbolic link otherwise it could help. There is SetDllDirectory or
AddDllDirectory function available that effects only subsequent calls to
LoadLibrary and LoadLibraryEx.

I will look into this patch further and let you know about my more
findings. Thanks.

Regards,
Muhammad Asif Naeem

On Wed, Dec 4, 2013 at 5:07 PM, MauMau <maumau307@gmail.com> wrote:

Show quoted text

From: "MauMau" <maumau307@gmail.com>

In addition, I'll remove libpq.dll from lib folder unless somebody

objects.
Currently, libpq.dll is placed in both bin and lib. I guess libpq.dll was
left in lib because it was considered necessary for ECPG DLLs.

The attached patch also removes libpq.dll from lib folder. I don't mind
whether this patch or yesterday's one will be adopted. I'll add this to
2014-1 commitfest this weekend if the patch is not committed until then.

Regards
MauMau

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

Win_lib_bin.patchapplication/octet-stream; name=Win_lib_bin.patchDownload
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 55de9e5..2289426 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -91,7 +91,6 @@ sub Install
 	}
 
 	CopySolutionOutput($conf, $target);
-	lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');
 	my $sample_files = [];
 	my @top_dir      = ("src");
 	@top_dir = ("src\\bin", "src\\interfaces") if ($insttype eq "client");
@@ -290,6 +289,9 @@ sub CopySolutionOutput
 		{
 			croak "Could not parse $pf.$vcproj\n";
 		}
+		# Few libraries required to be placed in bin directory instead of lib because of
+		# Windows Dll search path order
+		$dir = "bin" if ($pf eq 'libpq' || $pf eq 'libecpg' || $pf eq 'libecpg_compat' || $pf eq 'libpgtypes');
 		lcopy("$conf\\$pf\\$pf.$ext", "$target\\$dir\\$pf.$ext")
 		  || croak "Could not copy $pf.$ext\n";
 		lcopy("$conf\\$pf\\$pf.pdb", "$target\\symbols\\$pf.pdb")
#4Peter Eisentraut
peter_e@gmx.net
In reply to: Asif Naeem (#3)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

You should be able to do this without specifically referencing the names
"libpq" or "ecpg". Look into the Makefile, if it defines
SO_MAJOR_VERSION, then it's a library you are concerned with, and then
do the copying or moving.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Asif Naeem
anaeem.it@gmail.com
In reply to: Peter Eisentraut (#4)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

Hi MauMau,

Other than my pervious comments, patch looks good to me. Thanks.

Regards,
Muhammad Asif Naeem

On Wed, Feb 26, 2014 at 2:14 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

Show quoted text

You should be able to do this without specifically referencing the names
"libpq" or "ecpg". Look into the Makefile, if it defines
SO_MAJOR_VERSION, then it's a library you are concerned with, and then
do the copying or moving.

#6MauMau
maumau307@gmail.com
In reply to: Asif Naeem (#5)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

From: "Asif Naeem" <anaeem.it@gmail.com>

Other than my pervious comments, patch looks good to me. Thanks.

Thanks for reviewing. I understood that your previous comment was to
suggest copying the desired DLLs directly from Release/Debug folder to bin.
But I'm afraid it cannot be done cleanly... CopySolutionOutput() copies all
DLLS to lib folder with no exception as follows:

if ($1 == 1)
{
$dir = "bin";
$ext = "exe";
}
elsif ($1 == 2)
{
$dir = "lib";
$ext = "dll";
}

It seems like I have to do something like this, listing the relevant DLL
names anyway. I don't think this is not cleaner.

if ($1 == 1)
{
$dir = "bin";
$ext = "exe";
}
elsif ($1 == 2 && /* the project is libpq, libecpg, etc. */)
{
$dir = "bin";
$ext = "dll";
}
elsif ($1 == 2)
{
$dir = "lib";
$ext = "dll";
}

What do you think? Am I misunderstanding your suggestion?

Regards
MauMau

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Asif Naeem
anaeem.it@gmail.com
In reply to: MauMau (#6)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

Yes. Can you please take a look at Win_lib_bin.patch I shared earlier ?, I
think it is or similar approach will be appropriate. Thanks.

Regards,
Muhammad Asif Naeem

On Tue, Jul 8, 2014 at 5:53 PM, MauMau <maumau307@gmail.com> wrote:

Show quoted text

From: "Asif Naeem" <anaeem.it@gmail.com>

Other than my pervious comments, patch looks good to me. Thanks.

Thanks for reviewing. I understood that your previous comment was to
suggest copying the desired DLLs directly from Release/Debug folder to bin.
But I'm afraid it cannot be done cleanly... CopySolutionOutput() copies
all DLLS to lib folder with no exception as follows:

if ($1 == 1)
{
$dir = "bin";
$ext = "exe";
}
elsif ($1 == 2)
{
$dir = "lib";
$ext = "dll";
}

It seems like I have to do something like this, listing the relevant DLL
names anyway. I don't think this is not cleaner.

if ($1 == 1)
{
$dir = "bin";
$ext = "exe";
}
elsif ($1 == 2 && /* the project is libpq, libecpg, etc. */)
{
$dir = "bin";
$ext = "dll";
}
elsif ($1 == 2)
{
$dir = "lib";
$ext = "dll";
}

What do you think? Am I misunderstanding your suggestion?

Regards
MauMau

#8Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Asif Naeem (#7)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

Asif Naeem wrote:

Yes. Can you please take a look at Win_lib_bin.patch I shared earlier ?, I
think it is or similar approach will be appropriate. Thanks.

I think the suggestion by Peter Eisentraut upthread was pretty
reasonable -- the Makefiles are already aware that they are building a
shared library, so scrape the info off them. The less hardcoded stuff
in the msvc framework, the better.

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9MauMau
maumau307@gmail.com
In reply to: Alvaro Herrera (#8)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

From: "Alvaro Herrera" <alvherre@2ndquadrant.com>

I think the suggestion by Peter Eisentraut upthread was pretty
reasonable -- the Makefiles are already aware that they are building a
shared library, so scrape the info off them. The less hardcoded stuff
in the msvc framework, the better.

I overlooked his mail. Do you mean something like this? (I don't know yet
how to express this in Perl)

for each Makefile in under top_dir_in_source_tree or src/interfaces
{
if (Makefile contains SO_MAJOR_VERSION)
{
extract NAME value from Makefile
move lib/lib${NAME}.dll to bin/
}
}

But the source tree contains as many as 206 Makefiles. I'm worried that it
will waste the install time. Should all these Makefiles be examined, or 16
Makefiles in src/interfaces/?

Regards
MauMau

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Heikki Linnakangas
hlinnakangas@vmware.com
In reply to: MauMau (#9)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

This patch has been sitting in the commitfest with no activity since
this email from July. What's the real status, who's got the ball on this
one? Please update the commitfest app accordingly. If nothing happens in
the next few days, I'll mark this as Returned with Feedback.

On 07/09/2014 03:53 PM, MauMau wrote:

From: "Alvaro Herrera" <alvherre@2ndquadrant.com>

I think the suggestion by Peter Eisentraut upthread was pretty
reasonable -- the Makefiles are already aware that they are building a
shared library, so scrape the info off them. The less hardcoded stuff
in the msvc framework, the better.

I overlooked his mail. Do you mean something like this? (I don't know yet
how to express this in Perl)

for each Makefile in under top_dir_in_source_tree or src/interfaces
{
if (Makefile contains SO_MAJOR_VERSION)
{
extract NAME value from Makefile
move lib/lib${NAME}.dll to bin/
}
}

But the source tree contains as many as 206 Makefiles. I'm worried that it
will waste the install time. Should all these Makefiles be examined, or 16
Makefiles in src/interfaces/?

Regards
MauMau

--
- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Michael Paquier
michael.paquier@gmail.com
In reply to: MauMau (#9)
Re: [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

On Wed, Jul 9, 2014 at 9:53 PM, MauMau <maumau307@gmail.com> wrote:

But the source tree contains as many as 206 Makefiles. I'm worried that it
will waste the install time. Should all these Makefiles be examined, or 16
Makefiles in src/interfaces/?

Not really. IMO the best solution is to extract the path of the
Makefile in the vcxproj file of the project in Install.pm by looking
at ModuleDefinitionFile or ResourceFile, and then to scan it for
SO_MAJOR_VERSION. This way it is possible to determine in
CopySolutionOutput where a library needs to be copied. I'd also rather
go with the def file, and not the rc file for the path identification.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers