Consider replacing src/ini.cpp with an external library

[clanmills]

This is a single file of 297 lines.

728 rmills@rmillsm1:~/gnu/github/exiv2/0.27-maintenance/conan-build $ wc ../src/ini*
     297    1097    8513 ../src/ini.cpp
729 rmills@rmillsm1:~/gnu/github/exiv2/0.27-maintenance/conan-build $ 

I found this somewhere on the internet and edited to put it into the Exiv2 namespace. I added this as a single file because I thought it would be simpler than adding a new library dependency.

Both Dan and Christoph disagree with my decision.

With conan, adding new dependent libraries is easier than it used to be.

I only use conan with Visual Studio. For other platforms (macOS, Linux, Unix, MinGW and Cygwin) I find it easier to use the platform package manager to install the dependencies (libz, expat and curl). These are all "major players". I don't even know if the ini parser can be installed by the platform package manager and/or if there is an conan recipe. So, removing this little file, could result in lots of work. For sure, I wouldn't like to reach the point that building Exiv2 demands conan because I've never got conan to work on Cygwin and/or MinGW. #1224.

[clanmills]

Issue #1515 concerns replacing the /xmpsdk code in Exiv2 with building and linking Adobe XMPsdk from source. Regrettably, #1515 has been modified to include a discussion concerning src/ini.cpp. I'm going to restore #1515 to the original focus.

I've thought of a good/simple way to remove class Exiv2::INIReader from libexiv2.

We should have a callback from the library to identify a lens. The exiv2 application can compile/link INIReader to search configfile. User code can use the call-back to implement other "unknown lens" strategies such as executing exiftool.

Here's a suggestion (not a proposal) for the call-back:

std::string myLensRecognitionCallback(Exiv2::Image::UniquePtr& image)
{
    std::string result ; // result.size() != 0 when recognised
    // code can use image->exifData() etc to interrogate (and modify) the metadata
    return result;
}
Exiv2::setLensRecognitionCallback(myLensCallback)

This is a substantial change to the Exiv2 API and should be targeted at the 'main' branch. We cannot remove the API class Exiv2::INIReader from 0.27 without breaking API compatibility.

A downsides of this design is that existing users of libexiv2 (such as darktable) will loose the configfile feature of the library as the onus will be on the host application to implement the configfile feature. We can address that by providing a default implementation of myLensRecognitionCallback() which uses class INIReader. To do that requires linking ini.cpp, however class INIReader can be hidden from the Exiv2 API. All exiv2 applications (exiv2, the samples, darktable and others) will continue to support configfile without change.

I do not understand the hostility expressed about src/ini.cpp. The code has been very robust and helped many users with lens recognition. The INIReader code is 297 lines and wrapped in the Exiv2 namespace.

510 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance/build $ finder "*.dylib" -type f | xargs nm -g | grep INI
0000000000042240 T __ZN5Exiv29INIReader10GetBooleanENSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEES7_b
0000000000041f50 T __ZN5Exiv29INIReader10GetIntegerENSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEES7_l
0000000000041c80 T __ZN5Exiv29INIReader10ParseErrorEv
00000000000419a0 T __ZN5Exiv29INIReader12ValueHandlerEPvPKcS3_S3_
0000000000041c90 T __ZN5Exiv29INIReader3GetENSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEES7_S7_
00000000000420b0 T __ZN5Exiv29INIReader7GetRealENSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEES7_d
0000000000041db0 T __ZN5Exiv29INIReader7MakeKeyENSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEES7_
0000000000041bd0 T __ZN5Exiv29INIReaderC1ERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEE
00000000000418f0 T __ZN5Exiv29INIReaderC2ERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEE
511 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance/build $

511 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance/build $ c++filt __ZN5Exiv29INIReaderC1ERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEE
Exiv2::INIReader::INIReader(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)
512 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance/build $ 

[kevinbackhouse]

I just spent a hour looking at this. On Ubuntu, it's very easy to install the library: sudo apt-get install libinih-dev. But it's going to take a cmake wizard to figure out how to link with it, because find_package( INIH REQUIRED ) doesn't work. We'd have to add a file like this one, which is unfortunately far beyond my cmake abilities.

[piponazo]

I just noticed that there seems to be a conan recipe for that library: https://conan.io/center/inih

I am happy to work on this when I find some spare time 😉