Add option to disable tools (currently exiv2 binary) #32
Conversation
|
I think it is a good idea to add this variable to disable the generation of the apps. I would even control with that variable the generation of the samples. I think many developers might be only interested in the usage of the libraries and they might not be interested at all in compiling the applications. If @clanmills agrees, I will propose you some ideas to improve this Pull Request. |
|
Yes I thought about samples as well and ignored it for now - it is unfortunate that a subdirectory labelled 'unit tests' is installing stuff at all - and is seemingly also required to generate doc. |
|
With the autotools build, the default build is the library + about 10 little programs used by the test suite: exiv2/exifprint/addmoddel/etc.. $ make samples builds the other samples. I believe the CMake build defaults to building the libraries + exiv2 + samples + test programs. Perhaps well could polish this to be: 1 default builds = library + exiv2 application However I don't have any strong feelings about this. The library is about 75000 lines of C++. The samples and test programs are small (10000 lines of C++). What is "unit tests"? |
|
Took that from
|
In a multiarch build, one may want to build binaries only for native arch.
| add_executable( exiv2 ${EXIV2_SRC} ) | ||
| target_link_libraries( exiv2 PRIVATE exiv2lib ) | ||
| install(TARGETS exiv2 RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) | ||
| if(EXIV2_ENABLE_TOOLS) |
There was a problem hiding this comment.
As I said before I think it is a good idea, but the current version is not complete. Let's say that we set EXIV2_ENABLE_TOOLS=OFF. Then in the lines 300 and 305 we are assuming the existence of the exiv2 target.
I think the best we could do is to move the exiv2 sources to a different folder, but that would force us to also update all the visual studio solutions and the autotools code. Therefore that option is discarded for the moment.
In order to accept this Pull Request you would need to do 2 additional things:
- Move the EXIV2_SRC variable inside the
if(EXIV2_ENABLE_TOOLS)block. - Add also the
if(EXIV2_ENABLE_TOOLS)check for the lines 300 and 305.
There was a problem hiding this comment.
You have the correct idea in reorganizing the code. We really should have src/ for the library code (test/src=test applications) and samples/src for sample applications. However, this is lots of messing about. Your thoughts involving if (EXIV2_ENABLE_TOOLS) is the quick fix.
I'm got a couple of other things to suggest:
- It should be called EXIV2_ENABLE_BUILD_EXIV2APP "Build exiv2 command line program"
- The help string for EXIV2_ENABLE_BUILD_SAMPLES "Build sample applications"
In a multiarch build, one may want to build binaries only for native arch.