#385 closed enhancement (fixed)
Add OS X framework build option to cmake
Reported by: | kyngchaos | Owned by: | mloskot |
---|---|---|---|
Priority: | minor | Milestone: | GEOS Fund Me |
Component: | Build/Install | Version: | main |
Severity: | Unassigned | Keywords: | cmake |
Cc: |
Description
patch and new file attached. Two new cmake settings:
GEOS_ENABLE_MACOSX_FRAMEWORK -- enables the framework build. I wanted to leave the option of building a standard pair of libraries on OS X. Default ON.
GEOS_ENABLE_MACOSX_FRAMEWORK_UNIXCOMPAT -- adds my "unix compatibility" feature, that makes a standard unix bin|include|lib structure with symlinks to the corresponding framework parts available to framework-challenged unix ports. Default OFF.
The CMAKE_INSTALL_PREFIX must be set in configuration, since the default /usr/local is not appropriate for frameworks. Since CMAKE_INSTALL_PREFIX defaults from cmake itself, I wasn't sure if/how I could default it to /Library/Frameworks if the user doesn't set it.
I used builtin cmake target features for creating a framework when possible. A couple things needed some (sometimes messy) customization (don't worry, it doesn't affect other systems).
- For a framework, I changed the name of the library to all caps, "GEOS". Case shouldn't matter on most Mac systems, but a case-sensitive file system is an option on OS X (not recommended), so I post-process the headers to match.
- A framework is a single binary library, so I build the C API into the main C++ library. It's possible to do special linking to join multiple libraries into one, but I've had problems with that in the past.
Attachments (7)
Change History (30)
by , 14 years ago
Attachment: | mac_framework.patch added |
---|
by , 14 years ago
Attachment: | Info.plist.in added |
---|
comment:1 by , 13 years ago
Owner: | changed from | to
---|
comment:2 by , 13 years ago
Status: | new → assigned |
---|
comment:3 by , 13 years ago
Keywords: | cmake added |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
William, thanks for the patch. Applied to the trunk r3375 . I have no way to test it on OSX, so I'd appreciated your help.
Sandro, all unit tests pass for me on Linux:
mloskot@dog:~/dev/geos/_svn/build$ uname -a Linux dog 2.6.35-28-generic #50-Ubuntu SMP Fri Mar 18 18:42:20 UTC 2011 x86_64 GNU/Linux mloskot@dog:~/dev/geos/_svn/build$ make test Running tests... Test project /home/mloskot/dev/geos/_svn/build Start 1: geos_unit 1/4 Test #1: geos_unit ........................ Passed 0.15 sec Start 2: xmltester 2/4 Test #2: xmltester ........................ Passed 11.17 sec Start 3: bug234 3/4 Test #3: bug234 ........................... Passed 0.00 sec Start 4: TestSweepLineSpeed 4/4 Test #4: TestSweepLineSpeed ............... Passed 2.66 sec 100% tests passed, 0 tests failed out of 4 Total Test time (real) = 13.98 sec
comment:4 by , 13 years ago
You forgot the info.plist.in. I added that as an attachment separate from the patch.
I put it at the top level of the source (src/info.plist.in) because it's a framework property of the main GEOS target, but maybe there's a more appropriate place? Maybe a macosx subfolder?
comment:5 by , 13 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:6 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
done in r3376.
Mat: note that TestSweepLineSpeed isn't meant to be run as a regression test, as it doesn't compare actual behavior with any expected one. Also bug234 is not meant to be automatically tested. At least isn't by automake, not sure what the origin was, but should be tested either by geos_unit or xmltester..
comment:7 by , 13 years ago
That's got it. Thanks! Build tests OK.
I rarely run the unit tests and didn't consider that in the patch. I'll work on a supplement patch, but you don't need to let it hold up release.
comment:8 by , 13 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Patch added to support the unit tests. Tests run, but 1 failure (but that's not for this bug).
by , 13 years ago
Attachment: | CMakeLists.patch added |
---|
patch to add missing library versions to framework
comment:9 by , 13 years ago
I can't believe I missed the library versions in the framework properties. Got all the framework version stuff. Patch above (for src/cmakelists.txt).
Note: it looks like the cmake library versions in general need adjustment. In the libtool build (at least on OS X), there was a calculation done so that the link version was 1+ the major version. On OS X this is the current and compatibility versions stored inside the library file. ie for GEOS 3.3 that would be 4.3.0. If someone builds the cmake GEOS 3.3 now, it will be 3.3.0, which is then seen as an earlier version than the previous 4.2 (3.2.x) to the linker, and programs will fail to load GEOS without a rebuild.
Maybe it's just a Mac thing, someone should verify this on *nix/windows. At least, a Mac non-framework build should set VERSION and SOVERSION as I do for the framework in the patch.
comment:10 by , 13 years ago
OK, old news, more patches. Fix for linking - prefix the install_name in the library. This actually needs to be done for both the framework and the unix builds on OS X, so the capi CMakeLists is patched also.
comment:11 by , 13 years ago
Priority: | major → blocker |
---|
@mat, do you have an estimate of when you'll be able to bring these in ? Should we wait for them before a final release ?
comment:13 by , 13 years ago
Applied William's patch with missing library versions for OSX framework (r3383)
follow-up: 15 comment:14 by , 13 years ago
Applied src_CMakeLists.txt.patch OSX framework (r3384) with slight modification:
- this bit was originally placed in non-Apple branch, so moved outside:
if(APPLE) set_target_properties(geos PROPERTIES BUILD_WITH_INSTALL_RPATH TRUE INSTALL_NAME_DIR "${CMAKE_INSTALL_PREFIX}") endif()
comment:15 by , 13 years ago
comment:16 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
The SOVERSION support is marked as TODO in CMakeLists.txt. I also added new ticket #446.
I think it's OK to close this. Please, reopen if it still needs addition.
comment:17 by , 13 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Sorry to beat this one...
The "if(APPLE AND GEOS_ENABLE_MACOSX_FRAMEWORK)" only gets the case of the framework build, so the the "else" can still be an OS X build, it's just a dylib. So those set_target_properties() are OK inside the else block. I guess it does no harm, since the same properties are set for framework and dylib builds.
For the capi, it should stay inside the "if(NOT APPLE AND NOT GEOS_ENABLE_MACOSX_FRAMEWORK)", because a OSX dylib build will be in there, and for a framework build the geos_c target will not exist. NOTE: it should be "set_target_properties(geos_c", I messed that one up.
comment:18 by , 13 years ago
Indeed, that must go inside the non-framework block - the framework target is "GEOS", the non-framework target is "geos", and cmake is case-sensitive for targets. A framework build fails configuration because target "geos" does not exist if that bit is outside the non-framework block.
comment:21 by , 13 years ago
Milestone: | 3.3.1 → GEOS Future |
---|---|
Priority: | blocker → minor |
Doesn't look like this is happening anytime soon (unless someone provides a patch)
by , 13 years ago
Attachment: | bug#385.patch added |
---|
comment:22 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Thanks. Committed as r3485.
info.plist template, goes in src/