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

Allow setting BUILD_SHARED_LIBS to false for static build #2438

Closed
wants to merge 6 commits into from

Conversation

PaulWessel
Copy link
Member

@PaulWessel PaulWessel commented Jan 12, 2020

Creating a static GMT failed to produce working executables. There were several issues to address:

  1. The gmt_core_module.c produced by gmt_make_module_src.sh had a function gmt_core_module_lookup that we probably had never tested before. It was furthermore out of date, not knowing about modern module names. This has now been addressed and modules are again being found.
  2. The other problem was the reliance on a static array in gmt_init.c. For reasons not entirely clear to me, when supplemental modules were called and these again called functions in the static library, that static array (hash codes to handle parsing of GMT defaults) was reinitialized to zeros and no GMT defaults could be found. Making it a structure under the API private pointer means it is initialized once by GMT_Create_Session and does not need to me modified again during that session.
  3. The problem above is likely related to the fact that only the GMT core collection of modules are in the static library. The supplemental library (and any other GMT-compatible external modules, such as the MB-system plugins) remain as shared libraries accessed via the plugins (*.so files).

I have build both static and shared and run all tests and all seems well. It is important to give it a spin under Windows since shared/static behaves differently and I cannot guarantee that my solution works for Windows. Hence, need feedback from @joa-quim and @seisman on this.

Otherwise, closes issue #2436.

Otherwise, it gets recreated somehow to zero during static builds.
@seisman
Copy link
Member

seisman commented Jan 12, 2020

@PaulWessel Please remove the file "src/init" in this branch.

@seisman
Copy link
Member

seisman commented Jan 12, 2020

GMT still can't find supplemental modules, and the Linux and Windows builds fail probably due to some inconsistent compiler options. See the CI reports in #2440 for details.

@joa-quim
Copy link
Member

joa-quim commented Jan 12, 2020 via email

@PaulWessel
Copy link
Member Author

So I had to delete several builds that had libgmt.dylib and reboot before my static build would run and find the supplements. It would run fine in Xcode debug but then the same code would report not finding the supplements from the command line...I think the code is correct (for macOS for sure) but boy, Apple really is doing something odd under the hood with shared libraries and links.

@PaulWessel
Copy link
Member Author

@joa-quim, the supplemental and all other external libs will remain shared forever. The static is just about the core. We have no way of building static with libraries we don't even maintain (e.g., mbsystem.so).

@PaulWessel
Copy link
Member Author

Sure this is not OK? All checks passed.

Just add a comment regarding what is static and what remains shared
@seisman
Copy link
Member

seisman commented Jan 12, 2020

The CI in this PR are building shared libraries. See #2440 for static builds.

@seisman seisman closed this Jan 16, 2020
@seisman seisman reopened this Jan 16, 2020
@seisman seisman changed the base branch from 6.0 to master January 16, 2020 16:54
@PaulWessel
Copy link
Member Author

Hi @seisman, I guess this PR can be deleted once #3133 is merged?

@seisman
Copy link
Member

seisman commented Apr 24, 2020

Yes, we probably need to rethink about it if someone wants a static build in the future.

@PaulWessel PaulWessel closed this Apr 25, 2020
@PaulWessel PaulWessel deleted the staticbuildfix branch April 25, 2020 05:00
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.

None yet

3 participants