@spackbot fix style

@spackbot fix style

Let me see if I can fix that for you!

I was able to run spack style --fix for you!

==> Running style checks on spack
  selected: isort, mypy, black, flake8
==> Modified files
  lib/spack/spack/cmd/__init__.py
  lib/spack/spack/cmd/deactivate.py
  lib/spack/spack/cmd/extensions.py
  lib/spack/spack/compiler.py
  lib/spack/spack/provider_index.py
  lib/spack/spack/solver/asp.py
  lib/spack/spack/spec.py
  lib/spack/spack/test/cmd/common/arguments.py
  lib/spack/spack/test/concretize.py
  lib/spack/spack/test/spec_semantics.py
  lib/spack/spack/variant.py
  var/spack/repos/builtin.mock/packages/hypre/package.py
  var/spack/repos/builtin.mock/packages/openblas/package.py
==> Running isort checks
  isort checks were clean
==> Running mypy checks
Success: no issues found in 564 source files
  mypy checks were clean
==> Running black checks
reformatted lib/spack/spack/solver/asp.py
reformatted lib/spack/spack/spec.py
All done! ✨ 🍰 ✨
2 files reformatted, 11 files left unchanged.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> spack style checks were clean

I've updated the branch with style fixes.

I've cleaned up the commits here and squashed into the original implementation + @becker33's performance fix. With the fix, I see negligible overhead for this feature, other than that setup seems to take a little longer:

develop:

(spackbook):spack> spack solve --timers trilinos
Time:
    setup:         5.9787
    load:          0.0143
    ground:        1.8304
    solve:         3.0472
Total: 10.9299

This PR:

(spackbook):spack> spack solve --timers trilinos
Time:
    setup:         6.3022
    load:          0.0138
    ground:        1.9173
    solve:         3.0648
Total: 11.3539

Setup is going to get optimized away soon, so I'm fine with this as-is.

Leaving a few notes to anticipate comments we might get after this is merged:

Propagating non-default variants might change the virtual provider

This is due to the fact that we don't change/transform the weight of propagated variant. So if we propagate a non-default Spack might try to minimize nodes in a certain subDAG. For instance:

$ spack spec hdf5++shared

selects openmpi on linux, while:

$ spack spec hdf5~~shared

selects mpich. This is solvable by constraining the second spec.

Knowledge on the final DAG topology is needed to propagate the same variant

For instance we can currently do:

$ spack solve hdf5++shared ^zlib~~shared

but we can't:

$ spack solve hdf5++shared ^perl~~shared
==> Error: 'zlib' required multiple values for single-valued variant 'shared'
    Requested '~shared' and '+shared'

This might be fine on the command line, since the error is informative. If people start to use propagation in directives though this might lead to some confusion when things don't concretize anymore.

For the error message part:

$ spack solve hdf5 cflags=="-foptimize-loops" ^perl cflags=="-g"
==> Error: No valid compiler version found for 'hdf5'

while clearly in this case the issue is conflicting cflags.

RHEL8 issue here is fixed on develop; all PR-relevant tests are passing.

@alalazo i think a follow on PR that doesn’t count propagated variants as non-default would address the biggest issue you list here.

This PR (also with follow up PRs) slows down unit tests very significantly.

The slowest test on ae99829

86.64s call     lib/spack/spack/test/concretize_preferences.py::TestConcretizePreferences::test_preferred_compilers

now (on bc209c4) takes

182.03s call     lib/spack/spack/test/concretize_preferences.py::TestConcretizePreferences::test_preferred_compilers

This individual test now runs slower than all 3500 other tests combined when run with xdist, which is quite annoying for development (2 mins of waiting -> 3+ minutes)

If we can’t make a general fix for this, we can separate the propagation logic introduced here from concretize.lp and only include it if we see propagation used. Then we would at least only pay when using this feature.