On Fri, 2008-02-08 at 12:06 -0500, Jeff Squyres wrote:
> On Feb 7, 2008, at 7:03 PM, Adam C Powell IV wrote:
>
> > But then, why wouldn't programs expect to be able to include C headers
> > in a C++ extern C block?
>
> But that's exactly the issue: mpi.h is not a C header. It is mandated
> by the MPI standard to be both a C and a C++ header file. We (Open
> MPI) can't change the filename or the languages where it can be used.
Since this is part of the MPI standard, I can accept this logic.
> > Or rather, why shouldn't they be able to do so
> > with mpi.h -- or hdf5.h, which isn't mpi.h
>
> Both mpi.h and hdf5.h do the right things with regards to extern
> "C" {}. So they are C++-safe.
As long as one doesn't #include them from within an extern "C" block.
> > -- when numerous other C
> > header files allow it, possibly including other MPI implementations?
>
> Are you sure of that? It is my [possibly flawed] understanding that
> HDF5 can be compiled with or without MPI support. If that is right,
> it is therefore possible that Salome has been developed/tested/used
> with serial HDF5, and this issue never came up.
Could be, but it also links with MPI, and several instances of "#include
<mpi.h>" occur within extern "C" blocks -- some of which specifically
surround these includes. I have to believe that they've tested this
with some MPI implementation, and that it worked...
> Can you get Salome to compile with an HDF5 that was built with support
> for another MPI implementation?
Haven't tried, see above.
> > After all, it's called mpi.h not mpi.hh or .hxx or mpi_cxx.h, right?
> > And isn't the patched version cleaner, in that it separates the C and
> > C++ prototypes into different #ifdef/#define regions?
>
>
> My objection to your patch is twofold:
>
> 1. OMPI is not doing anything wrong, and I'm reluctant to "fix"
> something that is not wrong without a very good reason. To be blunt:
> one open source application that is coded wrong (and can easily be
> fixed) is not a very good reason, IMHO.
Good point.
> 2. Your patch still requires the application to use an OMPI-specific
> #define. That doesn't seem like a good idea.
You're right, this is perhaps the strongest argument against my patch.
> Put differently: from your description, Salome is a non-conformant MPI
> application and therefore may or may not work with any given MPI.
> Instead of having each MPI that Salome (HDF5) fails to compile with
> provide a workaround, the [much] more portable solution is to have
> Salome fix their #includes. This will allow Salome to compile against
> any MPI implementation.
Indeed. That is one of my (many) patches to Salome. (Though very happy
with the resulting suite, I'm extremely disappointed with their build
system -- to the point where I can't see how they successfully built the
thing at all given the state of their code!)
> I'm clearly not the only OMPI developer, though, so if the community
> decides to accept your patch, I certainly won't prevent anyone from
> doing so. These are just my opinions. :-)
>
> Have you talked to the Salome developers? Wouldn't an upstream fix
> alleviate the need for you to put in an Open MPI-specific patch (or
> any patch at all)? Alternatively, have you talked to the HDF5
> maintainers to see if they can remove <mpi.h> from their public header
> files?
I've tried to contact them in several ways, and heard nothing back...
So it seems I'll need to maintain my patch set out-of-tree for the time
being.
Regards,
-Adam
--
GPG fingerprint: D54D 1AEE B11C CE9B A02B C5DD 526F 01E8 564E E4B6
Engineering consulting with open source tools
http://www.opennovation.com/
