exiv2-0.26: build failure

[tgurr]

I (and our CI, see the run: https://galileo.mailstation.de/jenkins/job/media/2191/console) get a build failure:

[ 62%] Linking CXX executable ../bin/exiv2
cd /var/tmp/paludis/build/graphics-exiv2-0.26/work/build/src && /usr/x86_64-pc-linux-gnu/bin/cmake -E cmake_link_script CMakeFiles/exiv2.dir/link.txt --verbose=1
/usr/bin/x86_64-pc-linux-gnu-c++  -march=native -O2 -pipe  -rdynamic CMakeFiles/exiv2.dir/exiv2.cpp.o CMakeFiles/exiv2.dir/actions.cpp.o CMakeFiles/exiv2.dir/utils.cpp.o  -o ../bin/exiv2 -Wl,-rpath,/var/tmp/paludis/build/graphics-exiv2-0.26/work/build/src: libexiv2.so.26.0.0 

Error:
  * In program cave perform install --hooks --managed-output --output-exclusivity with-others =graphics/exiv2-0.26:0::media --destination installed --replacing =graphics/exiv2-0.26:0::installed --x-of-y 1 of 1:
  * When installing 'graphics/exiv2-0.26:0::media' replacing { 'graphics/exiv2-0.26:0::installed' }:
  * When running an ebuild command on 'graphics/exiv2-0.26:0::media':
  * Install failed for 'graphics/exiv2-0.26:0::media' (paludis::ActionFailedError)

make[2]: Leaving directory '/var/tmp/paludis/build/graphics-exiv2-0.26/work/build'
make[1]: Leaving directory '/var/tmp/paludis/build/graphics-exiv2-0.26/work/build'
libexiv2.so.26.0.0: undefined reference to `pthread_rwlock_unlock'
libexiv2.so.26.0.0: undefined reference to `pthread_rwlock_init'
libexiv2.so.26.0.0: undefined reference to `pthread_rwlock_rdlock'
libexiv2.so.26.0.0: undefined reference to `pthread_rwlock_destroy'
libexiv2.so.26.0.0: undefined reference to `pthread_rwlock_wrlock'
collect2: error: ld returned 1 exit status
make[2]: *** [src/CMakeFiles/exiv2.dir/build.make:151: bin/exiv2] Error 1
make[1]: *** [CMakeFiles/Makefile2:241: src/CMakeFiles/exiv2.dir/all] Error 2
make: *** [Makefile:133: all] Error 2

We run into that when trying to compile exiv2 with both curl/ssh options disabled:

Fails:
-DEXIV2_ENABLE_CURL:BOOL=FALSE
-DEXIV2_ENABLE_SSH:BOOL=FALSE

Works:
-DEXIV2_ENABLE_CURL:BOOL=FALSE
-DEXIV2_ENABLE_SSH:BOOL=TRUE

Works:
-DEXIV2_ENABLE_CURL:BOOL=TRUE
-DEXIV2_ENABLE_SSH:BOOL=FALSE

Works:
-DEXIV2_ENABLE_CURL:BOOL=TRUE
-DEXIV2_ENABLE_SSH:BOOL=TRUE

[clanmills]

Thanks for reporting this. I'm unable to reproduce this on MacOS-X (10.12) and Ubunutu (16.04LTS).

The issue seems to be the linking to pthread_rwlock_unlock being undefined. You'll have to install libpthread https://www.gnu.org/software/hurd/libpthread.html

365 rmills@ubuntu:~/gnu/git/exiv2/build $ uname -a
Linux ubuntu 4.4.0-78-generic #99-Ubuntu SMP Thu Apr 27 15:29:09 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
366 rmills@ubuntu:~/gnu/git/exiv2/build $ exiv2 -vV | grep thread
library=/lib/x86_64-linux-gnu/libpthread.so.0
367 rmills@ubuntu:~/gnu/git/exiv2/build $ 

Another observation. I am a little surprised by this because the CURL=0 and SSH=0 are the defaults! I'm wondering about your cmake syntax as I use -DEXIV2_ENABLE_CURL=1 to enable options and -DEXIV2_ENABLE_CURL=0 to disable.

[tgurr]

If I compile with -DEXIV2_ENABLE_CURL=1 -DEXIV2_ENABLE_SSH=1 it works fine and I get pretty much the same output like you do regarding libpthread.so.0 which belongs to glibc-2.25 so there shouldn't be any need to install libpthread separately, I would be surprised if libpthread.so.0 doesn't belong to glibc on ubuntu as well.

# uname -a
Linux lxpc6170 4.10.1-gentoo #1 SMP Mon Mar 6 16:47:28 CET 2017 x86_64 GNU/Linux
# exiv2 -vV | grep thread
library=/usr/x86_64-pc-linux-gnu/lib/libpthread.so.0
# cave owner /usr/x86_64-pc-linux-gnu/lib/libpthread.so.0
sys-libs/glibc-2.25::installed

[...]
-- Compiler:x Major:6 Minor:3
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - not found
-- Check if compiler accepts -pthread
-- Check if compiler accepts -pthread - yes
-- Found Threads: TRUE
[...]

As far as I can see CURL=0 and SSH=0 are the defaults for the win32 build only:

IF (WIN32)
    [...]
    OPTION( EXIV2_ENABLE_CURL          "USE Libcurl for HttpIo"                            OFF )
    OPTION( EXIV2_ENABLE_SSH           "USE Libssh for SshIo"                              OFF )
ELSE()
    OPTION( EXIV2_ENABLE_CURL          "USE Libcurl for HttpIo"                            ON  )
    OPTION( EXIV2_ENABLE_SSH           "USE Libssh for SshIo"                              ON  )
    [...]
ENDIF()

Since we're a source-based distribution we allow our users to set options when installing a package. In the case of exiv2 we have sftp and curl options. That's why we define the two cmake options manually so they're either enabled or disabled based on users choice. We could also just hard-enable them but maybe some users don't want remote options in exiv2 so I decided to add options for them. However we try to follow upstream suggestions where it makes sense so if you tell me a build without curl and ssh support is crippled and troublesome I might as well switch them on by default. However since offer that options they should work as intended.

-DEXIV2_ENABLE_CURL=0 -DEXIV2_ENABLE_SSH=0 yields the same error.

What I found regarding phtreads is this:
https://stackoverflow.com/questions/1620918/cmake-and-libpthread
I've made a quick test.patch.txt which is probably not thought through to the end but it allows me to compile exiv2 with -DEXIV2_ENABLE_CURL=0 -DEXIV2_ENABLE_SSH=0.

Other things I wondered about is in src/CMakeLists.txt ADD_EXECUTABLE( conntest ${CONNTEST} ) and ADD_EXECUTABLE( remotetest ${REMOTETEST} ). The tools are built regardless of -DEXIV2_ENABLE_BUILD_SAMPLES being disabled or enabled.

And another questions about the EXIV2_ENABLE_WEBREADY option, is it just a wrapper for enabling both CURL and SSH or does it actually add some functionality?

[tgurr]

Small correction about my comment regarding the CURL and SSH being ON by default, I just noticed that even though CURL and SSH are ON by default, EXIV2_ENABLE_WEBREADY is not and thus they're actually OFF by default as well.

[clanmills]

Lots of stuff here. EXIV2_ENABLE_WEBREADY turns on/off the remote IO. EXIV2_ENABLE_{CURL|SSH} give you fine-grained control of the WebReady (remote IO) capability. So, you may enable WEBREADY with CURL (and without SSH). Incidentally, ALL versions of Exiv2 support HTTP. Even if you disable CURL we have a "no thrills" HttpIo implementation which is sufficient for reading metadata over HTTP.

You are right to remind me that pthread support on some platforms is provided by glibc and not libpthread. Thank you for the patch in test.patch.txt. I'll review/commit this in the next 24 hours.

I appreciate your eagle eyes observing that some of the sample apps escape EXIV2_ENABLE_BUILD_SAMPLES Some of our "samples" (such as addmoddel) are essential components of the test suite and are always build. I believe remotest and conntest (which are part of the WebReady feature) are always built. As, mentioned before, HttpIo support is provided in all builds of exiv2. The test suite interrogates the output of exiv2 -vV (verbose Version) to determine the appropriate tests to apply for the build configuration.

Thank You for all the work you are putting into exiv2. It sounds as though you are providing a layer of tooling to help users build many open-source libraries. Thank You for doing that. And thank you for give me this feedback and patches. In the software world, 1+1=3. Thanks for working with me on this.

[tgurr]

Okay so EXIV2_ENABLE_WEBREADY doesn't actually add any additional funktionality and I get the same with -DEXIV2_ENABLE_CURL=1 -DEXIV2_ENABLE_SSH=1 if I understood correctly.

Thanks for all the explanation, also regarding the HTTP support in general and the background why some sample apps are build even if EXIV2_ENABLE_BUILD_SAMPLES is disabled.

About my patch I'm not quite sure if it's the best way to solve it (which is why I didn't provide another pull request), as I don't know how linking with Threads::Threads works on FreeBSD, MacOS and Windows.

But it's more of a problem on where to place the line I guess and under what conditions. I went through the CMakeList.txt again and spotted that you even already use that Threads::Threads CMake (>=3.1) macro and there might be some problems with it.

For example the required stuff in ./CMakeList.txt

set(THREADS_PREFER_PTHREAD_FLAG ON)
find_package(Threads REQUIRED)

is wrapped in the EXIV2_ENABLE_XMP condition:

IF( EXIV2_ENABLE_XMP )
    SET( HAVE_XMP_TOOLKIT ON )
    SET( XMPLIB "do" )
    IF (NOT MINGW)
        set(THREADS_PREFER_PTHREAD_FLAG ON)
    ENDIF()
    find_package(Threads REQUIRED)
ENDIF( EXIV2_ENABLE_XMP )

And I guess I found out why you couldn't reproduce the problem, in src/CMakeList.txt you have:

if( EXIV2_ENABLE_LIBXMP )
    ADD_DEPENDENCIES( exiv2lib xmp )
    if  ( MSVC )
        LINK_DIRECTORIES(${LIBRARY_OUTPUT_PATH}/$(ConfigurationName))
    else()
        ADD_DEPENDENCIES( exiv2lib xmp Threads::Threads)
        TARGET_LINK_LIBRARIES( exiv2lib ${PRIVATE_VAR} ${EXPAT_LIBRARIES} Threads::Threads)
    endif(MSVC)
    TARGET_LINK_LIBRARIES( exiv2lib ${PRIVATE_VAR} xmp )
ENDIF()

so when I try to build with:
-DEXIV2_ENABLE_LIBXMP=1 -DEXIV2_ENABLE_CURL=0 -DEXIV2_ENABLE_SSH=0
it works for me as well. That means your already existent line to link exiv2lib with pthreads
TARGET_LINK_LIBRARIES( exiv2lib ${PRIVATE_VAR} ${EXPAT_LIBRARIES} Threads::Threads)
is basically enough for it to work. Now Threads::Threads just has to be added regardless of whether DEXIV2_ENABLE_LIBXMP (and EXIV2_ENABLE_XMP) being enabled or not.

I'm not a CMake expert or a programmer so please take my suggestions with care.

[clanmills]

Timo

Thanks for all these suggestions and ideas. I'm just about to work on this issue, so your timing is perfect. I'm not an expert in CMake. I do what works and hope for the best.

Windows (MSVC) doesn't support pthreads. The code uses different Windows mutex/critical_section magic which depends on the version of Windows. I think Cygwin and MinGW use pthreads.

The CMakeLists.txt Threads code was added last year by Gilles:

  2746 robinwmill IF( EXIV2_ENABLE_XMP )
  3049 robinwmill     SET( HAVE_XMP_TOOLKIT ON )
  3049 robinwmill     SET( XMPLIB "do" )
  4689    cgilles     IF (NOT MINGW)
  4689    cgilles         set(THREADS_PREFER_PTHREAD_FLAG ON)
  4689    cgilles     ENDIF()
  4506 robinwmill     find_package(Threads REQUIRED)
  2746 robinwmill ENDIF( EXIV2_ENABLE_XMP )

I added the code in src/CMakeLists.txt concerning Threads::Threads. However I think it arrived in a patch with the RWLock code. That patch has generated many platform/build issues.

  2746 robinwmill if( EXIV2_ENABLE_LIBXMP )
  2924 robinwmill     ADD_DEPENDENCIES( exiv2lib xmp )
  3767 robinwmill     if  ( MSVC )
  3479 robinwmill         LINK_DIRECTORIES(${LIBRARY_OUTPUT_PATH}/$(ConfigurationName))
  4313 robinwmill     else()
  4313 robinwmill         ADD_DEPENDENCIES( exiv2lib xmp Threads::Threads)
  4313 robinwmill         TARGET_LINK_LIBRARIES( exiv2lib ${PRIVATE_VAR} ${EXPAT_LIBRARIES} Threads::Threads)
  3479 robinwmill     endif(MSVC)
  4000 danielkane     TARGET_LINK_LIBRARIES( exiv2lib ${PRIVATE_VAR} xmp )
  2746 robinwmill ENDIF()

I will look at the code you sent yesterday and have a Google concerning glibc/pthread. I'll create a new VM for a system which supports threads with glibc (and doesn't have libpthread). As always, I hope to fix this first time and I know you'll let me know if I don't!

[clanmills]

Reopen. Thank You @bkuhls for the patch. I'd like to spend more time on this to investigate all the facets discussed by @tgurr

[clanmills]

Timo

I've had a good look through the "Threads" code in the CMake*.txt files and simplified. I've built and tested this on MacOS-X, Linux and Cygwin. 290164d

Mostly, I've replaced Threads::Threads with ${CMAKE_THREAD_LIBS_INIT} which is set by FindThreads.cmake and understands gclib, libpthread and MSVC. By coincidence @bkuhls last week suggested making threads unconditional (always linked) and I accepted his patch for that.

I suspect we link threads in unnecessary places. For example, if threads are linked to libexiv2, I don't think we need to explicitly link them to applications which use the library. And a similar argument could be applied to EXPAT, ZLIB, CURL and so on. However ${EXPAT_LIBRARIES} is explicitly required by geotag which reads a GPX file written in XML.

I agree that we should show more respect for EXIV2_ENABLE_BUILD_SAMPLES However, changing this will probably produce a downstream "ripple" of build/test/platform issues which will consume time with little gain.

I'm going to close this issue now. If you discover more about this, please reopen this issue or a new issue and I will investigate.