On Fri, Jul 11, 2014 at 12:02 AM, Noah Misch <noah@leadboat.com> wrote:
> I started editing these patches for commit, but I ran into two defects.
Thanks for looking at that and helping out!
> First, clean.bat requires an update to remove the new win32ver.rc files.
Done. The removal of the additional rc files is grouped at the
beginning of clean.bat.
> Second, the MinGW build uses few or none of the new PGFILEDESC entries; you
> need to mention $(WIN32RES) as in (most) existing PGFILEDESC-using makefiles.
I see, that's specified in src/makefiles/Makefile.win32. For the
portion of conversion_procs, updating directly proc.mk seems to be
enough, so done this way. Also, an entry in conversion_procs was not
updated according to the PGFILEDESC you introduced.
Btw, looking more at those patches, I found a limitation with
contrib/spi for MinGW builds: in order to pass WIN32RES to OBJS, some
of the contrib defined as MODULES need to be defined as MODULE_big to
accept a custom list of OBJS, but MODULE_big is not able to accept a
list of shared libraries. This limitation causes all the dll generated
in contrib/spi to not be versioned with MinGW. MSVC works fine though.
> Here are the patches as they stood at the time of those discoveries.
> Project::AddDir() does more than look for PGFILEDESC. Your patches placed
> AddDir calls next to code made redundant by those calls; I removed the
> now-redundant code. I prefer four-argument Solution::AddProject() over
> three-argument Solution::AddProject() followed promptly by Project::AddDir(),
> but that was a mere cosmetic judgement. Our prevailing style is to not
> uppercase the letter after the hyphen in PGFILEDESC, and I edited PGFILEDESC
> wording here and there. Please update these to fix the two bugs noted above.
Ah, right. Nice catch.
> On Sat, Jun 21, 2014 at 10:13:40PM +0900, Michael Paquier wrote:
>> On Sat, Jun 21, 2014 at 11:47 AM, MauMau <maumau307@gmail.com> wrote:
>> > bin\isolationtester.exe
>> > bin\pg_isolation_regress
>> > bin\pg_regress.exe
>> > bin\pg_regress_ecpg.exe
>> > bin\zic.exe
>> > lib\dict_snowball.dll
>> > lib\regress.dll
>> Yes, I'd like to improve that in a next patch hopefully in CF2, except
>> if a committer picking up this patch thinks differently.
>
> While I'd have preferred to see all binaries covered at once, that's fine.
Thanks, I am still planning to work more on those things for CF2 and
next commit fests.
Updated patches are attached.
Regards,
--
Michael