Msvc build fix (for generating git.sln)

[PhilipOakley]

The 'msvc-build --vs' for creating a git.sln file that is readable and compilable by Visual Studio 2008 (last working version;-) had bit rotted.

This series, on top of 1.9.5.msysgit.0 (commit/0d6f7c802) fixes most of the issues, including: the duplicate GUID, perl make dependencies, many debug improvements, and others.

It's still not perfect, but does now run properly to create a git.sln which will build without error in VS08 express ed.

Note that 'make common-cmd.h' is still required before the 'msvc-build --vs' will compile without error.

Ultimately this will be submitted to upstream Git once review feedback has been obtained, so I'm not expecting a merge (obviously), rather I'd like others to see if it works for them.

Philip

[dscho]

Much better, thanks!

[buildhive]

MSysGit - the development behind Git for Windows » git #261 SUCCESS
This pull request looks good
(what's this?)

[PhilipOakley]

I suspect that 317efc0 may not properly remove (unlink) an empty errors file, plus I may not have fully described the scenario that creates an empty errors file.

Also 912f0ff says that 'devenv git.sln /useenv' should be used but the msvc-build script doesn't use that. I'm not sure if this affects the final destination of the git.exe and whether/how it should be tested.

[PhilipOakley]

The PR has been force updated, so the last few commits have been adjusted.

In particular commit/912f0ff is unchanged. I have also updated the .gitignore to properly include some of the build products and ignore Debug and Release folders (which actually hadn't worked for a while).

I hope to put a consolidated patch onto the list (and possibly the git-users list) to try and wet folks appetites, especially those who like being /in/ Visual Studio ;-)

Comments and suggestions welcome.

[buildhive]

MSysGit - the development behind Git for Windows » git #262 SUCCESS
This pull request looks good
(what's this?)

[PhilipOakley]

The previous version, for those that care, is at https://github.com/PhilipOakley/git/tree/msvc-PR318-1

[PhilipOakley]

I just tested that the (VS2008) project will convert to VS2010 (express), which it did. It had been compiled in VS2008 previously which may have helped. I had 6 minor issues with failure to write manifests which completed on a second build - possibly a virus scanner issue.

Next is to try a clean clone of msysgit (from ground zero net install) to a 'space in dir name' directory and see if it completes all the way through to a VS2010 build without problem.....

[PhilipOakley]

I have now tested (SUCCESS) the patch series with 'spaces in filepaths' by simply renaming my msysgit base install directory to 'msy git' (with a space) and everything worked (once I'd remembered to 'make common-cmds.h' ;-) ).

I had tried to net-install msysgit to a 'space in path' folder but was thwarted by the intolerance of windres from MinGW binutils, so I put that down to an education & experience [aka mistake].

I haven't checked that it rebases cleanly to master yet, but a diff showed no indication of conflicts, so I'm very hopeful.

[PhilipOakley]

I have also tested (SUCCESS) the conversion of the project from VS2008 (.sln) to VS2010 express, and it converted fine and compiled.

There was a minor glitch which was probably a virus scanner latency issue where some (6 of 42) of the resulting .manifest files weren't written, but simply building again filled those 6 back in (no actual recompiles IIRC.
edit: Oops I forget that I'd comment it already.

[t-b]

@PhilipOakley
I have had a try here is was I did:

• make common-cmds.h
• msvc-build --vs

I've compiled git using VS 2010 Pro and it finished without errors.
Nice!

Minor nits:

• It would be nice if the startup project of the solution would be git and not libgit.
• The readme should mention msvc-build more prominently. And also talk about the --vs option.

[dscho]

make common-cmds.h
msvc-build --vs

How about adding a rule for git.sln to the Makefile such as:

git.sln: common-cmds.h
        msvc-build --vs

[PhilipOakley]

@t-b Thanks for the confirmation.
I've found details of how to set the startup project http://stackoverflow.com/questions/1238553/vs2008-where-is-the-startup-project-setting-stored-for-a-solution so I'll look into fixing that.

The readme needs to be double checked by a fresh pair of eyes as well (volunteers?)

@dscho The msvc-build needs to be coped from msysgit to /contrib to make that work but a target of git.sln is a good idea.

I've still to sort out how to get the install to work for an independent compile http://stackoverflow.com/questions/28969853/how-to-add-a-phony-target-halfway-through-a-make-file
I need to break apart the install target as well to get everything into /bin.

[PhilipOakley]

@dscho Perhaps a slightly easier fix is to include the common-cmd.h check in the msvc-build script, to avoid too many discussions with Junio about polluting His Makefile, though it does need some fixing. I think he prefers elegant fixes;-)

[dscho]

to avoid too many discussions with Junio about polluting His Makefile

While Junio requires convincing arguments for literally every change, he is hardly unreasonable. I do not expect opposition against the git.sln target in the Makefile. We could always put it into the Windows-specific section -- as I did with the git-wrapper.exe rule here.

[t-b]

I tried to upgrade the created solution to VS 2013 Community.
Unfortunately the build fails, see here.

[PhilipOakley]

Thanks.
The message "module unsafe for SAFESEH image" appears to refer to 'Safe Exception Handlers' and incompatibilities between compiler versions.

I'll google some more...

Philip
----- Original Message -----
From: Thomas Braun
To: msysgit/git
Cc: Philip Oakley
Sent: Sunday, March 15, 2015 12:41 PM
Subject: Re: [git] Msvc build fix (for generating git.sln) (#318)

I tried to upgrade the created solution to VS 2013 Community.
Unfortunately the build fails, see here.

—
Reply to this email directly or view it on GitHub.

[PhilipOakley]

The bulk (hundreds?) of the 'error' messages (in the pastebin output) are the
"error LNK2026: module unsafe for SAFESEH image.".
Once I hid those I see a few more
"fatal error LNK1281: Unable to generate SAFESEH image." (25 places).
There were no other errors in t-b's report.

I'm guessing that the SAFESEH option became a default in VS2013 as I doubt we passed it as an option (extracted from the git Makefile's dry run)

https://msdn.microsoft.com/en-us/library/9a89h429.aspx describes the MS view of the SAFESEH option.

[PhilipOakley]

A quick grep through the pastebin output shows that ALL the LNK2026 errors are for zlib.lib.

http://stackoverflow.com/a/28443212/717355 shows that it (zlib.lib) is a common problem for this SAFESEH gotcha (includes pictures of how to determine/fix).

Now to see if there is a newer zlib.lib available that already meets the needs of the MS safety check, or if (our) zlib.lib was implicitly safe from that threat anyway.

[PhilipOakley]

[These are my note summarising the day's googling]

It looks like fixing the "Safe Execution Handler"(SAFESEH) issue automatically for VS2013+ builds will be problematic. My googling suggests this is a specific to VS2013+/32-bit compilation, and the inclusion of the (pre-compiled) zlib library that doesn't include the Microsoft(MS) SafeSEH processing.

The MS https://msdn.microsoft.com/en-us/library/9a89h429.aspx page gives details of the flag, including that its only valid for x86 targets (i.e. invalid for x64).

The MS blog http://blogs.technet.com/b/srd/archive/2009/02/02/preventing-the-exploitation-of-seh-overwrites-with-sehop.aspx gives details of why SEH can be / is a security issue. I'm not sure if git can be forced into the risk zone.

As noted in my earlier comment to this PR, zlib is not compiled with SEH, http://stackoverflow.com/a/28443212/717355 (first picture shows the zlib properties page).

The solution of adding /SAFESH:NO to the linker flags is given in http://stackoverflow.com/a/15983453/717355.

For those who wish to try compiling zlib from source with the /SAFESEH option the blog http://www.francescofoti.com/2013/12/compiling-the-zlib-compression-library-with-visual-studio-2013/ gives some guidance, as does https://beeproc.wordpress.com/2012/12/29/building-static-zlib-v1-2-7-with-msvc-2012/ and http://www.tannerhelland.com/5076/compile-zlib-winapi-wapi-stdcall/.

Aside: http://lwn.net/Articles/307732/ offers some comfort for those seeking cross compatibility. Also https://gcc.gnu.org/ml/gcc-help/2012-04/msg00306.html

In summary (so far),
Either we add /SAFESH:NO to the linker flags unconditionally, and x64 users will need to remove it (not sure what the error message will say, nor if that will be obvious).
Or we instruct x86 users to add the flag into their solution/projects [not tried seeing if that is once for the solution, or once per project (30+)]

[YueLinHo]

I tried to upgrade the created solution to VS 2013 Community.
Unfortunately the build fails, ...

Also failed on VS2012.
See here

[PhilipOakley]

From: Yue Lin Ho
I tried to upgrade the created solution to VS 2013 Community.
Unfortunately the build fails, ...

Also failed on VS2012.
See here

Thanks. Looks like its the same (Windows) Safe Exception Handler for zlib issue again (zlib.lib(inflate.obj) : error LNK2026: module unsafe for SAFESEH image.). So it looks like it either became a default in VS2012, or VS determines what the base OS vesrion is, and if SEH is a default for your base OS it sets it for compiling (or something like that.

I also see the "gettext.h : warning C4819: The file contains a character that cannot be represented in the current code page (950). Save the file in Unicode format to prevent data loss" so I'll add that to my 'to look at' list ;-)

[YueLinHo]

I also see the "gettext.h : warning C4819: The file contains a character that cannot be represented in the current code page (950).

My OS is Win7 Pro 64 Chinese Traditional (the current code page (950))

[YueLinHo]

In VS2013,
PROJECT -> Property -> Property Pages dialog
-> Configuration Properties -> Linker -> Advanced
-> Image Has Safe Exception Handle -> No (/SAFESEH:NO)
(Set to almost all projects, except the libgit, vcs-svn_lib, xdiff_lib projects)

Now, build OK. see here

[PhilipOakley]

From: Yue Lin Ho
I also see the "gettext.h : warning C4819: The file contains a character that cannot be represented in the current code page (950).

My OS is Win7 Pro 64 Chinese Traditional (the current code page (950))

I'd suspected that may be the case. Looking at gettext.h itself, I presume the warning is from the copyright notice:

• Copyright (c) 2010-2011 Ævar Arnfjörð Bjarmason
  So I assuming/hoping it's not really an issue.
  Philip

[t-b]

@PhilipOakley
Yes, I'd also say that this warning about the non-ascii characters in the comment is not really worrying.

[YueLinHo]

Run MSVC debugger, I got this:

But, pdb is not match.

C:\msysgit\bin\git.pdb: PDB does not match image.
C:\msysgit\bin\git.pdb: PDB does not match image.
C:\msysgit\bin\git.pdb: PDB does not match image.
C:\Windows\git.pdb: Cannot find or open the PDB file.
C:\Windows\symbols\exe\git.pdb: Cannot find or open the PDB file.
C:\Windows\exe\git.pdb: Cannot find or open the PDB file.
C:\msysgit\bin\git.pdb: PDB does not match image.
C:\msysgit\bin\git.pdb\f803424b970a4a419600b367978a1efb10\git.pdb: Cannot find or open the PDB file.
C:\msysgit\bin\MicrosoftPublicSymbols\git.pdb\f803424b970a4a419600b367978a1efb10\git.pdb: Cannot find or open the PDB file.

[PhilipOakley]

From: Yue Lin Ho
In VS2013,
PROJECT -> Property -> Property Pages dialog
-> Configuration Properties -> Linker -> Advanced
-> Image Has Safe Exception Handle -> No (/SAFESEH:NO)
(Except the libgit, vcs-svn_lib, xdiff_lib projects)

Now, build OK. see here

That's excellent news. It proves it does work in VS2013 if we can get that option into the projects.

I think it's possible to add that flag globally via the build script just after
https://github.com/msysgit/msysgit/blob/master/bin/msvc-build#L62
with something like (I'm guessing) LDFLAGS += -LSAFESEH:NO

[YueLinHo]

After re-build all, pdb is matched. I can debug, now.

@PhilipOakley Thank you so much. ^_^

[PhilipOakley]

Beautiful! All we need now is the make install step ;-)

[PhilipOakley]

@YueLinHo Would you be able to look at the saved vcproj (assuming VS2013 saves it) and report which field contains the "SAFESEH:NO" option (or however its coded) so that I can ensure that we have an option in the Makefile (or more likely 'config.mak.uname' which has the special msysgit options) for this case [I don't have a VS2013 I can access at the moment to test it myself]

[my earlier guess]

I think it's possible to add that flag globally via the build script just after
https://github.com/msysgit/msysgit/blob/master/bin/msvc-build#L62
with something like (I'm guessing) LDFLAGS += -LSAFESEH:NO

[YueLinHo]

It's added into *.vcxproj file

    <Link>
      ...
      <ImageHasSafeExceptionHandlers>false</ImageHasSafeExceptionHandlers>
    </Link>

I cleanup the whole msysgit folder.
So, I redo the whole procedure.
Perhaps it is a chance that writing down the complete steps I did:
(LANG=C is set long time ago, because of my Chinese Traditional Win7 Pro 64 bit OS)

• the current branch master of msysgit is on ae22057d27ff938c0a0fdda2875ac783d1b606aa
• Execute c:\msysgit\msys.bat
• git version shows:

git version 1.9.5

• make common-cmds.h
• msvc-build --vs
• then, git.sln is executed and upgraded to VS2013
• Set /SAFESEH:NO, see:
  
• Set as StartUp Project on git project
• Build solution
• Execute cmd.exe
• cd to c:\msysgit\git
• cmd: git version, shows

---

Oh~ I still got the problem that git.pdb is not matched this time.
Re-build doesn't fix it. :-/ (last time, I remember I modified some settings, some path, but I forgot the detail this time. sad...)
So, I can't run debug anymore. sorry.

[YueLinHo]

refer this and this

[YueLinHo]

I used ChkMatch tool (see here), then get this result:

c:\msysgit\git>chkmatch -c git.exe git.pdb
ChkMatch - version 1.0
Copyright (C) 2004 Oleg Starodumov
http://www.debuginfo.com/


Executable: git.exe
Debug info file: git.pdb

Executable:
TimeDateStamp: 55277419
Debug info: 2 ( CodeView )
TimeStamp: 55277419  Characteristics: 0  MajorVer: 0  MinorVer: 0
Size: 47  RVA: 00179dc0  FileOffset: 001789c0
CodeView format: RSDS
Signature: {f276f0de-9309-4b8c-9430-1990d1a1db6d}  Age: 1
PdbFile: C:\msysgit\git\git.pdb
Debug info: 12 ( Unknown )
TimeStamp: 55277419  Characteristics: 0  MajorVer: 0  MinorVer: 0
Size: 20  RVA: 00179df0  FileOffset: 001789f0

Debug information file:
Format: PDB 7.00
Signature: {fb86f4d9-4d8d-4bac-aa0f-c465fb762a95}  Age: 8

Result: Unmatched (reason: Signature mismatch)

Use chkmatch -m git.exe git.pdb to make it matched

Still failed. (Debugger ran, but did not stop at break point.)

[dscho]

@YueLinHo this is impressive, thanks for all your work! I am sure that this will help @PhilipOakley to adjust the PR accordingly.

[PhilipOakley]

@YueLinHo, Thank you for that information. It's very useful.

I tried injecting the option into the git.sln that the script generates (which targets VS2008).

When I added ImageHasSafeExceptionHandlers="false" into one of the vcproj (credential-store), and then tried opening the git.sln in VS2010 (to observe the conversion) - it barfed on that way of specifying it, which matched the opther options.

It looks as if VS2008 expected the option to be passed as a command line linker option (i.e. no menu option) - It's just a question of searching for the right place to insert it into the existing VS2008 .vcproj files, using the style it used (I think).

I also tried adding -SAFESEH:NO" to the end of the AdditionalOptions=" segment of the Name="VCLinkerTool" section. This appears to pass through untouched to the "command line" properties

The "however" is that it doesn't detect that this "Image Has Safe Exception Handlers" has been set in the "advanced options"

On the plus side it does compile without errors (I'd previously shown that 28 Feb) so the option isn't corrupting anything. The question is whether adding -SAFESEH:NO" to the end of the AdditionalOptions=" works when upping to VS2012

Just (re-?)checked what happens in VS2010 when I set the "Image Has Safe Exception Handlers" , and it does add the <ImageHasSafeExceptionHandlers>false</ImageHasSafeExceptionHandlers> despite the presence of AdditionalOptions= [...] -SAFESEH:NO", so it maybe that the alternative (if no 'right place is found') will be to create a(nother) script that parses for all the .vcxproj files and inserts the offending lines when required - which is not too hard but is another manual step to be remembered, which is not a good policy ;-)

[PhilipOakley]

Sent: Friday, March 13, 2015 10:03 AM
make common-cmds.h
msvc-build --vs

How about adding a rule for git.sln to the Makefile such as:

git.sln: common-cmds.h
msvc-build --vsHmm. I like the idea, but I've hit some problems.

It seems like the make file doesn't like this double invocation whereby I run:
$ make git.sln
and then then this target then runs the script, which promptly does a --dryrun with other options, and the captured dry-rumn isn't the same as it would be if invoked by itself.

I've no idea how to handle this issue, so any suggestions would be welcome.

my latest series is here https://github.com/PhilipOakley/git/tree/msvc-bld-rbase, based on a recent junio/master (so I could test the SubmitGit)

Philip
(I'm only going to have intermittent access in the next week, so still no hurry...)

[YueLinHo]

@PhilipOakley
Could you tell me the meaning of "bld-rbase" ? Thank you. ^_^
bld => build?
rbase => rebase?

[PhilipOakley]

@PhilipOakley
Could you tell me the meaning of "bld-rbase" ? Thank you. ^_^
bld => build?
rbase => rebase?

Yes, you are correct. It was to test SubmitGit, so isn't fully functioning (though it has improved!).

The main problem now is the 'make git.sln' target, which is recursive, and gets the command line options 'wrong' (i.e. I don't understand why it does what it does - the internal call appears to miss the V=1 option).

P.

[dscho]

Since I want to focus on 2.x rather than retired Git for Windows 1.x, I close this ticket. Please, everybody, move along to git-for-windows#256. Thank you.