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

jpeg-turbo detection doesn't properly handle not finding it #191

Open
mstewartgallus opened this issue Feb 4, 2015 · 2 comments
Open

jpeg-turbo detection doesn't properly handle not finding it #191

mstewartgallus opened this issue Feb 4, 2015 · 2 comments

Comments

@mstewartgallus
Copy link

The code here isn't good. It should error out if it can't find jpeg-turbo and not just fill in some randomness and result in a link failure halfway through building Vogl.

    else()
        if (BUILD_X64)
            set(BITS_STRING "x64")
        else()
            set(BITS_STRING "x86")
        endif()
        set(LibJpegTurbo_INCLUDE "${CMAKE_PREFIX_PATH}/libjpeg-turbo-2.1.3/include" PARENT_SCOPE)
        set(LibJpegTurbo_LIBRARY "${CMAKE_PREFIX_PATH}/libjpeg-turbo-2.1.3/lib_${BITS_STRING}/turbojpeg.lib" PARENT_SCOPE)
    endif()
@pwaller
Copy link

pwaller commented Feb 8, 2015

I just hit this. Needed to install more dependencies, would have been nice if cmake would have failed instead of the make.

vogl/vogl_build/release64$ make
[ 27%] Built target voglcore
[ 27%] Built target voglgen
[ 28%] Built target voglgen_make_inc
[ 33%] Built target backtracevogl
[ 50%] Built target voglcommon
[ 54%] Built target vogl
[ 54%] Built target voglbench
make[2]: *** No rule to make target `/libjpeg-turbo-2.1.3/lib_x64/turbojpeg.lib', needed by `../libvogltrace64.so'. Stop.
make[1]: *** [src/vogltrace/CMakeFiles/vogltrace.dir/all] Error 2

@kingtaurus
Copy link
Contributor

@sstewartgallus It looks like the code that you quote is from src/build_options.cmake.

I'll quote the full function here:

function(require_libjpegturbo)
    find_library(LibJpegTurbo_LIBRARY
        NAMES libturbojpeg.so libturbojpeg.so.0 libturbojpeg.dylib
        PATHS /opt/libjpeg-turbo/lib 
    )

    # On platforms that find this, the include files will have also been installed to the system
    # so we don't need extra include dirs.
    if (LibJpegTurbo_LIBRARY)
        set(LibJpegTurbo_INCLUDE "" PARENT_SCOPE)

    # On Darwin we assume a Homebrew install
    elseif (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
        set(LibJpegTurbo_INCLUDE "/usr/local/opt/jpeg-turbo/include"            PARENT_SCOPE)
        set(LibJpegTurbo_LIBRARY "/usr/local/opt/jpeg-turbo/lib/libturbojpeg.a" PARENT_SCOPE)

    else()
        if (BUILD_X64)
            set(BITS_STRING "x64")
        else()
            set(BITS_STRING "x86")
        endif()
        set(LibJpegTurbo_INCLUDE "${CMAKE_PREFIX_PATH}/libjpeg-turbo-2.1.3/include" PARENT_SCOPE)
        set(LibJpegTurbo_LIBRARY "${CMAKE_PREFIX_PATH}/libjpeg-turbo-2.1.3/lib_${BITS_STRING}/turbojpeg.lib" PARENT_SCOPE)
    endif()
endfunction()

CMake looks in /opt/libjpeg-turbo/lib in addition CMake associated paths (for the turbojpeg library). I believe the else section that you quote is a residual of the original build procedure (and is unlikely to find turbojpeg). I agree that it would be better to replace the else() with failing out with an error.

So, perhaps the else() above becomes:

message(FATAL_ERROR "Unable to find libturbojpeg")

In addition, the search for the header, /usr/include/turbojpeg.h should be made explicit (especially if header is placed in a path that is not being searched). The configuration process would succeed, but the build process would fail at the attempted to find turbojpeg.h during compilation of src/vogltrace/vogl_intercept.cpp.

# On platforms that find this, the include files will have also been installed to the system
# so we don't need extra include dirs.
if (LibJpegTurbo_LIBRARY)
   set(LibJpegTurbo_INCLUDE "" PARENT_SCOPE)

to something like (I haven't tested below, but I believe this should work):

if (LibJpegTurbo_LIBRARY)
   find_path(LIBJPEGTURBO_INCLUDE_DIR
                   NAME turbojpeg.h
                   PATHS /opt/libjpeg-turbo/inc)
   IF(NOT LIBJPEGTURBO_INCLUDE_DIR)
     message(FATAL_ERROR "Unable to find turbojpeg.h")
   ENDIF()
   include_directories(${LIBJPEGTURBO_INCLUDE_DIR})

I would recommend an additional set of checks to see the library found by CMake has the appropriate set of symbols defined and the version of the found library is correct.

EDIT: removed an extra ENDIF()

kingtaurus added a commit to kingtaurus/vogl that referenced this issue Feb 12, 2015
  Fix for issue ValveSoftware#191 (https://github.com/ValveSoftware/vogl/).
  (1) Fails when libjpegturbo is not found.
  (2) Adds a check for the header file turbojpeg.h
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

3 participants