qmake support is going away

Fri Apr 17 12:41:57 PDT 2015

On 17 April 2015 at 22:25, Dirk Hohndel <dirk at hohndel.org> wrote:
> The issue with base64 encoded patches is that they are such a pain to
> respond to :-(

no idea how to turn that off for gmail. :\

>
> On Fri, Apr 17, 2015 at 10:07:04PM +0300, Lubomir I. Ivanov wrote:
>> >
>> > So with the latest master, what still breaks for you?
>> >
>>
>> two patches attached:
>> 1) this adds some options - NO_TESTS, NO_DOCS etc.
>> please let me know if these don't make any sense.
>>
>> the PKGCONFIG_* options allow me to use my library path where libgit
>> and libdc are installed even if forked from the subsurface git.
>
> Why do you need that? Is it not possible to just install those in
> install-root next to the Subsurface sources? That's why Tomaz and I added
> that feature, so that regardless how things are setup you simply install
> things at a consistent spot and all works out of the box...
>

i have two reasons:
- FIND_PACKAGE() fails for me even if they are parallel to the subsurface source
it finds pkg-config (with a crazy hack i did writing a windows .cmd to
.exe wrapper), but nothing else.
- i have *all* my libraries in a location which differs from where the
projects i contribute to are

> OK, here's my take on your first patch:
>
>
>
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>> index 93b0bdf..b15b1a5 100644
>> --- a/CMakeLists.txt
>> +++ b/CMakeLists.txt
>> @@ -7,7 +7,14 @@ cmake_minimum_required(VERSION 2.8.11)
>>
>>  SET(CMAKE_AUTOMOC ON)
>>  SET(CMAKE_AUTOUIC ON)
>> -OPTION(PREFER_GIT_FROMSOURCE "Turn off if you wanna use system's libgit 0.21.5" ON)
>> +
>> +#options
>> +
>> +OPTION(PKGCONFIG_LIBGIT2 "use pkg-config to retrieve libgit2" OFF)
>> +OPTION(PKGCONFIG_LIBDC "use pkg-config to retrieve libdivecomputer" OFF)
>> +OPTION(NO_MARBLE "disable the marble widget" OFF)
>> +OPTION(NO_TESTS "disable the tests" OFF)
>> +OPTION(NO_DOCS "disable the docs" OFF)
>>
>>  SET(CMAKE_MODULE_PATH ${${PROJECT_NAME}_SOURCE_DIR}/cmake/Modules)
>>  INCLUDE_DIRECTORIES( . ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_BINARY_DIR} qt-ui qt-ui/profile)
>> @@ -30,18 +37,31 @@ pkg_config_library(LIBUSB libusb-1.0 QUIET)
>>
>>  # more libraries with special handling in case we build them ourselves
>>
>> -if(NOT ${PREFER_GIT_FROMSOURCE})
>> +IF(PKGCONFIG_LIBGIT2)
>
> So you are inverting "FROMSOURCE" and are turning it into FROMPKGCONFIG.
>
> Why is that better?

for me it implies better what the option does.
what you suggested is even better <libname>_FROM_PKGCONFIG.

>
>>       pkg_config_library(LIBGIT2 libgit2 REQUIRED)
>> +     SET(LIBGIT2_LIBRARIES "")
>
> Hmm - don't you clear out the variable that you just set?

if i don't set it to an empty string it passes "-l" to the linker.

>
>>  ELSE()
>>       FIND_PACKAGE(LIBGIT2 REQUIRED)
>>       INCLUDE_DIRECTORIES(${LIBGIT2_INCLUDE_DIR})
>>  ENDIF()
>>
>> -FIND_PACKAGE(Libdivecomputer REQUIRED)
>> -INCLUDE_DIRECTORIES(${LIBDIVECOMPUTER_INCLUDE_DIR})
>> +IF(PKGCONFIG_LIBDC)
>> +     pkg_config_library(LIBDC libdivecomputer REQUIRED)
>> +     SET(LIBDIVECOMPUTER_LIBRARIES "")
>
> Again clearing out the libs. Weird

same as above.

>
>> +ELSE()
>> +     FIND_PACKAGE(Libdivecomputer REQUIRED)
>> +     INCLUDE_DIRECTORIES(${LIBDIVECOMPUTER_INCLUDE_DIR})
>> +ENDIF()
>> +
>> +# optional marble
>>
>> -FIND_PACKAGE(Marble REQUIRED)
>> -include_directories(${MARBLE_INCLUDE_DIR})
>> +IF(NO_MARBLE)
>> +     ADD_DEFINITIONS(-DNO_MARBLE)
>> +     SET(MARBLE_LIBRARIES "")
>> +ELSE()
>> +     FIND_PACKAGE(Marble REQUIRED)
>> +     include_directories(${MARBLE_INCLUDE_DIR})
>> +ENDIF()
>
> Why doesn't this add a check that if Marble wasn't found NO_MARBLE is set?
>

i don't really know how to add the check, cmake wise.
could end up being another patch.

>>  SET(SUBSURFACE_LINK_LIBRARIES   ${SUBSURFACE_LINK_LIBRARIES} ${LIBDIVECOMPUTER_LIBRARIES}  ${LIBGIT2_LIBRARIES} -lusb-1.0)
>
> But those xxx_LIBRARIES variables were set to "" above. So how does this
> work?
>

it doesn't pass the empty -l to my linker, which quite bad.

>> @@ -51,7 +71,9 @@ STRING(COMPARE EQUAL "${${PROJECT_NAME}_SOURCE_DIR}" "${${PROJECT_NAME}_BINARY_D
>>  GET_FILENAME_COMPONENT(PARENTDIR ${${PROJECT_NAME}_SOURCE_DIR} PATH)
>>  STRING(COMPARE EQUAL "${${PROJECT_NAME}_SOURCE_DIR}" "${PARENTDIR}" insourcesubdir)
>>  IF(NOT (insource OR insourcedir))
>> -     add_custom_target(link_marble_data ALL COMMAND rm -f marbledata && ln -s ${${PROJECT_NAME}_SOURCE_DIR}/marbledata ${${PROJECT_NAME}_BINARY_DIR}/marbledata)
>> +     IF(NOT NO_MARBLE)
>> +             add_custom_target(link_marble_data ALL COMMAND rm -f marbledata && ln -s ${${PROJECT_NAME}_SOURCE_DIR}/marbledata ${${PROJECT_NAME}_BINARY_DIR}/marbledata)
>> +     ENDIF()
>>  ENDIF()
>>
>>  #configure Qt.
>
> That's a good one
>
>> @@ -339,12 +361,16 @@ ENDMACRO()
>>  ENABLE_TESTING()
>>  ADD_DEFINITIONS(-DSUBSURFACE_SOURCE="${CMAKE_SOURCE_DIR}")
>>  ADD_DEFINITIONS(-g)
>> -test(TestUnitConversion testunitconversion.cpp)
>> -test(TestProfile testprofile.cpp)
>> -test(TestGpsCoords testgpscoords.cpp)
>> -test(TestParse testparse.cpp)
>> +IF(NOT NO_TESTS)
>> +     test(TestUnitConversion testunitconversion.cpp)
>> +     test(TestProfile testprofile.cpp)
>> +     test(TestGpsCoords testgpscoords.cpp)
>> +     test(TestParse testparse.cpp)
>> +ENDIF()
>>
>> -ADD_CUSTOM_TARGET(documentation ALL mkdir -p ${CMAKE_BINARY_DIR}/Documentation/ \\; make -C ${CMAKE_SOURCE_DIR}/Documentation OUT=${CMAKE_BINARY_DIR}/Documentation/ doc)
>> +IF(NOT NO_DOCS)
>> +     ADD_CUSTOM_TARGET(documentation ALL mkdir -p ${CMAKE_BINARY_DIR}/Documentation/ \\; make -C ${CMAKE_SOURCE_DIR}/Documentation OUT=${CMAKE_BINARY_DIR}/Documentation/ doc)
>> +ENDIF()
>>
>>  # install Subsurface
>>  # first some variables with files that need installing
>
> So could I get one commit that adds "NO_TESTS" and "NO_DOCS"
> One commit that fixes the NO_MARBLE case
> And then one commit that makes sense and that deals with the pkgconfig
> stuff. I'm OK with the inverted naming logic (just have them be
> <libname>_FROM_PKGCONFIG) as long as it's used consistently.
>

this one was more of a review target, i guess.
ok, i will split them up.

> And I really want to know how your patch works if you are clearing out the
> variables for the LIBRARIES. I must be missing something there...
>

once i clear those variables i don't get the "-l" and
pkg_config_library() adds the libraries instead.

i've tried to use message() to try to make some sense of the entire thing.
calling pkg_config_library() appends the libraries to the
SUBSURFACE_LINK_LIBRARIES variable.

lubomir
--