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

Consolidate inconsistent CMake option names #1461

Merged
merged 7 commits into from Apr 28, 2015

Conversation

Projects
None yet
2 participants
@hkaiser
Copy link
Member

commented Apr 17, 2015

This is in response to #1430: Inconsistent CMake option names

hpx_add_config_define(HPX_RUN_MAIN_EVERYWHERE)
hpx_option(HPX_WITH_RUN_MAIN_EVERYWHERE BOOL "Run hpx_main by default on all localities (default: OFF)." OFF ADVANCED)
if(HPX_WITH_RUN_MAIN_EVERYWHERE)
hpx_add_config_define(HPX_HAVE_RUN_MAIN_EVERYWHERE)

This comment has been minimized.

Copy link
@sithhell

sithhell Apr 22, 2015

Member

I know that this has been there since a while, but I think it's harmful to have that set globally. It would make more sense to have that option on a application by application basis.

This comment has been minimized.

Copy link
@hkaiser

hkaiser Apr 22, 2015

Author Member

Sure that would be possible, but isn't that unrelated to the PR at hand?

This comment has been minimized.

Copy link
@sithhell

sithhell Apr 22, 2015

Member

Totally unrelated. Just wanted to make a note here.

@@ -94,5 +94,4 @@ if(_use_custom_allocator)
hpx_add_compile_flag_if_available(-fno-builtin-posix_memalign LANGUAGES CXX C)
endif()

hpx_add_config_define(HPX_MALLOC "\"${HPX_MALLOC}\"")

This comment has been minimized.

Copy link
@sithhell

sithhell Apr 22, 2015

Member

Why has this been removed?

This comment has been minimized.

Copy link
@hkaiser

hkaiser Apr 22, 2015

Author Member

You removed that on master yourself

This comment has been minimized.

Copy link
@sithhell

sithhell Apr 22, 2015

Member

D'oh ... misread the file where it was removed ... thought it had been removed from the root CMakeLists.txt, my fault.

@@ -38,14 +38,16 @@ The options are split into these categories:
* [link build_system.cmake_variables.HPX_NATIVE_TLS HPX_NATIVE_TLS]

This comment has been minimized.

Copy link
@sithhell

sithhell Apr 22, 2015

Member

It looks like not all cmake variables have been updated in the docs here. Could you rerun the doc generation please?

This comment has been minimized.

Copy link
@hkaiser

hkaiser Apr 22, 2015

Author Member

Yes, that's my fault, I'll update the doc files.

@sithhell

This comment has been minimized.

Copy link
Member

commented Apr 22, 2015

This is a breaking change. The ML needs to get notified before merging those changes. In addition, after merging, we need to adapt our automated buildbots to reflect those changes.

hkaiser added some commits Apr 22, 2015

Merge branch 'master' into fixing_1430
Conflicts:
	cmake/HPX_SetupAllocator.cmake
	docs/manual/build_system/cmake_variables.qbk
	src/components/binpacking_factory/CMakeLists.txt
	src/components/distributing_factory/CMakeLists.txt
Merge branch 'master' into fixing_1430
Conflicts:
	docs/manual/build_system/cmake_variables.qbk

hkaiser added a commit that referenced this pull request Apr 28, 2015

Merge pull request #1461 from STEllAR-GROUP/fixing_1430
Consolidate inconsistent CMake option names

@hkaiser hkaiser merged commit 2d50642 into master Apr 28, 2015

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@hkaiser hkaiser deleted the fixing_1430 branch Apr 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.