@becker33 I think all your requests are addressed now. I also cleaned up some of the comments in the tests

It may be arguable whether errmsg_requirements_external_mismatch is sufficiently-improved here: it used to be

    1. cannot satisfy a requirement for package 't1'.

and now it is

     1. Cannot build t1, since it is configured `buildable:false` and no externals satisfy the request

better, but perhaps still not good.

I noticed a general slowdown of concretization yesterday, with respect to the last time I measured on my machine. I performed a test on a reduced set of specs (just because it takes a lot of time to do the full test).

Here are the results of this PR compared to the previous commit on develop (ecde809 vs 3949a27)

benchmark.pr.csv
benchmark.develop.csv
radiuss-reduced.txt

It seems this PR caused a general slowdown of concretization. For spack solve hdf5 --timers it's ~20% on my machine. I'll check if that can be improved with some small fix, but let me know if you can reproduce, or not.

Benchmarking is reporting a regression for me comparing this with prior commit - the following is just the solve times (as the total times are much closer):

it's ~20% on my machine.

Is this average output as part of solve-benchmark?
I see a 14% increase in total solve time across all benchmarked specs.

There is likewise a regression when comparing ecde809 with 4170858

After 4170858, the only logic that isn't changing weights is

{ choose_version(node(ID, Package), Version) : pkg_fact(Package, version_declared(Version)) }
 :- attr("node", node(ID, Package)).

attr("version", node(ID, Package), Version) :- choose_version(node(ID, Package), Version).

If I revert that part on top of 4170858, then the benchparks show no significant timing change. This part is useful for error messages, but it only needs to exist when mixing in error_messages.lp, so optimistically would only increase concretization times for failed concretizations; I had trouble with that in other parts of this PR, but it seems like a worthy thing to chase to improve the timing.

EDIT: my initial attempt at this in #51576 does not work.

Is this average output as part of solve-benchmark?

No, it's the result of spack solve hdf5 --timers. I used that time to bisect the commit on my machine.

the only logic that isn't changing weights is, [ ... ]

Yes, it's the two extra rules that increased the time. Once v1.1 is released, I'll try to check if profiling with latest development version of clingo helps identifying better what to change. Unfortunately, using development version of clingo is also prone to trip into bugs (see potassco/clingo#583)

I've found that this PR makes the concretizer hang when given a deprecated spec. For example,

$ git checkout develop   # currently 443e1e3e36
$ spack spec [email protected]
<hangs>

$ git checkout 3949a273b19ee9aaf3e0e1637dc09c958d069e9c^
$ spack spec [email protected]
==> Error: failed to concretize `[email protected]` for the following reasons:
     1. Cannot satisfy '[email protected]'
     2. Cannot satisfy '[email protected]'
        required because [email protected] requested explicitly

If I manually update the python package to remove the deprecation, I get

$ git checkout develop
$ spack edit python

-    with default_args(deprecated=True):
+    with default_args(deprecated=False):

$ spack spec [email protected]
 -   [email protected]+bz2+crypt+ctypes+dbm~debug+libxml2+lzma~optimizations+pic+pyexpat+pythoncmd+readline+shared+sqlite3+ssl~tkinter+uuid+zlib build_system=generic patches:=0d98e93,4c24573,ebdca64,f2fd060 platform=linux os=linuxmint21 target=zen3 %c,[email protected]
...

I have confirmed this is not related to #51599 as I'm able to concretize openmpi without issue. It might be similar to #51279, but that issue was filed in early September before this PR was merged.

Update:
I found that this works, as well.

$ git checkout develop
$ spack spec --deprecated [email protected]
==> Warning: using "[email protected]" which is a deprecated version
 -   [email protected]+bz2+crypt+ctypes+dbm~debug+libxml2+lzma~optimizations+pic+pyexpat+pythoncmd+readline+shared+sqlite3+ssl~tkinter+uuid+zlib build_system=generic patches:=0d98e93,4c24573,ebdca64,f2fd060 platform=linux os=linuxmint21 target=zen3 %c,[email protected]
...

@hainest Can you check #51612 ? That should solve the performance regressions of this PR, and more. With that I get:

$ time spack solve [email protected]
==> Error: failed to concretize `[email protected]` for the following reasons:
     1. Cannot satisfy '[email protected]' 1(3.14.0)
     2. Cannot satisfy '[email protected]' 4(3.14.0)
        required because [email protected] requested explicitly 

real	0m21,530s
user	0m20,953s
sys	0m0,521s

Without instead:

$ time spack solve [email protected]
^C
==> Error: Keyboard interrupt.
^C

real	1m30,229s
user	1m29,591s
sys	0m0,519s

@alalazo I've not checked #51612, but #51611 fixes the issue with the concretizer hanging when using a deprecated spec.