Allow building with cmake (to ease building on windows), version 2 #29

vrabaud · 2017-02-27T13:24:35Z

This is a small rewrite of #16 , mostly to simplify the windows support and modify less code.

googlebot · 2017-02-27T13:24:39Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

vrabaud · 2017-02-28T08:25:00Z

@trondn, I can take over this pull request until it's merged but I would need you to agree to the Contributor License Agreeement. Is that ok with you ?

trondn · 2017-02-28T08:30:44Z

Sure :) (I don't know why the CLA check failed.. I've singed the CLA earlier..)

googlebot · 2017-02-28T08:38:36Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

vrabaud · 2017-02-28T08:40:33Z

ok, @trondn, the error was on my side, sorry. As asked by the bot, please confirm that you are ok with me committing what you authored, and we are good to go ! Thx !

trondn · 2017-02-28T08:44:15Z

Go for it :)

alkis · 2017-02-28T15:12:38Z

CMakeLists.txt

+CHECK_LIBRARY_EXISTS(fastlz fastlz_compress "" HAVE_LIBFASTLZ)
+CHECK_LIBRARY_EXISTS(quicklz qlz_compress "" HAVE_LIBQUICKLZ)
+
+CHECK_CXX_SOURCE_COMPILES("int main(void) { return __builtin_expect(0, 1) ? 1 : 0; }"


Wouldn't the following be enough:

CHECK_CXX_SOURCE_COMPILES("int main(void) { return __builtin_expect(0, 1); }" HAVE_BUILTIN_EXPECT)

alkis · 2017-02-28T15:12:51Z

CMakeLists.txt

+CHECK_CXX_SOURCE_COMPILES("int main(void) { return __builtin_expect(0, 1) ? 1 : 0; }"
+        HAVE_BUILTIN_EXPECT)
+
+CHECK_CXX_SOURCE_COMPILES("int main(void) { return __builtin_ctzll(0) ? 1 : 0; }"


alkis · 2017-02-28T15:16:50Z

CMakeLists.txt

+
+SET(SNAPPY_MAJOR 1)
+SET(SNAPPY_MINOR 1)
+SET(SNAPPY_PATCHLEVEL 3)


This should be PATCHLEVEL 4.

Optiligence · 2017-03-22T20:17:38Z

you can specify the version in the project command instead of manually setting it
SOVERSION will then be set automatically and WRITE_BASIC_PACKAGE_VERSION_FILE will also use it automatically.

Optiligence · 2017-03-22T22:10:00Z

everything ${CMAKE_INSTALL_PREFIX}/lib/cmake should be lib/cmake/Snappy, it currently installs to <build_dir>/<install_prefix>/<install_prefix>/lib/Snappy/cmake if a relative CMAKE_INSTALL_PREFIX is used

…n windows), version 2

Optiligence · 2017-03-22T23:39:59Z

also, the export set is currently kinda useless because it doesn’t export any targets (and this is basically what #16 (comment) was intended to be about)

i’d also vote for using lower case for the CMake code

I added proposed changes here https://github.com/Optiligence/snappy/tree/cmake
I’m not sure how to proceed with this though.

(This allows for easier builds on Windows)

vrabaud · 2017-03-23T13:12:19Z

@Optiligence , I believe I took your comments into account in a new commit (except fro capitalization for now). I also added some older fixes that I had squashed in the older CMakeLists.txt commit (shouldn't have sorry).

Optiligence · 2017-03-23T14:18:20Z

Several parts are different (missing, wrong), you can squash the commits from here https://github.com/Optiligence/snappy/tree/cmake_upper_case instead
although i’d really do the capitalization now too as a baseline instead of sometime later.
(I can also change the capitalization of my changes to upper case, although that defeats my intention of having it all lower case in the end.)
I also added a line on my previous HEAD commit and added another commit regarding static and shared flavors.

Optiligence · 2017-03-23T14:25:29Z

I was also thinking that the output name should contain the ABI version, as it currently does with autotools.

KindDragon · 2017-04-03T16:44:25Z

CMakeLists.txt

+CHECK_INCLUDE_FILES("string.h" HAVE_STRING_H)
+CHECK_INCLUDE_FILES("sys/byteswap.h" HAVE_SYS_BYTESWAP_H)
+CHECK_INCLUDE_FILES("sys/endian.h" HAVE_SYS_ENDIAN_H)
+CHECK_INCLUDE_FILES("stdint.h" HAVE_STDINT_H)


Duplicate from line 24

We need CMake 3.4 which is not in 14.04 hence the jump to 16.04.

wesm · 2017-04-18T19:23:16Z

Any updates here / anything others can do to help? We would love to put up a Snappy windows build on https://github.com/conda-forge/snappy-feedstock

alkis · 2017-04-22T09:15:02Z

Yes we are making progress. We should have arelease with cmake soon, plus continuous integration. On Apr 18, 2017 21:23, "Wes McKinney" <notifications@github.com> wrote: Any updates here / anything others can do to help? We would love to put up a Snappy windows build on https://github.com/conda-forge/snappy-feedstock — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#29 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAQO_bq7jxNyZ048ecAAdm9Ch1nQKA8Uks5rxQ2lgaJpZM4MNGTE> .

KindDragon · 2017-05-05T07:51:40Z

Please add CMake option to build tests https://gist.github.com/KindDragon/b8752d22541a6d9d488e066b6a93a853#file-snappycmakelists-txt-L113

alkis · 2017-06-02T06:55:57Z

I am closing this PR since we have submitted CMake internally and it will be part of the next release.

This lands google#29

vrabaud closed this Feb 28, 2017

vrabaud reopened this Feb 28, 2017

vrabaud force-pushed the cmake branch from e47ca7a to d9555c0 Compare February 28, 2017 08:39

alkis suggested changes Feb 28, 2017

View reviewed changes

jakirkham mentioned this pull request Mar 22, 2017

Allow building with cmake (to ease building on windows) #16

Closed

Optiligence pushed a commit to Optiligence/snappy that referenced this pull request Mar 22, 2017

Pull Request google#29: Allow building with cmake (to ease building o…

4aef60d

…n windows), version 2

vrabaud force-pushed the cmake branch from d9555c0 to f810875 Compare March 23, 2017 08:32

trondn added 2 commits March 23, 2017 09:41

Add support for building with cmake

fddfc36

(This allows for easier builds on Windows)

Compile fixes for Visual Studio 201[35] (using cmake & nmake)

192a75d

vrabaud force-pushed the cmake branch 5 times, most recently from ae6f3d6 to 5188792 Compare March 23, 2017 13:08

vrabaud force-pushed the cmake branch from 5188792 to cb53a06 Compare March 23, 2017 13:21

vrabaud force-pushed the cmake branch 3 times, most recently from 523dc27 to cf1aad0 Compare March 24, 2017 08:55

vrabaud force-pushed the cmake branch 2 times, most recently from 8145ab6 to 9b8e53a Compare March 24, 2017 10:58

KindDragon reviewed Apr 3, 2017

View reviewed changes

vrabaud force-pushed the cmake branch from 9b8e53a to 1c5d497 Compare April 6, 2017 08:50

vrabaud added 2 commits April 6, 2017 16:15

add more CMake fixes.

a433e00

Set CMake and Ubuntu 16.04 as default on Travis.

3a4cfc6

We need CMake 3.4 which is not in 14.04 hence the jump to 16.04.

vrabaud force-pushed the cmake branch from 1c5d497 to 3a4cfc6 Compare April 6, 2017 14:15

alkis closed this Jun 2, 2017

pwnall pushed a commit to pwnall/snappy that referenced this pull request Jun 5, 2017

Provide a CMakeLists.txt.

c813168

This lands google#29

Optiligence mentioned this pull request Jul 20, 2017

CMake improvements #38

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow building with cmake (to ease building on windows), version 2 #29

Allow building with cmake (to ease building on windows), version 2 #29

vrabaud commented Feb 27, 2017

googlebot commented Feb 27, 2017

vrabaud commented Feb 28, 2017

trondn commented Feb 28, 2017

googlebot commented Feb 28, 2017

vrabaud commented Feb 28, 2017

trondn commented Feb 28, 2017

alkis Feb 28, 2017

alkis Feb 28, 2017

alkis Feb 28, 2017

Optiligence commented Mar 22, 2017

Optiligence commented Mar 22, 2017

Optiligence commented Mar 22, 2017

vrabaud commented Mar 23, 2017

Optiligence commented Mar 23, 2017 •

edited

Loading

Optiligence commented Mar 23, 2017

KindDragon Apr 3, 2017

wesm commented Apr 18, 2017

alkis commented Apr 22, 2017 via email

KindDragon commented May 5, 2017

alkis commented Jun 2, 2017

Allow building with cmake (to ease building on windows), version 2 #29

Allow building with cmake (to ease building on windows), version 2 #29

Conversation

vrabaud commented Feb 27, 2017

googlebot commented Feb 27, 2017

vrabaud commented Feb 28, 2017

trondn commented Feb 28, 2017

googlebot commented Feb 28, 2017

vrabaud commented Feb 28, 2017

trondn commented Feb 28, 2017

alkis Feb 28, 2017

Choose a reason for hiding this comment

alkis Feb 28, 2017

Choose a reason for hiding this comment

alkis Feb 28, 2017

Choose a reason for hiding this comment

Optiligence commented Mar 22, 2017

Optiligence commented Mar 22, 2017

Optiligence commented Mar 22, 2017

vrabaud commented Mar 23, 2017

Optiligence commented Mar 23, 2017 • edited Loading

Optiligence commented Mar 23, 2017

KindDragon Apr 3, 2017

Choose a reason for hiding this comment

wesm commented Apr 18, 2017

alkis commented Apr 22, 2017 via email

KindDragon commented May 5, 2017

alkis commented Jun 2, 2017

Optiligence commented Mar 23, 2017 •

edited

Loading