The current autodetect implementation seems like the wrong approach to me.
I'm rather unhappy the base functionality was hacked up like it was
without any advanced notice or questions about original design intent.
We seem to have a set of base functions which are now more unreadable than
before, overly complex, and which leak memory.
The intent of the installdirs framework was to allow this type of
behavior, but without rehacking all this infer crap into base. The
autodetect component should just set $prefix in the set of functions it
returns (and possibly libdir and bindir if you really want, which might
actually make sense if you guess wrong), and let the expansion code take
over from there. The general thought on how this would work went
something like:
- Run after config
- If determine you have a special $prefix, set the
opal_instal_dirs.prefix to NULL (yes, it's a bit of a hack) and
set your special prefix.
- Same with bindir and libdir if needed
- Let expansion (which runs after all components have had the
chance to fill in their fields) expand out with your special
data
And the base stays simple, the components do all the heavy lifting, and
life is happy. I would not be opposed to putting in a "find expaneded
part" type function that takes two strings like "${prefix}/lib" and
"/usr/local/lib" and returns "/usr/local" being added to the base so that
other autodetect-style components don't need to handle such a case, but
that's about the extent of the base changes I think are appropriate.
Finally, a first quick code review reveals a couple of problems:
- We don't AC_SUBST variables adding .lo files to build sources in
OMPI. Instead, we use AM_CONDITIONALS to add sources as needed.
- Obviously, there's a problem with the _happy variable name
consistency in configure.m4
- There's a naming convention issue - files should all start with
opal_installdirs_autodetect_, and a number of the added files
do not.
- From a finding code standpoint, I'd rather walkcontext.c and
backtrace.c be one file with #ifs - for such short functions,
it makes it more obvious that it's just portability implementations
of the same function.
I'd be happy to discuss the issues further or review any code before it
gets committed. But I think the changes as they exist today (even with
bugs fixed) aren't consistent with what the installdirs framework was
trying to accomplish and should be reworked.
Brian
