New way to generate the include/exiv2/exv_conf.h file

[piponazo]

Hi,

I have been checking the way in which the exv_conf.h file was generated. There ware several things that I did in this branch:

• Before everything was in the file config/CMakeChecks.txt. All the CMake code should be in .cmake files, so I created a new file config/generateConfigFile.cmake where I moved all the relevant commands.
• There were several definitions that were generated in the CMake code but then they were not used at all in the project. I have removed them from the CMake code.
• Some of the definitions were not properly initialised. For example, some of those variables should be enabled for Linux systems, and they were not enabled. I had to touch slightly the c++ code to adapt the code to those CMake changes.
• Removed some global calls to INCLUDE_DIRECTORIES and use the better cmake approach: target_include_directories. In that way, we can control the visibility of the propagation of the flags.

Probably there are few other minor things here and there. Do not hesitate to ask if you have questions about the changes.

For the moment I only tried to compile the project on Linux. This should be tested on Mac and Windows before being merged.

[clanmills]

This is excellent work. Thank You very much. I've tested this on Mac/Linux/Cygwin/MSVC2005

Significant comments:
There's an error in src/CMakeLists.txt concerning local_time.c and getopt_win32.c
These files should be unconditionally compiled a linked into exiv2.exe

##
# modify source lists to suit environment
IF(NOT EXV_HAVE_TIMEGM )
    # SET( LIBEXIV2_SRC     ${LIBEXIV2_SRC} localtime.c    )
    # IF ( EXV_HAS_XMP_SDK_LIBS )
        SET( EXIV2_SRC    ${EXIV2_SRC}    localtime.c    )
    # ENDIF()
    SET( PATHTEST_SRC     ${PATHTEST_SRC} localtime.c    )
ENDIF()

IF( MSVC )
    # IF ( EXV_HAS_XMP_SDK_LIBS )
        SET( EXIV2_SRC    ${EXIV2_SRC}    getopt_win32.c )
    # ENDIF()
    # SET( LIBEXIV2_SRC     ${LIBEXIV2_SRC} getopt_win32.c )
ENDIF( MSVC )

Cosmetic Comments:

1. Should we change CMake_msvc.txt to CMake_msvc.cmake or configMSVC.cmake (or something like that)
2. Can you remove any copyright with my name (and leave the others)
   I do not want copyright for anything. I must have followed the lead set by others (Gilles et al).
   Leave other folk's copyright. Remove mine.

And after you've submitted:
Can you revise your patch and I will merge it and I'll test again.

I made a change in July to src/CMakeLists.txt for issue #11 which I believe has been lost. The change was Changes = 1c721b1 After I have merged your next pull request, I'll manually (or cherry-pick) the changes which make up 1c721b1. I'll update #11 to mention this.

I've very pleased indeed by this work. Thank you very much for contributing. I've added you as a contributor and I believe that will give you write privileges to Exiv2/exiv2.git.

[clanmills]

There's another feature I forgot to request. Can you add the define -DEXV_BUILD_CMAKE for the compiler (or put it into exv_conf.h if you prefer). I'll add code in version.cpp so that the command $ exiv2 --verbose --version prints build=cmake when set. I'll also update config/configure.ac to print build=autotools.

[piponazo]

Hi @clanmills, thanks for your kind words. I have several comments:

1. I have fixed the significant comment that you made.
2. I would prefer to stop doing other significant changes in this PR (the ones related with the CMake_msvc.txt file, and the removal of the copyright stuff with your name). If I continue doing changes in this branch it will be very difficult to review them. I prefer to do those things in different PRs, since I will continue working in CMake improvements. (This is just a first step 😄 ).
3. I also would prefer to skip to add the EXV_BUILD_CMAKE definition in this PR. I can do it in other PR, and also take care to modify the file version.cpp to print such information (same reasons than before).
4. I also tried by myself this branch on Windows with MSVC 2017 and I noticed two things. Firstly I could remove the file exv_msvc.h that was basically a copy of the file exv_conf.h that we are touching in this PR. I tried to compile the project directly with MSVC2017 and with the Ninja generator, and it works properly. Secondly, I noticed that MSVC 2017 is not mentioned in any of the documentation files. I will try to mention it in the future as I keep working on the CMake files.

I have been checking what happened with the changes you mentioned related with the issue #11 . Probably those changes were erased when you rewrote the master history few days ago.

Let me know if I have missed something.

[piponazo]

I am wondering if the changes that I did in the file include/exiv2/config.h (commit ea0c489) would affect the configuration and compilation of the project with the autotools. Let me know if there is any issue with that, I would think in an alternative solution.

Actually I do not know if in this project it is considered the usage of autotools on Windows.

[clanmills]

Thanks very much. I'm happy if you want defer the cosmetic matters. I've merger your pull request and will test it. Assuming it passes (which I expect of course), I'll update 'master' for #11, remove my personal copyright claims and retest.

When I've dealt with autotools/C++11/ADOBE_XMPSDK, I hope you'll make the CMake changes to support those features. That will be a good time to look at the cosmetic and file name matters.

We must retain exv_msvc.h because that file has to be manually edited to fine tune Visual Studio builds (msvc = Microsoft Visual C++). It is read from include/exiv2/config.h

autotools and CMake can be used on Windows to build on Cygwin. autotools are not used with Visual Studio.

CMake can be used with Visual Studio. I tested CMake/Visual Studio and discovered the Significant Comment concerning local_time.c and getopt_win32.c.

The architecture involving exv_conf.h (and exv_msvc.h) is a mess. Let's have an off-line discussion about this.

[clanmills]

I've tested this, and integrated the "lost overboard" changes from July and retested. Everything seems fine to me.

I made a change to config/config.h.cmake because basicio.cpp didn't compile. However on retesting, I don't think that's needed. My suspicion is the cmake/regenerator didn't run when I did a make on Cygwin (probably confused by CMakeCache). You can throw away my code as EXV_HAVE_UNISTD is being set correctly on Cygwin.

I'm off gardening at my son's tomorrow (it's going to be sunny). I'll do email/slack from my iPod. Back at the computer tomorrow evening when I'll deal with the outstanding pull request and two patches on Redmine.