gdal icon indicating copy to clipboard operation
gdal copied to clipboard

Make ODBC Install optional at compile time

Open gaige opened this issue 3 years ago • 2 comments

Noting up front that I'm happy to do the work, I'd just like some guidance for which of the options (or both, or neither) to pursue.

Expected behavior and actual behavior.

In versions prior to 3.4.x, setting the MDB_DRIVER_TEMPLATE and/or PGEO_DRIVER_TEMPLATE would facilitate opening using a specified Microsoft Access compatible driver under MacOS without installing that driver directly into odbcinst.ini. For our software package, we took advantage of this to allow the use of an embedded ODBC driver in the form of a .dylib.

As of 3.4.0, (except under Windows), CPLODBCDriverInstaller::InstallMdbToolsDriver is now called, which results in an attempt to install the driver in the odbcinst.ini file. On macOS, that's an infrequent, but possible use, and as such, taking advantage of using the full path of the driver in our MDB_DRIVER_TEMPLATE (or similar) allowed us to use our embedded driver without directly manipulating the odbcinst.ini , which meant that we could fall back to the user's installed drivers for other ODBC operations (for example, reading data to be merged from another driver).

Making things a bit more confusing for the users, the message:

ODBC: Unable to install MDB driver for ODBC, MDB access may not supported

is registered as an error, which means (regardless of the subsequent success in opening the file and reading the header during OGRODBCDataSource::Open, the call to GDALOpenEx will ultimately fail and close the file during the CPLGetLastErrorNo() check just before it returns.

Steps to reproduce the problem.

  1. Set MDB_DRIVER_TEMPLATE to a MDB-compatible driver so or .dylib file using a string similar to DRIVER=mydriver.so;DBQ=%s
  2. Call GDALOpenEx on a PGeo file twice.

The first call will fail (console should emit the error about not being able to install the ODBC driver) and the second call will succeed, since the installation process is protected by static std::once_flag oofDriverInstallAttempted

I see 2 opportunities here:

  • I recommend protecting the determining whether to make the install attempt not by WIN32, but by another define that explicitly enables or disables registration and making the default for that based on the existence of the WIN32 define. That would preserve existing behavior and would enable library users to determine whether this action is appropriate.
  • I would also recommend considering a warning for the installation failure instead of an error. The warning provides the user with the information that the attempt was made and failed, and for command-line users should (I believe) result in basically the same message as they would see with the error. However, it would not fail the open if the install fails, but the driver is successfully opened.

Operating system

macOS 11 and macOS 12

GDAL version and provenance

GDAL 3.5.0 built from source using the 3.5.0 tag

gaige avatar Jun 24 '22 10:06 gaige

CC @nyalldawson

rouault avatar Jun 24 '22 10:06 rouault

I'm on leave for the next two weeks, but will dive into this conversation when I'm back 👍

nyalldawson avatar Jun 24 '22 21:06 nyalldawson