Patch #1012

Avoid cyclic inclusion dependency between http.hpp and exiv2.hpp

Added by Thomas Beutlich over 2 years ago. Updated over 2 years ago.

Status:ClosedStart date:30 Dec 2014
Priority:NormalDue date:
Assignee:Robin Mills% Done:

100%

Category:build
Target version:0.25

Description

The attached patch avoids the cyclic inclusion dependency between http.hpp and exiv2.hpp.

http.patch Magnifier (534 Bytes) Thomas Beutlich, 30 Dec 2014 20:39

http_2.patch Magnifier (1.03 KB) Thomas Beutlich, 02 Jan 2015 19:23


Related issues

Related to Exiv2 - Bug #936: Video headers include internal include files and need to ... Closed 02 Dec 2013

Associated revisions

Revision 3508
Added by Robin Mills over 2 years ago

#1012. Thank you, Thomas, for reporting this a providing the patch.

Revision 3510
Added by Robin Mills over 2 years ago

#1012. r3508 broke the build on macosx and linux.
Fixed cosmetic issues in samples/httptest.cpp and <exiv2dir>/Makefile.

Revision 3511
Added by Robin Mills over 2 years ago

#1012. Thank You, Thomas for the patch.

Revision 3514
Added by Robin Mills over 2 years ago

#1012. Correction to r3513 for msvc2005/metacopy to respect new location of utils.hpp

Revision 3515
Added by Robin Mills over 2 years ago

#1012: more fixes to r3513. Fix msvc2005/metacopy and msvc2005/path-test to respect changed location of utils.hpp

Revision 3516
Added by Robin Mills over 2 years ago

#1012. Yet more fixes in . See r3513

Revision 3517
Added by Robin Mills over 2 years ago

#1012. Yet more fixes to path-test.vcproj. See r3513.

History

#1 Updated by Robin Mills over 2 years ago

  • Category set to build
  • Status changed from New to Assigned
  • Assignee set to Robin Mills

Thanks, Thomas. I'm sitting by a swimming pool on vacation in Tenerife, Spain. I'll add your patch when I get a better internet connection or get home next week. Happy New Year.

It is very helpful for you to find and identify matters such as this. Although the build server validates every commit on the trunk, I am currently building with MSVC 2005 on Windows + GCC 4.8.2 (Cygwin, MinGW, Linux) + Clang 600.0.56) on Mac. On my TODO list is for the build server to support MSVC 2008/10/12 and to building with CMake. CMake is working on Cygwin, Linux and Mac). However the build server does not use CMake at this time.

When you report a bug, may I ask you to let me know your build tools (compiler, platform). In this case, this issue is independent of the tools in use - however it's usually very relevant.

#2 Updated by Robin Mills over 2 years ago

  • Status changed from Assigned to Resolved

Fix submitted: r3508 and r3510. Thanks to Thomas for reporting this and providing the patch.

Curiously, Thomas' fix broke the build on Linux and Mac. However, thanks to having a build server, this was discovered effortlessly and easily fixed.

#3 Updated by Thomas Beutlich over 2 years ago

  • File http_2.patchMagnifier added
  • Status changed from Resolved to Feedback

Wheras r3508 successfully works on my VS 2010 IDE it fails after updating to r3510.

1>------ Build started: Project: exiv2lib, Configuration: Release Win32 ------
1>  basicio.cpp
1>C:\Projects\exiv2\include\exiv2\http.hpp(8): fatal error C1083: Cannot open include file: 'exiv2/exiv2.hpp': No such file or directory
1>  http.cpp
1>c:\Projects\exiv2\include\exiv2\http.hpp(8): fatal error C1083: Cannot open include file: 'exiv2/exiv2.hpp': No such file or directory
1>  version.cpp
1>C:\Projects\exiv2\include\exiv2\http.hpp(8): fatal error C1083: Cannot open include file: 'exiv2/exiv2.hpp': No such file or directory
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

The reason is the same as the original bug report. http.hpp is the only header that has an include directive of kind #include <exiv2/foo.hpp>, i.e. with angle brackets and exiv2 in the namespace. Also http.hpp includes exiv2.hpp and vice versa which is not clean programming. Attached is a second patch http_2 which solves both problems. Please double-check if it runs on other build environments.

#4 Updated by Robin Mills over 2 years ago

  • Status changed from Feedback to Resolved

Submitted r3511. Thank You, Thomas for the patch.

Please check build #1807 on the build server. (MSVC 2005 + Mac (clang) and GCC (Linux, MinGW, Cygwin). http://exiv2.dyndns.org:8080/job/Exiv2-trunk/1807/

Build #1806 failed on Cygwin/MSVC/MinGW because the Windows build machine had dropped dead for unknown reasons. I reboot the build server and build 1807 seems to be building normally.

Please test the msvc2005 build environment with Visual Studio 2010. I haven't had time to install and test this yet on the build machine. Differences between your build and environment are causing me additional/unexpected work.

#5 Updated by Thomas Beutlich over 2 years ago

Thanks, it is fine for me now.

#6 Updated by Robin Mills over 2 years ago

r3513. Regrettably the SVN log mentions #1004 instead of #1012.
r3514 Corrections to r3513

I have reviewed all #include directives in src/*.cpp and samples/*.cpp and made them consistent. Library (src/*.cpp) code never uses <exiv2/exiv2.hpp>. Sample code (samples/*.cpp) only uses <exiv2/exiv2.hpp> and some local includes (utils.hpp and exiv2app.hpp) which have been moved to src. utils.hpp and exiv2app.hpp are not part of the library and should not be public. Builds OK: http://exiv2.dyndns.org:8080/job/Exiv2-trunk/1817/

Please verify that I have not broken your private MSVC build environment. I have relocated include/exiv2/utils.hpp -> src/utils.hpp

#7 Updated by Thomas Beutlich over 2 years ago

Robin Mills wrote:

r3513. Regrettably the SVN log mentions #1004 instead of #1012.

You may try if you are allowed to post-edit the SVN commit message. Admin could provide the corresponding rights for it.

Please verify that I have not broken your private MSVC build environment. I have relocated include/exiv2/utils.hpp -> src/utils.hpp

Yes, it is working as expected (and cyclic header dependency is still gone.) Well done!

#8 Updated by Robin Mills over 2 years ago

Good. That's fine. I'll deal with the other stuff today. It's nice to be home and properly connected to the internet. The weather - oh well - the weather in England is always wet in winter.

I tried to edit the svn commit log and got an errors about "log editing not enabled". There's nothing I can do about that as I don't have an account on exiv2.org. I could raise a bug report about this, however I won't.

Thanks for the issues you detected with cppcheck. The Exiv2 project agreed to regularly scan our code with Coverity in 2015. Mahesh has the assignment. For sure eliminating all compiler warnings, all static analysis warnings and making our test suite more robust are all very worthwhile tasks to make our code strong and robust.

#9 Updated by Robin Mills over 2 years ago

  • % Done changed from 0 to 100

#10 Updated by Andreas Huggel over 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF

Redmine Appliance - Powered by TurnKey Linux