v4.0.x: Prefer external hwloc and libevent #5472
Merged
Conversation
This file contains hidden or 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
Signed-off-by: Gilles Gouaillardet <[email protected]> Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit open-mpi/ompi@ce2c9ff)
Switch from #-style to dnl-style. Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit open-mpi/ompi@83e4a45)
Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit open-mpi/ompi@a70ecf5)
Signed-off-by: Ralph Castain <[email protected]> (cherry picked from commit open-mpi/ompi@5cab823)
|
bot:mellanox:retest |
|
bot:retest |
jsquyres
changed the title
jsquyres
requested changes
Aug 7, 2018
There was a problem hiding this comment.
These commits nominally provide some of the requested functionality. But:
- Lots of warnings for external hwloc -- see Compiling with external hwloc results in many warnings #5524.
- There are cases with the external hwloc component that the external hwloc is not used but no explanation is provided as to why (but in other cases, a good explanation is provided).
- libevent and hwloc are quite different in their handling of internal vs. external. This may end up being confusing to users, and is going to lead to maintenance problems down the road, IMHO. I don't know how these two stack up against pmix's internal vs. external handling.
- README and/or NEWS needs to be updated to document this behavior.
- When the external components are selected, the configury of the internal components is confusing. From the output, it looks like the internal configury succeeds, but then it says it can't compile the component. Here's both hwloc and libevent internal configury outputs in this case:
checking whether hwloc configure succeeded... yes
checking for infiniband/verbs.h... (cached) yes
checking if MCA component hwloc:hwloc201 can compile... no
...
configure: /bin/sh './configure' succeeded for opal/mca/event/libevent2022/libevent
checking if MCA component event:libevent2022 can compile... no
At a minimum, I think #5524 needs to be solved before this PR is merged.
We know that hwloc:external will be configured first (because of its priority). Take advantage of that here in hwloc201 by having it refuse to configure / politely fail if hwloc:external succeeded. Also print out some additional lines in configure output indicating what is going on (i.e., hwloc:external succeeded, so this component will be skipped, or hwloc:external failed, so this component will be used). Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit 4e5f432)
We know that event:external will be configured first (because of its priority). Take advantage of that here in libevent2022 by having it refuse to configure / politely fail if event:external succeeded. Also print out some additional lines in configure output indicating what is going on (i.e., event:external succeeded, so this component will be skipped, or event:external failed, so this component will be used). Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit b063cb6)
|
Items 1 (warnings with external hwloc) and 5 (internal hwloc/libevent not configured -- and notice is given to explain why -- if external hwloc/libevent are selected) have been addressed with two new commits on this PR. I think items 2 (more ?AC_MSG_NOTICEs? when external is not selected) and 4 (README+NEWS) should be addressed before this PR can be merged. Given current resource constraints, item 3 (a wholesale revamp / unification of how internal-vs-external components are selected -- e.g., perhaps some common m4 between event/hwloc/pmix?) is outside the scope of this PR. |
ggouaillardet
commented
Aug 9, 2018
|
|
||
| # These flags need to get passed to the wrapper compilers |
Change # -> dnl. No code or logic changes. Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit 17aa64e)
Put argument to AM_CONDITIONAL inside []. No code or logic changes. Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit 80df3f0)
In order to make "make distclean" (and friends) work, we need to *always* invoke the embedded configure script -- even if we know that we're not going to use this component. But in cases where we know we're not going to use this component, we also need to avoid the side effects of the code path that is used when we *do* want to use this component. So split the two possibilities into two different macros: 1. MCA_opal_event_libevent2022_FAKE_CONFIG: which does almost nothing except invoke the underlying "configure" script. 2. MCA_opal_event_libevent2022_REAL_CONFIG: which does all the real work (including invoking the underlying "configure" script). Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit 69aa46e)
The Autoconf AC_CONFIG_* macros can only be instantiated exacly once
for any given file, *and* they must be in a code execution path at run
time for the target file to be generated at the end of configure.
For example, if you want to generate file ABC at the end of configure,
you must invoke the AC_CONFIG_FILES(ABC) macro in a code path that
will get executed when configure is run.
That's pretty straightforward.
What's not straightforward is two corner cases:
1. You cannot invoke the AC_CONFIG_FILES(ABC) macro for the same file
more than once. If you do, autoreconf will fail (even before you
can run configure).
2. If AC_CONFIG_FILES(ABC) is not in a code path that is executed by
configure, the file ABC is not registered properly, and ABC will
not be generated at the end of configure.
This applies to hwloc because hwloc's HWLOC_SETUP_CORE macro calls
both AC_CONFIG_FILES and AC_CONFIG_HEADER to setup its Makefiles
(etc.) so that targets like "make distclean" and "make distcheck" will
work properly. Hence, we *have* to invoke HWLOC_SETUP_CORE.
However, the MCA_opal_hwloc_hwloc201_CONFIG macro has a few side
effects. It would be nice to do able to do something like this:
```
if hwloc:extern is going to be used:
Invoke minimal HWLOC_SETUP_CORE (with no side effects)
else
Invoke full HWLOC_SETUP_CORE (with side effects)
fi
```
But we can't, because autoreconf will detect that AC_CONFIG_FILES has
been invoked on the same files more than once (regardless of whether
those code paths will be executed at run time or not). Kaboom.
Similarly, we can't do this:
```
if hwloc:extern is not going to be used:
Invoke full HWLOC_SETUP_CORE (with side effects)
fi
```
Because then hwloc's AC_CONFIG_FILES won't be registered properly when
hwloc:external *is* used (i.e., when the HWLOC_SETUP_CORE macro is not
in a code path that is executed at run time), and targets like "make
distclean" will fail because hwloc's Makefiles won't have been setup.
Kaboom.
But remember that the hwloc framework is a bit special: there will
only ever be 2 comoponents: external and internal. External is
guaranteed to be configured first because of its priority. So the
internal component (i.e., this component) immediately knows if it is
going to be used or not based on whether the external component
configuration succeeded or failed.
Specifically: regardless of whether the internal component (i.e., this
component) is going to be used, we have to invoke HWLOC_SETUP_CORE.
But we can manage the side effects: allow the side effects when
this/internal component is going to be used, and avoid the side
effects when this/internal component is not going to be used.
This is a little less clean than I would have liked, but because of
Autoconf's oddity about its AC_CONFIG_* macros, this is the only
solution I could come up with.
Signed-off-by: Jeff Squyres <[email protected]>
(cherry picked from commit 01e4570)
jsquyres
approved these changes
Aug 11, 2018
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.
(cherry picked from #5395 and #5466)