microsoft / ADBench

Benchmarking various AD tools.
MIT License
107 stars 40 forks source link

Rely on environment variables to find Python #196

Closed cgravill closed 4 years ago

cgravill commented 4 years ago

When CMake is locating Python on Windows https://cmake.org/cmake/help/latest/module/FindPython3.html it defaults to searching the registry first. This is awkward behaviour on e.g. GitHub Actions where it has a later version of Python available but wouldn't be used if calling e.g. python -V.

I think it's closer to expectations to respect the environment variables instead where we want to use a precise version for benchmark comparability. That matches the current documentation which instructs put the binaries on the PATH:

https://github.com/microsoft/ADBench/blob/191c5244e7a815984d302ccbc48372137235a9dd/docs/BuildAndTest.md#prerequisites

tomjaguarpaw commented 4 years ago

Does this mean that it ignores the registry entirely?

cgravill commented 4 years ago

With this proposed new setting yes. It completely ignores the registry.

A more friendly option would be to use the CMake LAST setting which first uses environment variables and would then fall back on the registry if there's nothing on PATH.

awf commented 4 years ago

Yep, great fix. Ouch!