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

Static library support improvements #123

Merged

Conversation

past-due
Copy link
Contributor

@past-due past-due commented Apr 6, 2018

  • #define UTF8PROC_STATIC to disable DLLEXPORT
  • [CMake] Automatically define UTF8PROC_STATIC if BUILD_SHARED_LIBS is off
    • This extends automatic definition of UTF8PROC_STATIC to CMake builds based on CMake's BUILD_SHARED_LIBS status, thus automatically handling shared / dynamic / static builds.
  • [Makefile] Support additional UTF8PROC_DEFINES
    • This can be used to specify flags like -DUTF8PROC_STATIC without modifying CFLAGS / environment directly.
    • Simply define UTF8PROC_DEFINES=-DUTF8PROC_STATIC when calling make on utf8proc.

@stevengj
Copy link
Member

One issue here is that people using utf8proc.h on Windows seem like that have to #define UTF8PROC_STATIC before #include <utf8proc.h> if they are linking the static version, as otherwise the declarations will have __declspec(dllimport) decorations. Is there a workaround?

@past-due
Copy link
Contributor Author

past-due commented Apr 28, 2018

@stevengj: That is required, yes, but there are several other projects that do this. (Additionally, this is no worse than the current status-quo, where folks are editing the files manually to accomplish the same thing.)

However, if they are using CMake, this patchset takes advantage of the PUBLIC specifier of target_compile_definitions. Assuming dependencies are set up properly, all code that depends on utf8proc will inherit the UTF8PROC_STATIC define automatically (if building a static lib). So there is no need to manually define UTF8PROC_STATIC before #include <utf8proc.h> when using CMake and its BUILD_SHARED_LIBS setting.

(I'm not familiar with similar functionality for makefile builds, but this patchset does NOT add that define automatically in any case for those - it just adds the ability for the developer to do so without having to edit this project's files / makefiles.)

@stevengj
Copy link
Member

Fair enough. In any case, the situation on Windows certainly isn't any worse after this patch.

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.

2 participants