Skip to content
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.

Sign up for GitHub

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

Jump to bottom

WIP: Minimal cmake build script #28

Merged
merged 4 commits into from
Mar 10, 2015
Merged

WIP: Minimal cmake build script #28

merged 4 commits into from
Mar 10, 2015

Conversation

tkelman
Copy link
Contributor

@tkelman tkelman commented Mar 8, 2015

The disallow_intree_builds() snippet is thanks to Tracy Wadleigh. Tested on Linux, will check with MSVC/MinGW next.

will close #23

@tkelman
Copy link
Contributor Author

tkelman commented Mar 8, 2015

Tested, at least that it'll build the library, on MSVC and MinGW (-G"MSYS Makefiles") too. Builds either static or shared library, defaults to static unless you ask for -DBUILD_SHARED_LIBS=ON. Can still be spruced up for building the tests, adding an install target, etc.

stevengj
stevengj reviewed Mar 8, 2015
View reviewed changes
.gitignore
@@ -8,7 +8,6 @@
*.dll
*.dylib
*.dSYM
*.txt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add data/*.txt (or similar) back in, to exclude the Unicode data files that are downloaded there.

@stevengj
Copy link
Member

stevengj commented Mar 8, 2015

Don't move lump.txt into data/, since it is documentation and not data. Maybe rename it to lump.md and make a Markdown-formatted version. (Also need to change mentions of lump.txt in the README and utf8proc.[ch] accordingly.)

@stevengj
Copy link
Member

stevengj commented Mar 8, 2015

Also, can you add a test to the .travis.yml file? (And we should test the Windows version with AppVeyor.)

@stevengj
Copy link
Member

stevengj commented Mar 8, 2015

Also should have a VERSION and SOVERSION, right?

@tkelman
Copy link
Contributor Author

tkelman commented Mar 8, 2015

Also should have a VERSION and SOVERSION, right?

Yeah. Will be a little annoying to have to remember to keep the Makefile and CmakeLists synchronized here, won't it?

@kmsquire
Copy link

kmsquire commented Mar 8, 2015

Why not just have CMakeLists.txt?

@tkelman
Copy link
Contributor Author

tkelman commented Mar 8, 2015

If we make CMakeLists.txt do everything that the current Makefile does, sure. Should we just duplicate the info until that point?

@kmsquire
Copy link

kmsquire commented Mar 8, 2015

I would think so, assuming @stevengj is okay with that plan.

@tkelman tkelman force-pushed the tk/cmake branch 3 times, most recently from 59a601c to 9a245ac Compare March 9, 2015 00:28
@tkelman
Minimal cmake build script
a8b688c 
move flags for MSVC

rename lump.txt to lump.md, add data/*.txt to .gitignore
@tkelman
test cmake on travis
d93070d
@tkelman
Create appveyor.yml
86f9019 
only testing with default 32 bit MSVC and mingw.org at the moment

add semicolons, fix quoting

add fast fail functionality for redundant PR builds

need separate folders for msvc vs mingw

need to add path to mingw to /etc/fstab
@stevengj
Copy link
Member

stevengj commented Mar 9, 2015

In the long term, we can think about moving over to cmake wholesale. In the short term (utf8proc 1.2), I'd prefer to keep both and just duplicate the info.

@tkelman
Copy link
Contributor Author

tkelman commented Mar 9, 2015

Will tkelman@ce053be suffice for now? See the travis/appveyor logs for what that does to the output file names.

@stevengj
Copy link
Member

stevengj commented Mar 9, 2015

@tkelman, it looks like this is calling -Wl,-soname,libutf8proc.so.1.2.0 on Linux, which is wrong; the soname should be -Wl,-soname,libutf8proc.so.1, I think. libutf8proc.so.1.2.0 is the library's "real name" — see this HOWTO. So, I guess the cmake SOVERSION should be 1?

@tkelman
Copy link
Contributor Author

tkelman commented Mar 9, 2015

Only using ${SO_MAJOR} seems to work. The "real name" is getting set to libutf8proc.so.1.2-dev but I guess that's alright for now?

@nalimilan
Copy link
Member

The -dev suffix looks weird for a .so file name. Can't you get rid of it?

@stevengj
Copy link
Member

stevengj commented Mar 9, 2015

@tkelman, it sounds like cmake wants both SOVERSION and VERSION to refer to the ABI version, not the API version. hence SOVERSION should be 1 and VERSION should be 1.2, the same as the version in the Makefile (not the version in utf8proc.c).

@tkelman
Temporary fix for getting VERSION and SOVERSION into cmake
263421f 
only use ${SO_MAJOR} for cmake SOVERSION

use 1.2.0 for version in cmake

use only abi version for VERSION property in cmake
@tkelman
Copy link
Contributor Author

tkelman commented Mar 10, 2015

Okay, I did that. Now if only Travis would run.

stevengj added a commit that referenced this pull request Mar 10, 2015
@stevengj
Merge pull request #28 from tkelman/tk/cmake
08f101a 
WIP: Minimal cmake build script
@stevengj stevengj merged commit 08f101a into JuliaStrings:master Mar 10, 2015
@tkelman tkelman deleted the tk/cmake branch March 10, 2015 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cmake script
4 participants
@tkelman @stevengj @kmsquire @nalimilan