-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
VS2015 compatibility #127
VS2015 compatibility #127
Conversation
…ompatibility, and snprintf inclusion
One can generate any VS project files with CMake already supported. |
True but the repo has pre-generated files for other VS versions, so this would just keep what is already there up to date. Not all users are familiar with CMake. |
ml /coff /Zi /c /Flmatch686.lst match686.asm | ||
ml /coff /Zi /c /Flinffas32.lst inffas32.asm | ||
ml /coff /Zi /c /safeseh /Flmatch686.lst match686.asm | ||
ml /coff /Zi /c /safeseh /Flinffas32.lst inffas32.asm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for adding /safeseh for older versions of Visual Studio... Is it actually supported and required in all versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Older ml.exe versions back to VS 2003.NET support /safeseh. From MS: "Marks the object as either containing no exception handlers or containing exception handlers that are all declared with .SAFESEH"
So if the .asm files are build w/o SAFESEH, and the VS linker flag /SAFESEH:YES is set, then the build will fail. This linker flag now the defaults to YES in VS 2012+. Building the .asm files with safeseh has no effect on earlier VS versions where the linker does not default to YES.
Having said all that, since there is a VS2015 specific asm build file being added, there is no danger in discarding this change if desired. I could only test on VS2015 so I cannot validate the regression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is recommended for older versions, I would think it should be separated as different patch set, so it can be validated separately by people who still use older Visual Studio version. The other choice would be dirty hack in my opinion as it would combine the two batch files and add detection for Visual Studio version and select code paths depending on it.
To have a save option for all VC version there is just a single line to be changed zlib\win32\Makefile.msc
copy zlib\win32\zlib.def and zlib\win32\zlib1.rc into zlib x86 |
There are many Visual Studio pull requests. Can someone who is able to test them across versions of Visual Studio merge them into a single pull request? Also please test it with the latest zlib develop branch here on github, and account for functions that have been added since 1.2.8. Thank you. |
See #182 which is based from #167. Other than that it also Provides some other fixes to the definition file and the resource script for the version info resource. The export Definition file was also fixed in it for all of the Project files and includes VS2013 for those who still use it. It targets the latest Develop branch though. |
Also #182 disables |
Please see zlib 1.2.9 for updated vstudio support. Please create new issues or new pull requests if there are problems or desired behaviors against that version. Thanks. |
[Issue madler#126] Fix implicit cast from unsigned char to signed int.
Includes project files, an AVX2-only option, ml compatibility, and snprintf inclusion