Uh oh!
There was an error while loading. Please reload this page.
Allow building with cmake (to ease building on windows) - #16
Conversation
Optiligence
commented
Jan 27, 2016
Just my 2 cents when i see this: |
(This allows for easier builds on Windows)
Auto generated by CLion to cover cmake, c/c++ and autotools
trondn
commented
Jan 27, 2016
I've never used the GenerateExportHeader or PackageConfig thing before :-) Updated the set to use that |
Nulifier
commented
Feb 5, 2016
Your config.h.in file doesn't work for MSVC 2015. You need to either check for _MSC_VER <= 1900 or increment it as it still doesn't have ssize_t. |
jakirkham
commented
Apr 19, 2016
Would be great if this works on VS 2008 also. |
ccleve
commented
Aug 9, 2016
I cloned today's version of google/snappy and applied this pull request to it. It built fine and the unit tests ran fine as well. I'm on Windows 10 with Visual Studio 14. There were a few compile warnings (attached). If the warnings are not of concern, then please accept this PR. It'll make building projects that depend on snappy easier. |
rapoth
commented
Aug 27, 2016
@trondn Thank you for submitting this PR! I can confirm this works on today's version of google/snappy with similar warnings with Visual Studio 14 2015 on Windows 10. When I run the unit tests, I get a warning:
Do you by any chance happen to know why? |
| # I haven't looked at what the soname for the previous | ||
| # versions of snappy... let's just start with 1.1.3 | ||
| SET(SNAPPY_SONAME "1.1.3") |
There was a problem hiding this comment.
That variable is useless now that you use SNAPPY_MAJOR in SET_TARGET_PROPERTIES.
| @@ -0,0 +1,155 @@ | |||
| PROJECT(Snappy C CXX) | |||
There was a problem hiding this comment.
We export a C only API by wrapping the C++ code. See snappy-c.h.
| TEST_BIG_ENDIAN(WORDS_BIG_ENDIAN) | ||
| IF (WORDS_BIG_ENDIAN) | ||
| MESSAGE(STATUS "Builing on big endian system") |
vrabaud
left a comment
There was a problem hiding this comment.
That should probably be its own PR no ? The smaller the PR, the better.
vrabaud
commented
Feb 8, 2017
The three commits should probably be independent PR, not to lose track of the awesomeness of the CMake commit :) |
vrabaud
commented
Feb 8, 2017
For the warning in VS, maybe you can add something like: |
| ADD_DEFINITIONS(-DHAVE_CONFIG_H) | ||
| INCLUDE_DIRECTORIES(BEFORE ${Snappy_SOURCE_DIR}) |
There was a problem hiding this comment.
Forgive my Cmake noobishness, but Cmake best practices says do not use include_directories but target_include_directories instead.
| ADD_DEFINITIONS(-D_CRT_SECURE_NO_WARNINGS) | ||
| ENDIF (WIN32) | ||
| ADD_DEFINITIONS(-DHAVE_CONFIG_H) |
| * } | ||
| * free(output); | ||
| */ | ||
| SNAPPY_PUBLIC_API |
There was a problem hiding this comment.
Is it possible to remove all SNAPPY_PUBLIC_API and use WINDOWS_EXPORT_ALL_SYMBOLS instead?
Not to be noisy, but is there any news on this PR? Edit: I guess the work is continuing in PR ( #29 ). |
alkis
commented
Jun 2, 2017
Closing this because CMake support has been added internally and will be part of the next release. |
The patch series allows for building on windows with the following steps:
(It should also build on unix systems by dropping the "-G NMake Makefiles" part)