fix: respect PYTHON_VERSION if set in classic mode

[henryiii]

If PYBIND11_PYTHON_VERSION is not set, but PYTHON_VERSION is, on the first run, use that for the hint. We could add a warning or error instead - but the current behavior is a bit odd and unhelpful.

[bstaletic]

Why don't we also show a warning in the branch where we allow the old variable?

[henryiii]

You mean a warning if PYBIND11_PYTHON_VERSION is set instead of PYTHON_VERSION?

[bstaletic]

The other way around. The "legacy" way of doing things is using PYTHON_VERSION. Right? In that case showing a warning seems like a good idea.

[henryiii]

That's what I suggest at the top ("We could add a warning or error instead"). The old way of doing things is with PYBIND11_PYTHON_VERSION. However, since PYTHON_VERSION is the output variable, it is a natural thing to set PYTHON_VERSION instead when looking for Python; currently this messes up the search. I can add a warning but still do the "right" thing, then?

With FindPython, there is no version-based search variable that is available like this (though I'm discussing it in an issue). PYBIND11_* variables do not affect FindPython. The recommendation is to use:

find_package(Python EXACT 3.7 ...)

(But I'm arguing this can't be done from the command line, and is rather a package requirement).

[bstaletic]

The old way of doing things is with PYBIND11_PYTHON_VERSION. However, since PYTHON_VERSION is the output variable, it is a natural thing to set PYTHON_VERSION

Oh, thanks for clarifying!

I can add a warning but still do the "right" thing, then?

Right. Any reason not to?

[bstaletic]

With the added warning, I'm fine with this. Thanks @henryiii

[YannickJadoul]

Looks good, yes. More of a fix/convenience, indeed, so do merge when you're happy with it, AFAIC!