v2.0.x: --with-hwloc=external fix #3066
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rhc54
approved these changes
Mar 2, 2017
|
@ggouaillardet could you please review? |
ggouaillardet
approved these changes
Mar 7, 2017
There was a problem hiding this comment.
This PR works fine for me.
two comments
- i made a PR to @jsquyres branch in order to add support for hwloc 1.5 (currently, hwloc from 1.8 is supported) hwloc: add support for hwloc v1.5 #3083 should be merged shortly. we might want to do this all at once.
- yes this is a pretty big change. that being said, only internal stuff is modified, and this is what distro maintainers are expecting (e.g.
--with-hwloc=external). so even if--with-hwloc=/usrseems to work, i am fine to do this in thev2.0.xbranch
jsquyres
force-pushed
the
pr/v2.0.x/hwloc-external-fix
branch
from
0b5c980
to
bf87a85
Compare
Frameworks are usually required to have a framework/framework.h file. However, this is sometimes problematic (see the hwloc use case/problem description, below). This commit allows frameworks to have an "autogen.options" file (i.e., project/mca/framework/autogen.options) that specifies things that autogen needs to know about the framework. Currently, the only option recognized in autogen.options is "framework_header", which allows a framework to specify that its header file is named something other than "framework.h" (the framework header file must still be in the project/mca/framework directory; it simply may be named something other than framework.h). More options may be introduced over time. The use case that motivated this is the hwloc framework (open-mpi#2616). Per MCA framework rules, the hwloc framework is required to have an opal/mca/hwloc/hwloc.h file. However, the hwloc library itself *also* has an hwloc.h file. This causes a problem when configuring Open MPI with --with-hwloc=external (meaning: do not use the hwloc embedded within the Open MPI source code tree -- instead, use an hwloc installation from outside the Open MPI source code tree). Specifically, when in the opal/mca/hwloc directory, the presence of "-I." in DEFAULT_INCLUDES (put there by Automake) causes a confusion between the hwloc.h in opal/mca/hwloc/hwloc.h and the system-installed hwloc.h. Chaos ensues (see the GitHub issue for more detail). The solution is to rename the opal/mca/hwloc/hwloc.h to something else (e.g., hwloc-internal.h), and extend autogen.pl to allow frameworks to have an alternate name for their framework header file. This commit introduces the autogen.pl mechanism to allow the alternate header file name. A follow-on commit will effect this change in the hwloc framework (and update all the places in the code base to use the new filename). Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit a065b9b)
Per a prior commit, the presence of "hwloc.h" can cause ambiguity when using --with-hwloc=external (i.e., whether to include opal/mca/hwloc/hwloc.h or whether to include the system-installed hwloc.h). This commit: 1. Renames opal/mca/hwloc/hwloc.h to hwloc-internal.h. 2. Adds opal/mca/hwloc/autogen.options to tell autogen.pl to expect to find hwloc-internal.h (instead of hwloc.h) in opal/mca/hwloc. 3. s@opal/mca/hwloc/hwloc.h@opal/mca/hwloc/hwloc-internal.h@g in the rest of the code base. Thanks to Orion Poplawski (@opoplawski) for reporting the issue. Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit fec519a)
hwloc v1.5 does not support HWLOC_OBJ_OSDEV_COPROC nor hwloc_topology_dup(), so for this version : - do not search for coprocessors - do not try hwloc_topology_dup(), note this is not used anywhere in the code base Thanks Jeff for helping with the wording Signed-off-by: Gilles Gouaillardet <[email protected]> (back-ported from commit open-mpi/ompi@7e01be6)
jsquyres
force-pushed
the
pr/v2.0.x/hwloc-external-fix
branch
from
bf87a85
to
1be33d2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2616 for v2.0.x. Thanks to @opoplawski for reporting.