FPChecker: adding fpchecker package (https://github.com/LLNL/FPChecker)

[ilagunap]

Add FPCheckcer package

[alalazo]

Style tests are failing, but other than that LGTM

[ilagunap]

I addressed the style problems in the last commit. Can this be merged now?

[alalazo]

Can we add a conflict for unsupported compilers?

[alalazo]

Apologies, I missed this the first time I reviewed. It's fine if this package needs %clang, but we should probably add conflicts for other compilers.

[ilagunap]

I'm a bit confused with adding conflicts for other compilers. If I add a conflict with gcc and I built LLVM with gcc, that would create conflict, right? But that's not what I want because we don't care how LLVm was built. It seems that when I add add conflict with gcc, and only allow LLVm it will try to build the entire spec of dependencies with LLVM, which I don't want. Could you explain more why do we need a conflict? The doc is really vague on conflicts. Thanks!

[ilagunap]

I removed the conflicts I previously added with gcc, intel, etc. I don't understand why we need the conflicts. Could you give more details? @alalazo

[alalazo]

I'm a bit confused with adding conflicts for other compilers. If I add a conflict with gcc and I built LLVM with gcc, that would create conflict, right?

In the current software model compilers are node attributes. Their values can vary from one node to the other (but we generally privilege consistence) and allowed values come from a known set (e.g. intel, gcc, xl, clang, etc.). Dependencies are instead other kind of executables needed at either build-time or run-time or dependency libraries.

Since compilers are special cased, we have in the roadmap to turn them into proper dependencies but we're not at that point yet. This is just to say that we may be in one of those cases where our software model falls short and we need to find a workaround.

With that in mind, when you write:

conflicts('%gcc')

you are stating that you don't want fpchecker to be compiled with GCC. I assumed that was the case since you are explicitly forcing the compilers to be clang and clang++. If my understanding is correct what you really want is to use the libraries and compilers from:

depends_on('llvm')

to build fpchecker. Is that correct?

If we don't add conflicts this will work as long as fpchecker has no dependents. As soon as another package depends on fpchecker%gcc it will try to use gcc as a compiler and that may cause issues. Would that ever happen?

[ilagunap]

1. We don't expect that another package will depend on fpchecker. This tool is standalone and we don't support a model on which another package will use a service from fpchecker. If somebody does that it would yield undefined behavior.

2. Even if another package tries to build using fpchecker%gcc it won't actually use gcc because we force the compiler to be clang in CMake. We don't allow the user to specify a different compiler at build time (via setting CMake vas) because this tool is an extension of clang/LLVM.

3. Now, let's say my point 1 and 2 are invalid and we add conflicts('%gcc'). Doesn't that mean that users have to explicitly indicate clang like this spack install fpchecker %clang@12.0.1? If so, what happens if I don't have clang@12.0.1 installed in spack? In that case do I have to install first clang@12.0.1, add it to spack, and then run spack install fpchecker %clang@12.0.1? This seems to me like asking too much to users when all they need is simply spack install fpchecker.

[alalazo]

Even if another package tries to build using fpchecker%gcc it won't actually use gcc because we force the compiler to be clang in CMake.

This is true, but the model of the software is incorrect (will falsely report that the software uses GCC) and there may be parts of Spack that inject compiler flags based on %gcc rather than %clang. To be clear, I'm not saying not to do this since we don't have a clean way to support tools like fpchecker that are essentially compiler extensions - I am just pointing the attention to what might go wrong with this approach.

Even if another package tries to build using fpchecker%gcc it won't actually use gcc because we force the compiler to be clang in CMake. We don't allow the user to specify a different compiler at build time (via setting CMake vas) because this tool is an extension of clang/LLVM.

As far as I understand the only way to use fpchecker with another package is to register it as a clang-like compiler in Spack and then compiling the package in question with clang@X.Y-fpchecker. So I tend to agree with you that given the current limitations with compilers the best approach might be to avoid conflicts. Give me a couple of days to see if I can think of something better, but otherwise I'll merge as is.

[ilagunap]

Thanks. I understand your concern regarding this:

This is true, but the model of the software is incorrect (will falsely report that the software uses GCC) and there may be parts of Spack that inject compiler flags based on %gcc rather than %clang.
However, it doesn't seem to me that adding compiler conflicts will solve the issue.

Also note that fpchecker is not the first package that is an extension of LLVM and has this constraint. See the archer package: https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/archer/package.py

Archer can only be built with clang since it's an extension of it. We set clang and clang++ in cmake_args(self) and we didn't add any conflicts with other compilers. I'm essentially using the same recipe of archer in fpchecker.

Thank you - hopefully you can merge this soon.

[alalazo]

However, it doesn't seem to me that adding compiler conflicts will solve the issue.

It solves that issue, but adds others. For instance there's no link between ^llvm and the %clang while in this case we need clang to be provided by the underlying llvm. Thanks for pointing out archer, I wan't aware of that tool. I'll go ahead and merge this PR as is. We should probably revisit both recipes in some time when we'll introduce compiler as dependencies.

[alalazo]

Thanks!