@eugeneswalker

This is great, this means we have a strong guarantee that no deprecated packages will ever be installed, unless it is unavoidable by the constraints in the packages (e.g. ruby@:2.3)

Edit: i think this is even nicer; spack solve ruby@:2.3 toggles ~openssl to avoid installing a deprecated version :D you'd have to specify spack solve ruby@:2.3 +openssl to get a deprecated openssl.

@haampie Yeah, the first priority of the solver is to avoid using deprecated versions. It must be obliged to do that to put one into the DAG 😆

@tgamblin I added a warning that currently looks like:

Let me know if you want it formatted differently

Do you think we should actually error out if only a deprecated version is possible? And have them run with --allow-deprecated or set a config option to allow it? It seems like we should be louder about this by default.

You could imagine someone using this with a buildcache and —cache-only — it would make you add some config to your env if the only available versions end up being deprecated.

Do you think we should actually error out if only a deprecated version is possible? And have them run with --allow-deprecated or set a config option to allow it?

For security related deprecations like openssl I would say definitely. Not sure about other software where versions are deprecated because of obsolete build systems or lack of some features.

What if we extend packages.yaml to have this in our defaults:

packages:
  all:
    allow_deprecated_versions: False

and allow people to either override this behavior or selectively opt-out with:

packages:
  glib:
    allow_deprecated_version: True

?

Or we could distinguish between 'vulnerable' and 'deprecated'? In case of openssl they deprecated 1.0 after the abi breaking 1.1 release, but only some and not all of 1.0 and 1.1 were known to be vulnerable.

I like @haampie's suggestion but what if we just add a reason for deprecation?

version(..., deprecated="vuln")
version(..., deprecated="bug")
version(..., deprecated=True)

We could then decide on the solver side how to handle the different categories.

Would this logic (in pseudo-code) be good:

if reason is True or reason == 'vuln':
    tty.die(msg)
tty.warn(msg)

?

With the last commit Spack will error out if deprecated == "True" or deprecated == "vuln":

and emit a warning for other reasons. No configuration handles at the moment.

@tgamblin This is ready for another review Pinging also @adamjstewart and @sethrj since this PR extends what done in #20767

This is a nice new capability, but the deprecation strings probably need further thought, since the two main reasons ('bug' and 'vuln') are very open-ended and often overlapping.

Fair point, let me know if you have suggestions. I interpreted vuln as denoting software with vulnerabilities e.g. something in the CVE list, and bug as some bug not related to security e.g. some typo leading to numeric errors etc. but I agree this is not clear from docs.

I personally don't love this solution. As mentioned in https://spack.readthedocs.io/en/latest/packaging_guide.html#deprecating-old-versions, there are many different reasons why a version could be deprecated. I don't see any point in enumerating all of those reasons. Instead, I would keep deprecated as a Boolean and avoid it regardless of the reason for deprecation. That will keep the API simple and only involve minor changes to the optimization logic.

I tend to agree with @adamjstewart because of the potential granularity issues with descriptions. It might be helpful to look at a couple of example cases and try to classify how/why they'd be "deprecated". For example, OpenSSL regularly fixes bugs in older versions, but the bugs are various levels of severity. Similarly, its "vulnerabilities" are classified into levels of severity as well -- would you mark an older version with a significant bug and a minor vulnerability as deprecated='vuln' or 'bug'?

A simple boolean avoids having to ask those kind of difficult questions. A version should be non-deprecated if and only if it's the latest patch of a still-supported branch.

Examples:

• HDF5 has version branches for 1.12, 1.10, 1.8 ; everything but the latest versions of those should be deprecated
• OpenMPI lists 4.1 and 4.0 as "still supported", older versions are "retired" (last patch 18 Mar 2020) and should be deprecated.

However there might be room for a middle ground:

• Qt has "official releases" for 5.12, 5.15, 6.0, and 6.1. Some apps require older Qt5 versions though and it would be useful to have e.g. a flag indicating that "version 5.9.234 is the last supported one so is less bad than choosing 5.10.123 which was actually released a year earlier". And even though Qt4 is ridiculously obsolete, we've been helping it limp along with patches because numerous software still requires it.
• Likewise there's going to be a "long tail" for apps that haven't converted over from python 2, so it might be useful to mark the final version 2.7.18 as "less deprecated" than 3.1.5 .

...but again there we start worrying about granularity in the deprecation scale.

Ultimately I think this is like trying to give something a 5-star rating vs thumbs up/down. Fewer options tends toward less subjectivity in how a version will be graded.

Thinking about it a bit more, it seems to me that a concretization error for a deprecated package may be a bit too pedantic. (Maybe someone needs an old version of Ruby for some reason?) As long as it's explicitly confirmed by the user (which is the default already) it should be OK? In automated builds it'll fail unless you add the --deprecated flag (which nobody does by default...). And then distinguishing between different forms of deprecation might be unnecessary.

Oh I missed that the default is now to raise an error for deprecated versions. It seems to me that the config option should be a 'silent/verbose/error' choice when concretization results in deprecated packages. And that perhaps deprecated versions whose source is completely unavailable (heaven forfend) do need a special treatment that errors out on concretization rather than at fetch time... but then again there are the cases of externally installed packages as well as alternative source caches... so maybe fetch-time is the right way to handle that.

I'm personally fine with any solution be it minimal or with more fine-grained control on what deprecation means. The first commit in this PR is sufficient for clingo to avoid using as much as possible deprecated versions (but won't prevent using them if that's the only choice). The following commits build on the first to give some handles to customize behavior (so user can decide to error on certain types of deprecation and warn on others). I'm also fine splitting this into two PRs if merging the first commit as a standalone is considered an option. Let me know how to proceed with that.

I don't see any point in enumerating all of those reasons. Instead, I would keep deprecated as a Boolean and avoid it regardless of the reason for deprecation.

I think the definition of "avoid" enters this PR. Would that mean "avoid if not absolutely necessary" or "error if I need to use a deprecated version"? What I can do as a middle ground is to move to concretization the behavior that is now in the fetcher:

1. If deprecated: false Spack would error on concretization
2. If deprecated: true Spack would just warn that you're using a deprecated version and proceed

and leave deprecated= as just a boolean. Would that be a sensible behavior for the use cases we are interested in?

I think it would be a good idea to split this into two separate PRs, it sounds like the first commit is non-controversial.

As for whether it should error or warn, I don't have a strong preference. If the only possible way to install a particular configuration is to use a deprecated version, I think we should allow that.

Went back to a minimal version that tries to avoid deprecated versions and warn users if they end up in the spec.

@alalazo looks like this is now conflicted.

@tgamblin Already split, conflict resolved

Thanks! :)