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

Fix c++11 0.27 #1194

Merged
merged 19 commits into from
May 2, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ project(exiv2 # use TWEAK to categorize the build
# 0.27.3.19 = RC1 Not for release
LANGUAGES CXX C
)

include(cmake/mainSetup.cmake REQUIRED)

# options and their default values
Expand All @@ -29,8 +28,9 @@ option( EXIV2_ENABLE_SSH "USE Libssh for SshIo (WEBREADY)"

option( EXIV2_BUILD_SAMPLES "Build sample applications" ON )
option( EXIV2_BUILD_EXIV2_COMMAND "Build exiv2 command-line executable" ON )
option( EXIV2_BUILD_UNIT_TESTS "Build unit tests" OFF )
option( EXIV2_BUILD_UNIT_TESTS "Build unit tests" ON )
option( EXIV2_BUILD_DOC "Add 'doc' target to generate documentation" OFF )
option( EXIV2_BUILD_USE_C++11 "Use the C++11 compiler" OFF )

# Only intended to be used by Exiv2 developers/contributors
option( EXIV2_TEAM_EXTRA_WARNINGS "Add more sanity checks using compiler flags" OFF )
Expand Down
5 changes: 3 additions & 2 deletions README-SAMPLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ $
#### geotag

```
Usage: geotag {-help|-version|-dst|-dryrun|-ascii|-verbose|-adjust value|-tz value|-delta value}+ path+
Usage: geotag {-help|-version|-dst|-dryrun|-ascii|-remove|-verbose|-adjust value|-tz value|-delta value}+ path+
```

Geotag reads one or more GPX files and adds GPS Tages to images. _Code: [geotag.cpp](samples/geotag.cpp)_
Expand All @@ -208,6 +208,7 @@ If the path is a directory, geotag will read all the files in the directory. It
| -dst | Apply 1 hour adjustment for daylight saving time. |
| -dryrun | Read arguments and print report. Does not modify images. |
| -verbose | Report progress. |
| -remove | Removes GPS tags from images. |
| -adjust value | Add/subtract time from image data. |
| -tz value | Specify time zone. For example PST = -8:00 |
| -delta value | Correction between Image DataTime and GPS time. |
Expand Down Expand Up @@ -643,4 +644,4 @@ Read an XMP packet from a file, parse and re-serialize it.

Robin Mills<br>
[email protected]<br>
Revised: 2019-06-20
Revised: 2020-05-01
13 changes: 8 additions & 5 deletions cmake/compilerFlags.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# These flags applies to exiv2lib, the applications, and to the xmp code

if ( EXIV2_BUILD_USE_C++11 )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we wound need to following. Replace these lines by:

if ( EXIV2_BUILD_USE_C++11 ) 
  set(CMAKE_CXX_STANDARD 11)
else()
  set(CMAKE_CXX_STANDARD 98)
endif()

set(CMAKE_CXX_STANDARD_REQUIRED ON)	
set(CMAKE_CXX_EXTENSIONS ON)

And more important is to remove all the usages of:

target_compile_features(XXX PRIVATE cxx_std_98)

set(CMAKE_CXX_STANDARD 11)
endif()

if ( MINGW OR UNIX OR MSYS ) # MINGW, Linux, APPLE, CYGWIN
if (${CMAKE_CXX_COMPILER_ID} STREQUAL GNU)
set(COMPILER_IS_GCC ON)
Expand All @@ -22,10 +26,6 @@ if ( MINGW OR UNIX OR MSYS ) # MINGW, Linux, APPLE, CYGWIN


if (COMPILER_IS_GCC OR COMPILER_IS_CLANG)
# On Solaris target_compile_features(${application} PRIVATE cxx_std_98) fails to set this flag
if ( CMAKE_HOST_SOLARIS )
add_compile_options(-std=gnu++98)
endif()

# This fails under Fedora, MinGW GCC 8.3.0 and CYGWIN/MSYS 9.3.0
if (NOT (MINGW OR CMAKE_HOST_SOLARIS OR CYGWIN OR MSYS) )
Expand Down Expand Up @@ -58,7 +58,6 @@ if ( MINGW OR UNIX OR MSYS ) # MINGW, Linux, APPLE, CYGWIN
# This seems to be causing issues in the Fedora_MinGW GitLab job
#add_compile_options(-fasynchronous-unwind-tables)


if ( EXIV2_TEAM_USE_SANITIZERS )
# ASAN is available in gcc from 4.8 and UBSAN from 4.9
# ASAN is available in clang from 3.1 and UBSAN from 3.3
Expand Down Expand Up @@ -90,6 +89,10 @@ if ( MINGW OR UNIX OR MSYS ) # MINGW, Linux, APPLE, CYGWIN
endif()
endif()
endif()
if ( EXIV2_BUILD_USE_C++11 )
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-deprecated")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, replace the usage of CMAKE_CXX_FLAGS by the more appropriate usage of add_compile_options:

  add_compile_options(-Wno-deprecated)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed that 100%. I don't mess with the compile_options for -Wno-deprecated The user can pass that from the command when required. And that's what I've documented in README.md.

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-deprecated")
endif()
endif ()

# http://stackoverflow.com/questions/10113017/setting-the-msvc-runtime-in-cmake
Expand Down
3 changes: 3 additions & 0 deletions cmake/printSummary.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ if ( EXIV2_ENABLE_EXTERNAL_XMP )
else()
OptionOutput( "XMP metadata support: " EXIV2_ENABLE_XMP )
endif()
if ( EXIV2_BUILD_USE_C++11 )
OptionOutput("Use C++11: " EXIV2_BUILD_USE_C++11 )
endif()
OptionOutput( "Native language support: " EXIV2_ENABLE_NLS )
OptionOutput( "Conversion of Windows XP tags: " EXIV2_ENABLE_PRINTUCS2 )
OptionOutput( "Nikon lens database: " EXIV2_ENABLE_LENSDATA )
Expand Down
10 changes: 10 additions & 0 deletions include/exiv2/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,15 @@ typedef int pid_t;
#endif
//////////////////////////////////////

// https://softwareengineering.stackexchange.com/questions/291141/how-to-handle-design-changes-for-auto-ptr-deprecation-in-c11
#if __cplusplus >= 201103L
#include <memory>
#include <sys/types.h>
#include <unistd.h>
template <typename T>
using auto_ptr = std::unique_ptr<T>;
#else
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a clever trick, I did not think about that.

I think you can remove lines 103 & 104 from here. We do not need the using instruction when not using C++11.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember what using does. I got the code from StackOverflow (in the comment).

using std::auto_ptr;
#endif

#endif // _CONFIG_H_
1 change: 0 additions & 1 deletion samples/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ list(APPEND APPLICATIONS remotetest)
# ******************************************************************************
foreach(application ${APPLICATIONS})
target_link_libraries(${application} PRIVATE exiv2lib)
target_compile_features(${application} PRIVATE cxx_std_98)
if( EXIV2_ENABLE_PNG )
target_link_libraries( ${application} PRIVATE ${ZLIB_LIBRARIES} )
endif()
Expand Down
36 changes: 31 additions & 5 deletions samples/geotag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class Options {
bool dst;
bool dryrun;
bool ascii;
bool remove;

Options()
{
Expand All @@ -96,6 +97,7 @@ class Options {
dst = false;
dryrun = false;
ascii = false;
remove = false;
}

virtual ~Options() {} ;
Expand All @@ -113,6 +115,7 @@ enum // keyword indices
, kwDST
, kwDRYRUN
, kwASCII
, kwREMOVE
, kwVERBOSE
, kwADJUST
, kwTZ
Expand Down Expand Up @@ -790,6 +793,7 @@ int main(int argc,const char* argv[])
keywords[kwADJUST ] = "adjust";
keywords[kwTZ ] = "tz";
keywords[kwDELTA ] = "delta";
keywords[kwREMOVE ] = "remove";

map<std::string,string> shorts;
shorts["-?"] = "-help";
Expand All @@ -803,6 +807,7 @@ int main(int argc,const char* argv[])
shorts["-s"] = "-delta";
shorts["-X"] = "-dryrun";
shorts["-a"] = "-ascii";
shorts["-r"] = "-remove";

Options options ;
options.help = sina(keywords[kwHELP ],argv) || argc < 2;
Expand All @@ -811,6 +816,7 @@ int main(int argc,const char* argv[])
options.version = sina(keywords[kwVERSION],argv);
options.dst = sina(keywords[kwDST ],argv);
options.ascii = sina(keywords[kwASCII ],argv);
options.remove = sina(keywords[kwASCII ],argv);

for ( int i = 1 ; !result && i < argc ; i++ ) {
const char* arg = argv[i++];
Expand All @@ -831,6 +837,7 @@ int main(int argc,const char* argv[])
case kwDRYRUN : options.dryrun = true ; break;
case kwVERBOSE : options.verbose = true ; break;
case kwASCII : options.ascii = true ; break;
case kwREMOVE : options.remove = true ; break;
case kwTZ : Position::tz_ = parseTZ(value);break;
case kwADJUST : Position::adjust_ = ivalue;break;
case kwDELTA : Position::deltaMax_ = ivalue;break;
Expand Down Expand Up @@ -881,8 +888,8 @@ int main(int argc,const char* argv[])
s-= m*60 ;
printf("tz,dst,adjust = %d,%d,%d total = %dsecs (= %d:%d:%d)\n",t,d,a,A,h,m,s);
}
/*
if ( options.verbose ) {

if ( options.verbose && gTimeDict.size() ) {
printf("Time Dictionary\n");
for ( TimeDict_i it = gTimeDict.begin() ; it != gTimeDict.end() ; it++ ) {
std::string sTime = getExifTime(it->first);
Expand All @@ -894,7 +901,10 @@ int main(int argc,const char* argv[])
printf("%s %s\n",sTime.c_str(), sPos.c_str());
}
}
*/

if ( !gTimeDict.size() && gFiles.size() && !options.remove ) {
printf("nothing in time dictionary!\n");
}
for ( size_t p = 0 ; p < gFiles.size() ; p++ ) {
std::string path = gFiles[p] ;
std::string stamp ;
Expand All @@ -905,8 +915,22 @@ int main(int argc,const char* argv[])
if ( image.get() ) {
image->readMetadata();
Exiv2::ExifData& exifData = image->exifData();

// if asked to remove, cycle the metadata and remove GPSInfo
// you can remove and geotag in the same run
// this is very useful when the TZ is incorrectly set to "clean" GPS before writing new data
if ( options.remove ) {
for (Exiv2::ExifData::iterator pos = exifData.begin(); pos != exifData.end(); ++pos) {
if ( pos->groupName() == "GPSInfo" ) {
if ( options.verbose ) {
std::cout << "remove: " << pos->key() << std::endl;
}
if ( !options.dryrun ) exifData.erase(pos);
}
}
}
if ( pPos ) {
exifData["Exif.GPSInfo.GPSProcessingMethod" ] = "65 83 67 73 73 0 0 0 72 89 66 82 73 68 45 70 73 88"; // ASCII HYBRID-FIX
exifData["Exif.GPSInfo.GPSProcessingMethod" ] = "charset=Ascii HYBRID-FIX";
exifData["Exif.GPSInfo.GPSVersionID" ] = "2 2 0 0";
exifData["Exif.GPSInfo.GPSMapDatum" ] = "WGS-84";

Expand All @@ -924,7 +948,9 @@ int main(int argc,const char* argv[])

printf("%s %s % 2d\n",path.c_str(),pPos->toString().c_str(),pPos->delta());
} else {
printf("%s *** not in time dict ***\n",path.c_str());
if ( gTimeDict.size() ) {
printf("%s *** not in time dict ***\n",path.c_str());
}
}
if ( !options.dryrun ) image->writeMetadata();
}
Expand Down
4 changes: 0 additions & 4 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,6 @@ generate_export_header(exiv2lib
STATIC_DEFINE exiv2lib_STATIC
)

target_compile_features(exiv2lib_int PRIVATE cxx_std_98)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some other usages of target_compile_features in the unitTests and samples folders.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got rid of all the target_compile_features magic.

target_compile_features(exiv2lib PRIVATE cxx_std_98)


# Conditional addition of sources to library targets
# ---------------------------------------------------------

Expand Down
1 change: 0 additions & 1 deletion src/basicio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ typedef short nlink_t;
// *****************************************************************************
// class member definitions
namespace Exiv2 {

BasicIo::~BasicIo()
{
}
Expand Down
2 changes: 1 addition & 1 deletion test/data/geotag-test.out
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ Exif.GPSInfo.GPSAltitudeRef Byte 1 Below sea level
Exif.GPSInfo.GPSAltitude Rational 1 14.3 m
Exif.GPSInfo.GPSTimeStamp Rational 3 09:54:28
Exif.GPSInfo.GPSMapDatum Ascii 7 WGS-84
Exif.GPSInfo.GPSProcessingMethod Undefined 58 65 83 67 73 73 0 0 0 72 89 66 82 73 68 45 70 73 88
Exif.GPSInfo.GPSProcessingMethod Undefined 18 HYBRID-FIX
Exif.GPSInfo.GPSDateStamp Ascii 20 2008:05:08 09:54:28
1 change: 0 additions & 1 deletion unitTests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ target_compile_definitions(unit_tests
exiv2lib_STATIC
TESTDATA_PATH="${PROJECT_SOURCE_DIR}/test/data"
)
target_compile_features(unit_tests PRIVATE cxx_std_98)

if (exiv2lib_COMPILE_DEFINITIONS)
target_compile_definitions(unit_tests PRIVATE ${exiv2lib_COMPILE_DEFINITIONS})
Expand Down