CMake for building the support library

[mknejp]

I have added a CMakeLists.txt that allows building the support-lib with CMake. The target name is djinni and it comes with a few options:

• DJINNI_WITH_OBJC adds the Objective-C support files to the target
• DJINNI_WITH_JNI adds the JNI support files to the target
• DJINNI_FRAMEWORK builds the library as a .framework on Apple systems
• DJINNI_STATIC_LIB builds Djinni as a static library instead of the default dynamic

There is nothing stopping you from having a single djinni library that contains both language support bindings if you're doing desktop work. The default library type is SHARED to make sure the JNI_OnLoad and JNI_OnUnload entry points are present. This has led to confusion in the past. If you chose to turn DJINNI_STATIC_LIB on it is your job to call jniInit() and jniShutdown() appropriately.

Because the CXX_STANDARD property cannot be propagated to consuming targets it needs to be set to >= 11 manually there.

The cache variable DJINNI_RUN_PATH is set to the location of Djinni's run script so it can be passed as argument to add_custom_command()-based generator scripts.

Note this includes the changes of #281 because it depends on its new file structure to make sure building from the repository and from an installed binary are source compatible.

[smarx]

Automated message from Dropbox CLA bot

@mknejp, it looks like you've already signed the Dropbox CLA. Thanks!

[artwyman]

Since they're mixed, this will need to wait to be resolved/merged until after #247.

This seems like a good idea, but since I don't use CMake myself, I can't really review it in detail. I'd welcome comments from others in the community who do use CMake. Lacking that, I'm relatively willing to merge whatever works for you, then accept further PRs if others try it out and suggest changes.

It doesn't look like the new CMake project for the support-lib is actually being referenced by the existing CMakeLists.txt files for the example or test suite. Am I misinterpreting that? It would be valuable for the new file to actually be used by something buildable out of the Djinni repo. Otherwise there will be nothing to ensure they don't break with future changes.

[mknejp]

It doesn't look like the new CMake project for the support-lib is actually being referenced by the existing CMakeLists.txt files for the example or test suite.

That's right. My priority was to get the support lib exposed to users first, then worry about adding the test suite later.

[artwyman]

I'd encourage you to go ahead and add some test coverage, lest it break constantly.
Since it's a new file with no impact on existing users, though, I'm okay with merging it for people to try out (once #247 is resolved), even without test coverage.

[4brunu]

I have tested this and it works :)
@mknejp @artwyman what do you guys think of making this PR independent of #247?
Because the pending PR (#247) seems to be a complicated and long process until it get's merged, and this PR is simple but extremely useful :)
If you want I can try to help tweak the CMakeLists.txt to work with the current Djinni configuration.

[artwyman]

Decoupling PRs is a good idea in my book.

[4brunu]

@mknejp Is it ok to you that I create a PR with the CMakeLists.txt modified to the current djinni structure, or do you want to do that?

[jesperrix]

What are the status of this one? I think it could be really usefull to build the supportlib with CMake.

[4brunu]

I think this PR is pending the approval of #281.
I have managed to create a simpler CMake integration to the current djinni structure, without the Framework support for iOS.
I'm not sure if I should create a PR with it, or wait for the pending PR's.
@mknejp @artwyman What do you think?

[mknejp]

I don't think the install target can be made code-compatible with source builds if #281 is not accepted first. Otherwise you'd have to change your code (or re-generate) based on whether you use Djinni directly form the repository or from pre-installed binaries/headers.

[mknejp]

This has now been adjusted for the changes in #281.

[mknejp]

Update: new option DJINNI_STATIC_LIB

[artwyman]

Sending this back to resolve conflicts and update after #281 is fully resolved (hopefully soon). I'll do another full review after that point.

[4brunu]

@mknejp I really would like to see this PR get merged, but it cannot be merged before https://github.com/dropbox/djinni/pull/281…
Any new on this?
If this will take too long to get merged, I would be willing to create a PR with CMake for building the support library with the current structure of djinni, or you could do it if you want, because you created the PR first 🙂
Even if the CMake integration wouldn't support all the features that this PR support, it would be good to have an immediate solution, and this PR could be an improvement.
@mknejp @artwyman What do you think about this?
Thanks 🙂

[artwyman]

@4brunu if you want to send a separate PR for CMake support without the reorg, I'd be open to that, since it doesn't look like #281 is likely to be resolved quickly.

[mknejp]

I think with #303 merged this isn't needed anymore. Unfortunately I haven't worked on a project that uses Djinni for quite a while now so I don't know if I ever get around to finishing this.