Patch #856

CMAke: building tests and refactoring on msvc

Added by Daniel Kaneider over 4 years ago. Updated almost 4 years ago.

Status:ClosedStart date:07 Oct 2012
Priority:NormalDue date:
Assignee:Robin Mills% Done:

90%

Category:build
Target version:0.24

Description

Hi Robin,

I did some other changes to the scripts.

I added the include dir for the tests. In this way the directory can be found. Since this line was absent, and the testsuites built and ran under Unix, this could just mean that the installed exiv2 library is used for the tests. I have seen Gilles' comment in TODO-CMAKE stating, that "They should use the installed version of libexiv2, not some temporary version in the src or build tree.".
But IMHO they should use the includes from the current sources, and not from the installed version. How else are you supposed to test a new version of exiv with a new header file?

I have seen a description in README-CMAKE how to build exiv2 out of source. Some remarks for this:

  • Out of source doesn't imply to create a directory 'build' INSIDE the sources. What I would consider OOS is to have
    exiv2
      src
      ...
    exiv2.build
    

    The build directory could be anywhere. There must not be any limitations for a working OOS build. From my tests OOS builds don't work at the moment. This is mainly due to some 'SolutionDir/../' uses in various points.
  • Moreover there was a usage of a OUT_OF_SOURCE variable inside the CMake scripts. Such variables are also very evil.
What I did in patch is the following:
  • Fixed the tests under windows. If it causes problems on Unix platforms, move the include_dirs to the IF-MSVC.
  • Removed uses of OUT_OF_SOURCE, using CMAKE_SOURCE_DIR. There is just one point left, which could be converted to a simple CMAKE Copy without having to create a pre-build event. Similarly SolutionDir should be replaced by CMAKE_BINARY_DIR.
  • Fixed the post-build event, copying the depended .dll files. I simply missed this in my previous patch.

I hope this helped a bit.

Best,
Daniel

exiv2Tests.patch Magnifier (3.64 KB) Daniel Kaneider, 07 Oct 2012 04:36

Associated revisions

Revision 2895
Added by Robin Mills over 4 years ago

Fix. Issue: #856. Refactoring msvc/CMake code. Thanks to Daniel for this patch.

History

#1 Updated by Robin Mills over 4 years ago

Daniel.

You said "I hope this helped a bit", and my response is: "You've helped a lot". It's very good to get some feedback and even better to get a patch, another pair of hands and most importantly - another brain working on the job. Thanks very much indeed.

I've submitted your patch because it looks good to me. I've built on the Mac and the test suite passes. I'm out of town for a couple of days. I'll build everything later in the week and see if that reveals anything, review/update README-CMAKE and TODO-CMAKE.

About the tests:
Shawn and I are considering generating output all the executables in a single 'bin' directory and also adding ctest instructions to the CMakeLists.txt files (issue #850). I agree with you (apologies to Gilles), and we should build/link/test the fresh build to the local sources. We can install/test to /usr/....exiv2 tree. Shawn and I are working on this for #850.

Thanks very much for contributing to exiv2. This is very much appreciated.

#2 Updated by Daniel Kaneider over 4 years ago

Thanks Robin for your fast response. CMake is pretty much the way to go!

#3 Updated by Robin Mills over 4 years ago

Daniel

Yes. Andreas and I agree that our plan is to only support CMake. Gilles contributed most of the existing CMake scripts and I agreed in June to take on CMake/msvc. One day we'll be there.

For sure CMake works fine on Cygwin/Linux/Mac at generating Makefiles. I've played with Eclipse (on Windows and Linux) and Xcode.

One day our CMake support will be strong enough to discontinue support for config/msvc/msvc64. Your contribution is helping us reach that goal. Thanks very much.

You are of course welcome to join the team.

#4 Updated by Robin Mills over 4 years ago

  • Category set to build
  • Priority changed from High to Normal

#5 Updated by Robin Mills over 4 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 90

#6 Updated by Robin Mills almost 4 years ago

  • Status changed from Resolved to Closed

Fixed in 0.24.

Also available in: Atom PDF

Redmine Appliance - Powered by TurnKey Linux