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.

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

[CMake] Build the C API into static libs #102

Closed

Conversation

dakotahawkins
Copy link

To support building (e.g., GDAL) against static libs only, add the C API
to the static target.

Otherwise, if building with CMake, the C API is only available in a
dynamic library.

In the future, this could be a lot cleaner -- e.g. with more idiomatic
CMake configuration of shared vs. static builds -- but for the time
being I hope this will be acceptable.

To support building (e.g., GDAL) against static libs only, add the C API
to the static target.

Otherwise, if building with CMake, the C API is only available in a
dynamic library.

In the future, this could be a lot cleaner -- e.g. with more idiomatic
CMake configuration of shared vs. static builds -- but for the time
being I hope this will be acceptable.
@dakotahawkins
Copy link
Author

See #103 for a 3.6 backport of this change.

@dakotahawkins
Copy link
Author

Fixes trac #878.

@dbaston
Copy link
Member

dbaston commented Jun 8, 2018

Any thoughts on this, @strk or @mloskot ?

@mloskot
Copy link
Contributor

mloskot commented Jun 8, 2018

@dbaston The whole CMake configuration should be rewritten completely. It's an old-school CMake written as quick prototype for convenience of developers who'd like to develop GEOS in IDEs. It used to be marked as unofficial build setup, not sure what's its status now.

Regarding @dakotahawkins 's hack, if it works it probably is fine.

@dakotahawkins
Copy link
Author

dakotahawkins commented Jun 14, 2018

@mloskot According to this:

Starting at GEOS 3.5, we officially also support CMake. We will happily accept tickets for GEOS 3.5 against CMake. We would appreciate a patch if you have one, but not required..

It does seem like the CMake needs a rewrite, but this capability is a much more immediate need for me personally.

@mloskot
Copy link
Contributor

mloskot commented Jun 14, 2018

@dakotahawkins It's been years since I looked at that page. Looks like @robe2 updated it.

@strk
Copy link
Member

strk commented Jun 14, 2018 via email

@dakotahawkins
Copy link
Author

@strk I'm still coming up to speed on it, personally. I didn't mean what I said to sound insulting, apologies!

Personally, I'm glad this project has CMake support at all!

Based on looking through trac issues (two relevant issues here and here, not sure why exactly the second one is closed) it seems like there is at least some consensus on room for improvement.

@strk
Copy link
Member

strk commented Jun 14, 2018 via email

@dakotahawkins
Copy link
Author

@strk I will look at it this weekend. The thing that worries me the most is weird MacOS stuff I won't be able to try very easily.

@robe2
Copy link
Member

robe2 commented Aug 6, 2018

Sorry I missed this and I already tagged 3.6.3 and 3.7.0beta2, I've slated it for 3.6.4

https://trac.osgeo.org/geos/ticket/878

@dakotahawkins
Copy link
Author

For what it's worth we're using this in production and it's working well.

@strk strk closed this in 3f09f50 Aug 19, 2018
@dakotahawkins
Copy link
Author

@robe2 @strk Why rewrite/reauthor the commit? Is that just the process?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants