This PR looks good to me modulo the MPI issues. @eschnett, @nrichart: are you guys ok with the -Qunused-arguments fix? I don't have a problem with it.

I'm ok with it. But just out of curiosity what are the unused arguments passed to clang? Shouldn't be those removed instead of hiding them?

What arguments are unused?

@eschnett @nrichart @tgamblin :
The unused arguments come from Spack's wrappers to compilers, i.e. CC=/Users/davydden/spack/lib/spack/env/clang/clang. Since I use Spack's compiled OpenMPI and OMPI_CC=cc, then that's what is used by mpiwrappers, AFAIK. Those unused parameters are numerous -L flags which are added to wrappers based on the dependencies of petsc.
Here's a part of PETSc config log where it got confused by those unused parameters as they are printed into stderr log even though they are warnings:

Executing: /Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/openmpi-1.10.2-37xieeupgsteaq6btru6wmhxfi44xqtn/bin/mpicc -c -o /var/folders/5k/sqpp24tx3ylds4fgm13pfht00000gn/T/petsc-8RPaEA/config.setCompilers/conftest.o -I/var/folders/5k/sqpp24tx3ylds4fgm13pfht00000gn/T/petsc-8RPaEA/config.setCompilers   /var/folders/5k/sqpp24tx3ylds4fgm13pfht00000gn/T/petsc-8RPaEA/config.setCompilers/conftest.c
Possible ERROR while running compiler:
stderr:
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/boost-1.60.0-eo7nn3v27nxa7lxqv5tttjzikshwt56i/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/bzip2-1.0.6-leelnsg3humpngfeofkrdpgtsofrm5ya/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/openmpi-1.10.2-37xieeupgsteaq6btru6wmhxfi44xqtn/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/hwloc-1.11.2-pxsmp4nhfdjc3jb7odj5lhppu7wqna5b/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/libpciaccess-0.13.4-erc6tr3ghndi5ed3gbj6wtvw6zl4vobf/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/zlib-1.2.8-cyvcqvrzlgurne424y55hxvfucvz2354/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/hdf5-1.8.16-diujq2w7ew4qviquh67b3lkcqsxtf77y/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/hypre-2.10.1-4kql32qmjpp7ict2qczkyyv6a4mbrgbl/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/openblas-0.2.16-qplsxnxlbaqnz2iltdo7fdhlayvuaxel/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/metis-5.1.0-i5y5b6r5ca4f77u5tketagpkn6joxasp/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/ncurses-6.0-sabhdwxbdbbapfo6wodglfmyo6u3c3hj/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/openssl-1.0.2g-answvmhu3lodpmgulgzryygkkimmsn34/lib'
clang: warning: argument unused during compilation: '-L/Users/davydden/spack/opt/spack/darwin-x86_64/clang-7.0.2-apple/parmetis-4.0.3-auubrjcwhqmqnpoqjwgwgz4bcjnxzunx/lib'

Consequently petsc config thinks something is wrong and no C/CXX preprocessor is available.

The same happens with every single build. Just other config system are less picky about this. That's why I believe the proper fix is NOT what is done here, but instead one should add -Qunused-arguments to cc and cxx wrappers.

p.s. For a record, here's again the discussion on PETSc mailing list: http://lists.mcs.anl.gov/pipermail/petsc-users/2016-March/028910.html

Note, that those -L flags are not only the dependencies of petsc, but actually the whole graph starting from petsc. If you ask me, i would remove those from wrappers completely as they harm more than they help. In this example petsc does not need to know anything about hwloc, libpciaccess, lib, ncurses or openssl. Additionally it's config system will add those flags anyway for things which are used. In other words, i would shift the burden of adding -L and -I flags to package developers. And that aint gonna be too many places. CMake config will be building those anyway, and only those which are needed...

Lastly, I am not sure wrapper will be working with externally provided MPI compilers. Say Intel MPI is installed on a cluster, it might get difficult to convince it inside Spack building system to use Spack's wrappers to Intel cc,cxx,etc compilers. So in this case package developers should not rely on the presence of extra flags in Spack's wrappers.

@davydden: Intel MPI (and most vendor MPIs) are based on MPICH. Intel MPI's mpicc respects MPICH_CC, MPICH_CXX, and MPICH_F90 seem to work just fine, at least as of Intel MPI v5.1.

The wrappers are there to make building easy for packagers, not to put extra burdens on them. They're intended to help out when building nasty things that do not have nice configure systems, or for which the build system doesn't provide convenient customization of dependency paths. There are a LOT of those packages in the HPC universe. They also make it very easy to make a baseline autotools package; often you don't even have to provide --with-foo=prefix when you make a first attempt at packaging something in Spack. Obviously some packages require a little help, but this has allowed us to get off the ground quickly.

We haven't had too much trouble with the wrappers so far. Aside from naming issues with libtool, they've been pretty transparent, and the added RPATHs help people avoid LD_LIBRARY_PATH hell. The -I parameters ensure that you don't pull in symbols from system versions of dependencies accidentally, and the -L parameters have been harmless with the exception of these warnings, which seem to be suppressed pretty easily. We could consider adding some of these flags to clang by default.

I don't see how the -L parameters are going to cause much harm, especially when you consider that all of those paths need to be in scope for ld.so at runtime anyway (the exception to this at the moment is build dependencies, but once #378 is merged, build dep paths will no longer be added erroneously). If anything, the paths ensure that the link is consistent when the packagers (or the build system) forget to include proper -L paths. This happens more often than you might think, especially for libraries that are commonly shipped with distributions, and since Spack is a system designed to support many concurrently installed versions of things, I think it's better to have some simple safety measures than it is to make the packagers verify all of this.

I could be proven wrong, but again, the wrappers have been pretty effective so far. Adding -L, -I, and -rpath is not more than your typical MPI wrapper compiler does, and plenty of people use those without issue. That said, there are probably arguments to be made about how deep in the DAG we should go when adding flags; we've already talked about reducing RPATHs to include only immediate dependencies, so that might happen.

One final use we have for the wrappers is to support applying tools to builds. We're pretty interested in being able to instrument installations with things like LLVM and other tools, and the wrapper give us a convenient handle on the entire build environment. A couple examples. First, we would like to make it possible to use some in-house thread sanitizers on entire packages, and at the moment this requires that the entire program be instrumented at compile time, including dependencies. The wrappers are great for this. There is also a cflags PR where we can inject flags straight into a build. We're interested in using that as an auto-tuning harness. Tools like Scalasca or TAU could also benefit and have been pretty receptive to the idea in the past. Spack is essentially an archive of packages whose build systems have already been instrumented with a compiler wrapper, which I think can get us some very useful functionality in the future.

Ok I think that answers everyone's questions -- thanks @davydden! Do you want to add an issue to address the cc and cxx unused argument flag issues?

@tgamblin it's good to know that it works for Intel MPI. I will create an issue for cc and cxx flags.