Обсуждение: MSVC builld of 9.5.12 is broken?

Hello, hackers.

I've tried to build last state of REL9_5_STABLE branch (commit
1f19e46124eee8c6a54834) and under Win32 encountered  following errors:


  C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\bin\amd64\link.exe /ERRORREPORT:QUEUE
/OUT:".\Release\createdb\createdb.exe"
 
/INCREMENTAL:NO /NOLOGO
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\zlib\lib\zdll.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\openssl\lib\VC\ssleay32MD.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\openssl\lib\VC\libeay32MD.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\libintl\lib\libintl.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\iconv\lib\iconv.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\libxml2\lib\libxml2.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\libxslt\lib\libxslt.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\icu\lib\icuin.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\icu\lib\icuuc.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\dependencies\wineditline\lib64\edit.lib"
Release/libpq/libpq.lib Release/libpgcommon/libpgcommon.lib
Release/libpgport/libpgport.lib ws2_32.lib kernel32.lib user32.lib
gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib
oleaut32.lib uuid.lib odbc32.lib
odbccp32.lib /NODEFAULTLIB:libc /MANIFEST /MANIFESTUAC:"level='asInvoker'
uiAccess='false'" /manifest:embed /DEBUG /PDB:".\Release\createdb\createdb.pdb"
/SUBSYSTEM:CONSOLE /STACK:"4194304" /TLBID:1 /DYNAMICBASE:NO /NXCOMPAT /IMPLIB:".\Release\createdb\createdb.lib"
/MACHINE:X64 /ignore:4197 .\Release\createdb\win32ver.res .\Release\createdb\createdb.obj
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\Release\libpq\libpq.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\Release\libpgcommon\libpgcommon.lib"
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\Release\libpgport\libpgport.lib"
createdb.obj : error LNK2019: unresolved external symbol
handle_help_version_opts referenced in function main
[C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\createdb.vcxproj]
createdb.obj : error LNK2019: unresolved external symbol
connectMaintenanceDatabase referenced in function main
[C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\createdb.vcxproj]
createdb.obj : error LNK2019: unresolved external symbol fmtId
referenced in function main
[C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\createdb.vcxproj]
createdb.obj : error LNK2019: unresolved external symbol
appendStringLiteralConn referenced in function main
[C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\createdb.vcxproj]
.\Release\createdb\createdb.exe:
 
fatal error LNK1120: 4 unresolved externals
[C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\createdb.vcxproj]
Done Building Project
"C:\pgbuild\pgpro-9.5-msvc2013-amd64\builddir\postgresql\postgrespro-9.5.12.1\createdb.vcxproj" (default
targets) -- FAILED.

And much more similar errors for all applications in src/bin/scripts
Problem is reproducible with both MSVC 2010 and MSVC2013 and both 32
and 64 bit builds.

These errors seems to be caused by commit  91f3ffc5249  
which improves readability of src/bin/scripts/Makefile for
humans by moving list of common object files into variable.

Unfortunately, it makes this Makefile unreadable for perl scripts
in src/tools/msvc, which generate Microsift Visual Studio projects.

So, Mkvcbuild.pm should be somehow fixed as well. As this module all
consists of special cases and hidden knowledge, the simplest way to fix
is to add special parsing of SCRIPTS_COMMON variable.

--



--

On Tue, 27 Feb 2018 11:43:34 +0100
Magnus Hagander <magnus@hagander.net> wrote:

> On Tue, Feb 27, 2018 at 11:27 AM, Victor Wagner <vitus@wagner.pp.ru>
> wrote:
> 
> > Hello, hackers.
> >
> > I've tried to build last state of REL9_5_STABLE branch (commit
> > 1f19e46124eee8c6a54834) and under Win32 encountered  following
> > errors:
> >
[skip]
> 
> It's also interesting to note that this did not break in HEAD, 10 or
> 9.6. And none of those actually have the SCRIPTS_COMMON code.

It seems that it early stages of 9.6 cycle there was another approach
taken to improve readability of this Makefile - just all common code 
put into one C file. So there is no need for SCRIPTS_COMMON variable, 
because its name is longer than name of common.o which would be its sole
contents.

> I'm unsure why this was introduced in 9.5 and earlier, but not in the
> newer ones.  This smells like a possible backpatch mistake, in which
> case that part should probably be backed out of the old branches
> rather than teaching mkvcbuild about it.

Note that said commit (91f3ffc5249) is not limited to rearranging
makefile. It also changes a lot into C code itself. So it is not a
question of reverting commit - it is making new commit, which reverts
changes in just one file.



> Noah? Can you confirm if it was intentional?
 

On Tue, 27 Feb 2018 11:43:34 +0100
Magnus Hagander <magnus@hagander.net> wrote:


> I'm unsure why this was introduced in 9.5 and earlier, but not in the
> newer ones.  This smells like a possible backpatch mistake, in which
> case that part should probably be backed out of the old branches
> rather than teaching mkvcbuild about it.

Patch for Mkvcbuild.pm is actually quite simple (see attach).

Really, I have more complicated patch, which supports recursive
substitution as gmake does. It was developed to simplify inclusion of
PGXS extensions into contrib tree. (authors of modern extension often
use complex Makefile constructs).

I've not presented it to community because I hoped that current MSVC
build system would be soon replaced by cmake.

-- 

On Tue, 27 Feb 2018 13:35:33 +0100
Magnus Hagander <magnus@hagander.net> wrote:


> > Note that said commit (91f3ffc5249) is not limited to rearranging
> > makefile. It also changes a lot into C code itself. So it is not a
> > question of reverting commit - it is making new commit, which
> > reverts changes in just one file.  
> 
> 
> Oh, I missed that.
> 
> I think we should revert *just the changes to the Makefile*, and of
> course leave the rest of the comimt. Can you confirm if that fixes
> the problem?

It seems that it is not so easy. I've tried to revert changes in the
Makefile, and found out that some utilities (createlang, droplang,
pg_isready) now need more common object files, than they need before
this commit .

Attached patch to makefile which fixe problem for 9.3 branch.
I think it should do for 9.4 and 9.5 too.

--
 
 

On Tue, 27 Feb 2018 13:49:29 +0100
Magnus Hagander <magnus@hagander.net> wrote:


 
 
> Does not apply cleanly to 9.5 at least for me. Probably easy to fix,
> but I still feel we shouldn't mess around with the buildsystem in
> back branches unless we actually have to.

How interesting - somewhere between 9.3 (for which patch was made) and
9.5 path to the Makefile in windows-specific script  was changed from
Windows-style separators to unix-style and this broke context.

-- 

On Tue, 27 Feb 2018 14:14:04 +0100
Magnus Hagander <magnus@hagander.net> wrote:

> 
> Thanks. I've pushed this for 9.3-9.5.
> 
> Please verify that it looks good if you can (it takes a while for the
> buildfarm to get around to it)

Works at least for 9.5, although I haven't run tap tests on this build.

---

On Tue, Feb 27, 2018 at 11:43:34AM +0100, Magnus Hagander wrote:
> On Tue, Feb 27, 2018 at 11:27 AM, Victor Wagner <vitus@wagner.pp.ru> wrote:
> > These errors seems to be caused by commit  91f3ffc5249
> > which improves readability of src/bin/scripts/Makefile for
> > humans by moving list of common object files into variable.
> >
> > Unfortunately, it makes this Makefile unreadable for perl scripts
> > in src/tools/msvc, which generate Microsift Visual Studio projects.

> It's also interesting to note that this did not break in HEAD, 10 or 9.6.
> And none of those actually have the SCRIPTS_COMMON code.
> 
> I'm unsure why this was introduced in 9.5 and earlier, but not in the newer
> ones.  This smells like a possible backpatch mistake, in which case that
> part should probably be backed out of the old branches rather than teaching
> mkvcbuild about it.
> 
> Noah? Can you confirm if it was intentional?

It was intentional; the release made common.c depend on dumputils.c in 9.5 and
earlier, so everything that needs common.c now needs all four files.  (From 9.6,
relevant code had moved to libpgfeutils.)  Nonetheless, your fix was correct.
Thanks for pushing it.

On Tue, Feb 27, 2018 at 04:14:02PM +0300, Victor Wagner wrote:
> How interesting - somewhere between 9.3 (for which patch was made) and
> 9.5 path to the Makefile in windows-specific script  was changed from
> Windows-style separators to unix-style and this broke context.

You are looking for that, which was added during 9.4's development:
commit: 854adb83711da8fda2a8f028c27ad8956179c04a
author: Peter Eisentraut <peter_e@gmx.net>
date: Sat, 25 Apr 2015 08:58:01 -0400
Replace backslashes by forward slashes in MSVC build code

This makes it possible to run some stages of these build scripts on
non-Windows systems.  That way, we can more easily test whether file
moves or makefile changes might break the MSVC build.

Peter Eisentraut and Michael Paquier
--
Michael

В Wed, 28 Feb 2018 10:45:24 +0900
Michael Paquier <michael@paquier.xyz> пишет:


> Replace backslashes by forward slashes in MSVC build code
>
> This makes it possible to run some stages of these build scripts on
> non-Windows systems.  That way, we can more easily test whether file
> moves or makefile changes might break the MSVC build.

It would be nice to be able at least run perl -wc on these scripts on
non-windows platform.

Unforutnately following development seems to break this.

Now Mkvcbuild.pm depends on Win32 module and Win32API::File module,
both of which are not exist on non Win32 platforms.

Or there exist somewhere fake Win32 module which would satisfy
dependencies and do nothing?



--
                                   Victor Wagner <vitus@wagner.pp.ru>