elastic / elasticsearch-sql-odbc

ODBC driver for Elasticsearch SQL
Other
14 stars 30 forks source link

Installation Prerequisite Check on Windows 7 #251

Open Leaf-Lin opened 4 years ago

Leaf-Lin commented 4 years ago

Failed Installations

Environment

Windows 7 32 bit

Expected Behaviour

Failed with unsupported OS

Actual Behaviour

Installer failed with

This installer requires the Visual 2017 C++ Redistributable (x86) 

from https://github.com/elastic/elasticsearch-sql-odbc/blob/6.8/installer/src/Installer/Program.cs#L154

It should have failed in https://github.com/elastic/elasticsearch-sql-odbc/blob/6.8/installer/src/Installer/Program.cs#L149  but somehow windows 7 still passed?

Steps to reproduce the behaviour

Install ODBC driver 6.8.0 on a Windows 7

Leaf-Lin commented 4 years ago

Related reporting on windows 2012 R2 also passed the prerequisite check, but should have failed https://github.com/elastic/elasticsearch-sql-odbc/issues/146

droberts195 commented 4 years ago

I think the problem is this: https://support.microsoft.com/en-gb/help/3202260/versionnt-value-for-windows-10-and-windows-server-2016

Microsoft doesn't like it when people refuse to install on particular versions of Windows simply by checking the version rather than required capabilities. So Windows 10 reports itself to be Windows 8.1 in most APIs. (There are sneaky ways to find out like checking the version of kernel32.dll instead.)

Then this combines with a silly error in the formula Installed OR (NOT ((VersionNT64 = 1000 AND MsiNTProductType = 1) OR (VersionNT64 = 1000 AND MsiNTProductType <> 1))) where the NOT doesn't make sense. The formula is saying Windows 10 is not acceptable. All other versions are OK. But, per above, Windows 10 is pretending to be Windows 8.1 so it passes too.

droberts195 commented 4 years ago

A quick fix that would rule out the very oldest versions of Windows would be Installed OR VersionNT64 >= 603. That would allow installation on Windows 8.1, Windows Server 2012r2 (both Windows NT 6.3 under the hood), plus the more recent versions that pretend to be those versions. The documented supported versions could stay the same.

bpintea commented 4 years ago

Thanks @Leaf-Lin for the report and @droberts195 for the quick check. The installer didn't get a lot of attention past the release and there still are a few tasks pending around testing (see #37, Test4 might have caught this issue). It might get a redo after other stack components (provided @ygel will get to it).

However, besides the Windows version itself, we'd probably need to check the bitness too, since (according to some reports) the registry might not be correctly written on a 32 bit OS, even if the installation succeeds (passing the incorrect check) and the driver might still not work.

Note: the 32-bit installer is meant for 32-bit apps on a 64-bit OS.