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.

Sign up for GitHub

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

Jump to bottom

Merge hpx_init and hpx_wrap into one static library #4462

Merged
merged 5 commits into from Mar 28, 2020
Merged

Merge hpx_init and hpx_wrap into one static library #4462

merged 5 commits into from Mar 28, 2020

Conversation

msimberg
Copy link
Contributor

@msimberg msimberg commented Mar 25, 2020 •
edited

Merges hpx_init and hpx_wrap, and moves them out of the main source directories. This seems to work on Linux at least, will wait and see that Windows and macOS also like this change. In theory there should be no problems with them being merged.

Fixes #4466 (maybe).

@msimberg msimberg added this to the 1.5.0 milestone Mar 25, 2020
@msimberg
Copy link
Contributor Author

Merges hpx_init and hpx_wrap, and moves them out of the main source directories. This seems to work on Linux at least, will wait and see that Windows and macOS also like this change. In theory there should be no problems with them being merged.

Flyby: I've attempted to simplify some things in the CMakeLists.txt, e.g. not using cuda_add_library for hpx and hpx_init. It should not be necessary if hpx_compute_cuda propagates the CUDA dependencies correctly.

hkaiser
hkaiser previously approved these changes Mar 26, 2020
View reviewed changes
Copy link
Member

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot!

@msimberg
Copy link
Contributor Author

I've updated this again. I had to add a separate config file for the init library, otherwise HPX_HAVE_DYNAMIC_HPX_MAIN would not be included in the config definitions. Things are working again on Linux, hoping for the same on the other platforms.

@msimberg msimberg marked this pull request as ready for review March 26, 2020 11:27
msimberg and others added 3 commits March 27, 2020 10:31
@msimberg
Copy link
Contributor Author

The latest commit changed compile tests to be executables rather than static libraries. This is due to to all of them being written with a main function and a few of them even including hpx_main.hpp (which caused failures, since hpx_main.hpp is now in a separate include directory). I think this is a convention we can stick to.

hkaiser
hkaiser previously approved these changes Mar 27, 2020
View reviewed changes
@msimberg
Copy link
Contributor Author

I don't know how it was triggered, but an ugly hack that I put in place for the header tests in the compute modules was causing problems. I've removed the hack as it's not needed anymore.

@McKillroy
Copy link

Built it on windows and except the broken "quickstart/search_by_key_demo" example it worked.

@McKillroy McKillroy mentioned this pull request Mar 27, 2020
Closed
@msimberg
Install hpx_init header files
4a73923 
Change hello_world_component to use hpx_main.hpp to test that hpx_main.hpp is
installed correctly.
@hkaiser hkaiser merged commit 9e5e05c into master Mar 28, 2020
@hkaiser hkaiser deleted the hpx-init-wrap-modules branch March 28, 2020 17:36
@msimberg msimberg mentioned this pull request Apr 17, 2020
Closed
@msimberg msimberg mentioned this pull request Apr 27, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hkaiser hkaiser hkaiser left review comments

@aurianer aurianer Awaiting requested review from aurianer

@biddisco biddisco Awaiting requested review from biddisco

Assignees
No one assigned
Labels
category: CMake category: init
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.

Compile error on Windows, current stable:
3 participants
@msimberg @McKillroy @hkaiser