If this PR were merged, how would you forward variants?

Well, for the case of gtkplus, you would have to either run:

spack install gtkplus+X ^cairo+X ^pango+X

or set them in your packages.yaml. But at least it wouldn't crash! The danger then becomes there is nothing stopping someone from running:

spack install gtkplus+X

and later discovering that it doesn't work because the dependencies were built without X support. The stance I took to address this in #1553 was to raise an error if gtkplus+X is being built with cairo~X or pango~X. This is obviously not ideal.

In general, why would a package explicitly require otherpackage~feature, unless that package really doesn't build in that case? It seems that half the confusion that this PR addresses is that some packages say hdf5~mpi when they mean "I need HDF5, but I don't need MPI". That's wrong, as hdf5~mpi means "I need HDF5, and I really cannot handle an HDF5 with MPI at all". Grepping for the character ~ in dependencies should find most of these; one would assume that there's always a comment explaining the situation. (I assume that some cases were added merely out of desperation to make concretization work.)

Idea: Add a new named argument why_not to depends, and require it to be set to an explanatory string when the ~ character is used to exclude a feature from a dependency...

Finally, mpi is a special case since if there's MPI anywhere in the dependency chain, there's a good chance that cc will fail and mpicc needs to be used. I say that's a bug in the MPI package; it should probably change Spack's cc setting to its mpicc, or -- even better -- should declare its include and library requirements in a transparent manner instead of pointing to an opaque mpicc script. (I know that I volunteered to fix this, but sadly didn't have the time to do so yet.)

if there are any changes to contretizer coming from @tgamblin , i would not merge this PR until they are merged upstream and the problem is reassessed.

@eschnett :

if there's MPI anywhere in the dependency chain, there's a good chance that cc will fail and mpicc needs to be used.

Can you please elaborate? It should be perfectly fine to compile a serial-only packageA with GCC and then use it in another packagesB that utilize some MPI (say Openmpi compiled with GCC).

@adamjstewart Thank you for writing this up and bringing the issues to our attention. My thoughts are:

1. The write-up is valuable. But as @davydden said, it feels a bit all over the place. It's clear that things are broken and that this is frustrating, but it's not clear yet exactly what is broken or how we think it should be fixed. I think some editing of it would do us all service: to try to be more specific about WHAT the problems are, WHAT doesn't work, and HOW those things don't work. Maybe I could take some time with what you've written and a bunch of spack spec tries to flesh this out?

2. I'm wondering why you don't think this PR should be merged, since it looks to me like it does some good things. Namely:
   a) Variants should generally default the same way in different packages.
   b) As you point out, the ubiquitous attempts at manual variant forwarding are not working --- and they introduce at least as many problems as they solve. I think that getting rid of them is a good start. Clearly, we need to finish the job with Spack-supported variant forwarding or universal variants. But either way, we need to clear out the manual attempts.

BTW... I've finally articulated how the multi-valued variants in #2386 are different from boolean variants; and this could guide us in deciding when to use which feature. With boolean variants, a request for pkg will match pkg+x. But with multi-valued variants, I presume that a request for pkg-x=y will only match pkg-x=y.

@davydden Let me take HDF5 as example. If configured with +mpi, the HDF5 header files contain an #include <mpi.h>, and (obviously) also require the MPI libraries for linking. If you build another serial package against this HDF5 version, then you need to build and link it (although it is serial!) with mpicc.

If the MPI package properly declared its include and library requirements to Spack, then Spack would handle this automatically.

Hi all: just to clarify, the issue is really that the concretizer does not currently do backtracking for decisions made about variants. It can currently do one level of backtracking when it tries to figure out which implementation of a virtual dependency to choose. What it should do is backtrack from much deeper levels as it searches for a satisfiable configuration. The variants should just be another generalized choice in the solver.

Implementing a fix for this is on my list, and I am kind of thinking (based on all the variant conflicts) that it should get moved up in priority. The dependencies in Spack have gotten more complex now that all of X11 and more fancy MPI options are in Spack. So I guess people discovered that the dependency model really does cover most of the things they want, but the concretizer needs to grow up.

I think a reasonable plan is:

1. Finish the stuff left for 0.10 and get it out the door.
2. I already pushed the exact versions stuff from 0.10 off to 1.0 because the changes to the concretizer were more sweeping that I would've liked. There are some concepts that have become conflated over time in the constraint solve ("satisfies" vs. "could satisfy") that need refactoring, and I don't trust the refactor without better package testing.
3. This change should probably be done concurrently with (2), as it affects the main part of the concretization algorithm.

So my suggestion is that we put this in the 1.0 bin and I can focus on it with @mathstuf @becker33 and @scheibelp between now and January-ish.

Thoughts?

There are some concepts that have become conflated over time in the constraint solve ("satisfies" vs. "could satisfy") that need refactoring, and I don't trust the refactor without better package testing.

This is worth another 👍

More thoughts: to mirror what others said, it's very helpful that @adamjstewart has written this up. The changes in this PR seem like they are intended to hard-code the best "common-case" choices for things, so that for unconstrained specs, things work decently and so that had package constraints don't throw too many monkey wrenches in the path to satisfiability.

We could merge this as a stop gap, but there is useful information in those constraints. I am not completely convinced that this PR is going to make things work better for most people. I think I'd need tests to see that.

At a really high level, my immediate priorities right now are:

1. Get the last few bugs out the door for 0.10
2. Getting a CDash site set up with a reasonably comprehensive set of package builds so that we can see what works and what does not. Right now we are playing whack-a-mole with github issues and we need a dashboard so we can make the thing converge to green.
3. Get the concretizer fixes in.

Does that sound like a reasonable set of priorities? I think (2) is necessary before we can do (3) right. Also, getting (2) working means we can start recording many of the tests cases @adamjstewart mentioned above in a set of nightly package tests. This makes (3) work that much better.

frankly i don't understand what's dramatically wrong with

depends_on('hdf5+mpi`, when='+mpi')
depends_on('hdf5~mpi`, when='~mpi')

Let's take a quick walkthrough with gtkplus, as it is the simplest problem to understand. Currently in develop, we have this set of conditions:

gtkplus

variant('X', default=True)

depends_on("pango")                                                         
depends_on("pango~X", when='~X')                                            
depends_on("pango+X", when='+X')

pango

variant('X', default=False)

depends_on("cairo")                                                         
depends_on("cairo~X", when='~X')                                            
depends_on("cairo+X", when='+X')

cairo

variant('X', default=True)

If you run spack spec gtkplus with no packages.yaml, you get:

==> Error: Invalid spec: '[email protected]%[email protected]~X arch=linux-fedora25-x86_64'. Package pango requires variant +X, but spec asked for ~X

If things were working properly, the concretizer would notice that I didn't specify any variants, default to gtkplus+X, and propagate +X down the line with the depends_on statements. But it isn't.

What are some possible solutions? What if we set the ~X to be the default for all 3 packages? Now, the following commands work:

spack spec gtkplus
spack spec gtkplus +X

But the following doesn't work:

spack spec gtkplus ^pango+X

In an ideal world, Spack would say that since pango needs +X, so does gtkplus, but it doesn't work. What about another scenario. If we set ~X to be the default for all packages, and a user happens to always need cairo+X, they could modify their packages.yaml like so:

packages:
  cairo:
    variants: +X

That's perfectly legal right? But now spack spec gtkplus no longer works.

I' m feeling really sorry as it seems I ( unintentionally ) opened a vase of pandora.

I can summarize here my intended use for spack and hopefully get some hints:

I' m trying to use spack to automate the building of a sw stack for remote visualization.
One of the target for installation is a cluster that has no GPU available, so one of the goal was to build Paraview with mesa.

Ideally this could be handled by a single spack install command..... but I do not expect it would be so easy.

I raised the issue and the following PR thinking to just fix typos in some underneath packages.

Unfortunately problems I detected involve spack internals such as concretization, and could not be solved at the package level.
Nevertheless, as I would really like to be able to code a working recipy in spack, any suggestion to good practice that relieve these problems would be really welcomed.

Many of you suggest using package.yaml: would it be possible to collect some real examples ( if it can be openly disclosed) of configuration that are really in use?

As some build steps ( as qt ) are really long, even splitting install into steps ( build tools, base libraries, applications ) could be acceptable, providing some hints on how to properly set up package.yaml to avoid rebuilding dependencies in following step.

@adamjstewart

But the following doesn't work:
spack spec gtkplus ^pango+X
In an ideal world, Spack would say that since pango needs +X, so does gtkplus, but it doesn't work.

thanks for explanation, now i see the problem.

I would still suggest do it step-by-step. First part of your PR which makes variants consistently true by default. This can go into 0.10 as it is won't break things. The rest needs to be tested, preferably grouped into suites, say all PDE packages like petsc, deal.ii, p4est, fenics, gmsh.

@adamjstewart :

But the following doesn't work:
spack spec gtkplus ^pango+X

is it because you have

depends_on("pango")                        

in addition to

depends_on("pango~X", when='~X')                                            
depends_on("pango+X", when='+X')

? The last two should be sufficient. Maybe this is the root of the bug?

@citibeth:

@tgamblin Then we will need to change a fundamental way that pattern-matching currently works.

No, we won't. This doesn't violate anything.

Let -> mean depends_on.

Consider :

• A -> B means A requires B but doesn't care how it is built. A+mpi or A~mpi will do.
• A -> B+mpi means that the B used to satisfy A must have +mpi.
• A -> B~mpi means that the B used to satisfy A must have ~mpi.

These are all valid builds if the only constraint is A -> B:

• A+mpi ^B+mpi
• A~mpi ^B+mpi
• A+mpi ^B~mpi
• A~mpi ^B~mpi

Adding these two constraints to a package:

• A -> B+mpi when A+mpi
• A -> B~mpi when A~mpi

Just means that these are the only two valid builds:

• A+mpi ^B+mpi
• A~mpi ^B~mpi

Any concrete A MUST have a value for the mpi variant, so A, concretized, will become either A+mpi or A~mpi. There is no "don't care" build -- it either has MPI or it doesn't when it's actually built. The constraints you're arguing against just impose a requirement on B based on how A is built and don't allow builds where one has MPI and the other doesn't.

What is the matter with that?

It sounds like everyone is on board with the first 2 commits that make sure that all mpi and X variants have the same default. Are there any other common variants that should be aligned? Of course, things quickly break if you change any of the defaults in packages.yaml, but there doesn't seem to be anything we can do about that until the underlying issue (the concretizer) is fixed.

It sounds like a lot of people are not on board with my last commit, which removes the depends_on variant forwarding. I'm fine with removing this commit for now if it helps get the first 2 commits in. I'm not sure how useful those depends_on statements are though. Even if it did work, it is kind of annoying to have to always say:

depends_on('hdf5')
depends_on('hdf5+mpi', when='+mpi')
depends_on('hdf5~mpi', when='~mpi')

I think there are two long term solutions. The universal variants/variant forwarding idea that @citibeth and I have proposed would solve the problem of having to be so annoyingly explicit on every dependency, and I think that is worth doing. As for which we go with (universal variants vs. variant forwarding) there are enough open issues discussing the pros and cons of both already. It can wait until after 0.10 I guess.

But the above syntax should still work. That is a bug in the concretizer that @tgamblin explained. That should also be fixed, and I'm ok with waiting until after 0.10 I guess. The bug has always been present. It's just been exacerbated now that software stacks are getting deeper and X11 support has been proliferated.

How about I remove the last commit and we merge the first 2 commits? It's a useful stopgap, although it doesn't solve every situation.

How about I remove the last commit and we merge the first 2 commits? It's a useful stopgap, although it doesn't solve every situation.

agreed.

How about I remove the last commit and we merge the first 2 commits? It's a useful stopgap, although it doesn't solve every situation.

👍

As for which we go with (universal variants vs. variant forwarding) there are enough open issues discussing the pros and cons of both already.

The two concepts don't cover the same terrain. Having something "universal" means one single object to represent a property in the entire DAG. It could be useful for things like shared and static (even though you will lose the opportunity to have them mixed) or other properties that should be uniform across any given build.

Variant forwarding will help in cases where you need to make a decision based on somebody else. The most relevant example is openmp: if multiple packages in a DAG can be OpenMP multi-threaded what you usually want is to activate only one of those and avoid multiple nested regions.

I'm not in deep enough to have a useful opinion on the details, but will contribute the datapoint that as an end user I'm comfortable with the plan and priorities that @tgamblin described up above (interim fixes and fixing the concretizer the right way in early January, if I understand).

While having to do things like [email protected] ^pango+X is magical and a bit annoying, there aren't that many new users arriving on a daily basis so just helping them (us...) over the hump as needed and fixing the problem well in a timely manner seems perfectly reasonable.

Between now and then, "How much can go wrong?"

BTW... the universal variants I've suggested would have the property I'm
arguing against here: If you turn on universal variant mpi, then it will
be enabled for EVERY package in the DAG that uses it.

mpi as a universal variant is a wrong example: what if I always use serial arpack-ng or serial hdf5 as part of a software, regardless of whether this software was compiled with +mpi or ~mpi?

But it is a change in the way we've interpreted variants up till now.

Sure, but this is a matter of convention and interpretation. For the most part, we try to model variants this way, but I don't think that even modeling all variants as +feature results in what you're asking for. Consider @alalazo's OpenMP case. Some packages can build with OpenMP. That is a feature. You may need A+openmp -> B~openmp to enforce mutual exclusion and to avoid nested OpenMP regions, because OpenMP in B actually conflicts with OpenMP in A.

In the MPI case... why should A care whether or not B links to MPI behind the scenes?

In most cases, if B -> mpi and A -> B but A !-> mpi , A wasn't intended to be run with mpirun. If A isn't run with mpirun, and some routine from B calls something that calls an MPI routine, things break because the code isn't run in parallel. You'll likely just get an error that some MPI routine was called before MPI_Init(). I believe this is the case with HDF5. You can have an MPI program that links either the serial or parallel HDF5 library but I do not believe you can link a serial program to hdf5+mpi and expect it to run correctly. They may have fixed this in more recent HDF5 versions, but this is always the annoyance with HDF5 libs. So depending on hdf5~mpi is basically depending on serial HDF5.

So basically I want a logically complete dependency system, because that lets me express cases like these when I need to. Figuring out what the best usage conventions are on top of that depends on what's elegant, what works, and what we can do with the current state of HPC software, which has all kinds of nasty linking issues.

And speaking of NP complete problems, references to this writeup about package managers and SAT came across the Hacker News radar recently.

@hartzell: indeed. You're the second person to send me that today. What we're talking about here is making Spack's concretizer solve basically that problem, but with variants, compilers, and optional dependencies within each build. I would say the current concretizer is somewhere between heuristics and a full backtracking solver.

The nice thing about Spack is we don't have to solve version SAT for everything that is installed at once, just within each build. Also Spack has some leeway that most PM's don't w.r.t. build dependencies (which can be RPATH'd to versions of libs separate from those in the DAG to be built).

@adamjstewart remove the last commit and we can finish with this one?

Git has a revert command that will apply a new commit reversing an existing
commit.

IMHO it is easier by doing git rebase -i HEAD~3 and then removing the last one, and git push. My perception is that @adamjstewart separated commits intentionally, so I think one should not squash them during the merge and thereby the history should be clean.

Ok, last commit removed. Should be good to go now.