On Tue, Mar 3, 2015 at 8:36 PM, Asif Naeem <anaeem.it@gmail.com> wrote:
> Thank you Michael. I have looked the patch.
Thanks for the review!
> Overall logic looks good to me,
> I have checked it with MSVC{2013,2008}. It works for MSVC 2013 but fail for
> MSVC 2008, I think the condition "if ($proj =~
> qr{ResourceCompile\s*Include="([^"]+)"})" is not going to work for MSVC2008
> and MSVC2005 i.e.
> [...]
Thanks for the details, my patch is definitely missing vcproj entries.
Note that I don't have yet an environment with MSVC 2008 or similar, I
just got 2010 on my box for now. So you will have to wait until I have
a fresh environment to get an updated patch...
> It seems that search criteria can be narrowed to skip reading related
> Makefile for SO_MAJOR_VERSION string when VS project file is related to
> StaticLibrary or Application.
Well, agreed, and the patch takes care of that for vcxproj files by
only installing shared libraries if they use DynamicLibrary. Perhaps
you have in mind a better logic?
> Although this patch is going to make MSVC
> build consistent with Cygwin and MinGW build, following files seems
> redundant now, is there any use for them other than backward compatibility ?
> i.e.
>> inst\lib\libpq.dll
>> inst\lib\libpgtypes.dll
>> inst\lib\libecpg_compat.dll
>> inst\lib\libecpg.dll
Indeed, I haven't noticed them. But we can simply remove them as they
are installed in bin/ as well with this patch, it seems better for
consistency with MinGW and Cygwin in any case.
--
Michael