-
-
Notifications
You must be signed in to change notification settings - Fork 66
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
Enable building E57Format as a shared library #40
Conversation
@pscamodio could you please try this to see if it works for you? The main differences from your PR are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi asmaloney
I'm testing this version of the lib and it seem to work.
But there's one problem with the cmake install
Currently if you build a shared library the .dll file is not installed
You need to add
RUNTIME DESTINATION bin to the install(TARGET call
install(
TARGETS
E57Format
EXPORT
E57Format-export
ARCHIVE DESTINATION lib
+ ARCHIVE DESTINATION bin
)
Time for coffee!
Thank you. |
Thanks for prompting this change with your PR @pscamodio! I would love to eliminate Xerces, but it's so embedded in this code it's not easy. I started to do it, but it really needs a rewrite. |
@asmaloney just found out this PR, you are mentioning getting ride of xerces, it seems the PDAL crew did it with their fork |
You are right, I made a mistake. I was trying to understand some differences in e57 handling between CC and pdal, found in their initial PR a mention of switching to libxml2, read the wrong cmakelist. Sorry for the noise |
No problem. I wish they hadn't forked the fork though 😄 There are few enough people doing this that it would be nice to keep any fixes in one place so everyone can benefit. 🤷♂️ |
No description provided.