Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exiv2 0.27 : new header exiv2/version.hpp have been introduce which make a puzzle with backward version #483

Closed
cgilles opened this issue Oct 14, 2018 · 2 comments

Comments

@cgilles
Copy link
Collaborator

cgilles commented Oct 14, 2018

Look in this file at line #75 :
https://github.com/KDE/digikam/blob/master/core/libs/dmetadata/metaengine_p.h#L75

metadaengine_p.h/cpp is the private implementation of the digiKam Qt Exiv2 wrapper. In this class, all that we need from Exiv2 is called. The rest of digiKam don't know Exiv2. This prevent to make Exiv2 public headers fully opaque from lead digiKam implementation. This improve the compatibility in code with previous Exiv2 versions at compilation time.

Other advantage, the C++ exception wrapper is located in the same class and not dispatched everywhere. This simplify the maintenance in time.

Lokk now the puzzle at line 75.

Before Exiv2 0.27, all library versions information was located in one file : exiv2/exv_conf.h

Now, these information are split in a new file exiv2/version.h

The break a little bit the compatibility with Exiv2 0.26 and older. Note that in all cases digiKam forace to use 0.26 as minimum Exiv2 version even if previous version can be used. This permit to reduce the crash report already know and fixed in 0.26. I must admit that digiKam was quickly instable with Exiv2 < 0.26.

So with the exiv2/version.hpp, we need to wrap with preprocessing rules this header to check if Exiv2 version > 0.26, else the compilation is broken.

Note the pre-processor rule, which is a copy that EXIV2_MAKE_VERSION macro defined in... version.hpp. so i cannot use this as well to wrap version.hpp (:=)))

Note also the macro that i define at line #85 :

https://github.com/KDE/digikam/blob/master/core/libs/dmetadata/metaengine_p.h#L85

This kind of macro must be defined in a Exiv2 header, not in digiKam. I cannot found an equivalent in 0.27. This macro simplify the branching of code with pre-processor rules to preserve compatibility with older version of the library...

For digiKam 6.0.0, we can live with this implementation, but it's not the best. This can be improved and simplified in the future...

Gilles Caulier

@clanmills
Copy link
Collaborator

clanmills commented Oct 14, 2018

This seems rather strange, Gilles. include/exiv2/version.hpp has been in the code-base for years.

The design is that all application code should #include <exiv2/exiv2.hpp> and nothing else. exiv2/exiv2.hpp will pull in the rest of the API including version.hpp. For example in samples/exifprint.hpp

// ***************************************************************** -*- C++ -*-
// exifprint.cpp
// Sample program to print the Exif metadata of an image

#include <exiv2/exiv2.hpp>

#include <iostream>
#include <iomanip>
#include <cassert>
...
    if ( _tstrcmp(file,_t("--version")) == 0 ) {
    	exv_grep_keys_t keys;
    	Exiv2::dumpLibraryInfo(std::cout,keys);
    	return 0;
    }
...

@cgilles
Copy link
Collaborator Author

cgilles commented Oct 14, 2018

You have right. 0.26 has well version.hpp... It's strange that i cannot find it by re-installing 0.26 after to uninstall 0.27.
In all cases the macro EXIV2_TEST_VERSION macro disappear with 0.27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants