-
-
Notifications
You must be signed in to change notification settings - Fork 445
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
Include Boost as CMake subproject #6510
base: master
Are you sure you want to change the base?
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
34d3833
to
f86b00c
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
f86b00c
to
2208267
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
@srinivasyadav18 This now works, I just have to make some fixes to the CI environments before merging |
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
9d5f9b0
to
cad8155
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
cad8155
to
04789f4
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
retest |
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
retest |
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
04789f4
to
a38189e
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
retest |
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
retest |
8e6f9e4
to
4e33db8
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
4e33db8
to
89ba1b2
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
89ba1b2
to
bebdc4a
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
@hkaiser This is ready to merge, thee CI failures are unrelated |
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
6dda989
to
c6a37f9
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
@@ -61,6 +61,7 @@ install( | |||
) | |||
|
|||
# Install dir | |||
set(HPX_CONFIG_IS_INSTALL TRUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By convention, in CMake files we usually use ON
and OFF
as boolean values.
cmake/HPX_SetupBoost.cmake
Outdated
"${Boost_MAJOR_VERSION}.${Boost_MINOR_VERSION}.${Boost_SUBMINOR_VERSION}" | ||
if(HPX_PARCELPORT_LIBFABRIC_WITH_LOGGING | ||
OR HPX_PARCELPORT_LIBFABRIC_WITH_DEV_MODE | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The libfabric parcelport has been removed recently.
cmake/HPX_SetupBoost.cmake
Outdated
filesystem | ||
functional | ||
fusion | ||
iostreams |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed for the iostream_component only. Should we separate this?
cmake/HPX_SetupBoost.cmake
Outdated
functional | ||
fusion | ||
iostreams | ||
log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed anymore as the libfabric parcelport has been removed.
cmake/HPX_SetupBoost.cmake
Outdated
optional | ||
parameter | ||
phoenix | ||
regex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex is required for the inspect tool only. Should we separate this (at least make it dependent on HPX_WITH_TOOLS
)?
cmake/HPX_SetupBoost.cmake
Outdated
bind | ||
config | ||
exception | ||
filesystem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filesystem dependency is optional, I believe. It is possibly needed for the inspect tool, not sure...
cmake/templates/HPXConfig.cmake.in
Outdated
# Boost has been installed alongside HPX | ||
# Let HPX_SetupBoost find it | ||
if(NOT HPX_CONFIG_IS_INSTALL) | ||
message(FATAL_ERROR "HPX_WITH_FETCH_BOOST=ON requires HPX to be installed.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this mean? Could you please try reformulating the error message to clarify what the user should (or shoudn't) do?
cmake/templates/HPXConfig.cmake.in
Outdated
# By default Boost_ROOT is set to HPX_BOOST_ROOT (not necessary for PAPI or | ||
# HWLOC cause we are specifying HPX_<lib>_ROOT as an HINT to find_package) | ||
if(NOT Boost_ROOT AND NOT "$ENV{BOOST_ROOT}") | ||
set(Boost_ROOT ${HPX_BOOST_ROOT}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please cmake-format this file as well?
The MacOS build that fetches Boost now fails because of the restrictions introduced ( |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
85d9e8a
to
fbb7fab
Compare
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Draft PR for properly fetching and installing boost as a CMake subproject.
Still needs fixes, but I paused working on it since we need boostorg/cmake#65 to move forward with this approach.
Can also move to using
find_package
Config mode, since Module mode (usingFind<PackageName>.cmake
) is being deprecated for Boost.