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

Fix includes for inclusion as an external project #44

Merged
merged 10 commits into from
Dec 15, 2020
Merged

Conversation

Shillaker
Copy link
Collaborator

@Shillaker Shillaker commented Dec 11, 2020

When using Faabric as an external project it can either be added with add_subdirectory in which case we need the public target_include_directories or as an installed library in which case the transitive header includes should still work.

@Shillaker Shillaker self-assigned this Dec 11, 2020
@Shillaker Shillaker marked this pull request as ready for review December 14, 2020 10:37
@Shillaker Shillaker requested review from csegarragonz and removed request for csegarragonz December 14, 2020 10:37
@Shillaker Shillaker changed the title Use public target include dirs Fix includes for inclusion as an external project Dec 14, 2020
@csegarragonz
Copy link
Collaborator

Closes #43

@csegarragonz csegarragonz linked an issue Dec 15, 2020 that may be closed by this pull request
@csegarragonz
Copy link
Collaborator

csegarragonz commented Dec 15, 2020

@Shillaker I was trying to rebuild this myself, but the examples build seems to fail (albeit the tests "pass"). In fact, if you check the latest action, under the tab Build Examples you will see that the build step for the server fails.

To replicate just run the steps in the workflow file:

inv dev.cmake --shared
inv dev.cc faabric --shared
inv dev.install faabric --shared
inv examples

To workaround this issue, we need to link with protobuf in examples/CMakeLists. The following works for me:

target_link_libraries(${example_name}
        ${FAABRIC_LIBS}
        pistache
        protobuf
        )

PS: I think this is not completely right, just a workaround that happens to work in my machine.

I will now have a look into why the tests actually pass.

@Shillaker Shillaker merged commit bab6054 into master Dec 15, 2020
@Shillaker Shillaker deleted the includes branch December 15, 2020 11:58
csegarragonz pushed a commit to csegarragonz/faabric that referenced this pull request Dec 15, 2020
* Public include dirs for libraries

* Update all includes

* Add spdlog dependency

* Formatting

* Adding linkage between object library and normal library

* Build proto headers first

* Missing dependency

* Fixing up examples build
@csegarragonz csegarragonz mentioned this pull request Dec 18, 2020
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.

Fix broken build
2 participants