qmake support is going away

[Dirk Hohndel]

On Fri, Apr 17, 2015 at 10:41:57PM +0300, Lubomir I. Ivanov wrote:
> 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. :\

Linus uses gmail and his attachments are text... but you are on Windows I
assume? Which browser?

> > 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

That's interesting. Maybe we should fix THAT :-)

Is it forward vs. backward slash? Or something similarly stupid?

> 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

Makes sense.

> >> @@ -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.

OK, then let's go with that.

> >>       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.

so the variable contains "-l" ??? that's very odd. I wonder why.

> >> +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.

Yep.
If find_package() it's supposed to set the variable to MARBLE_NOT_FOUND or
something along those lines.
Simply insert debug prints with message(STATUS <whatever you want to print>)
to find out what it gets set to...

> >>  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.

Of course. But then where does the library get passed to the linker.
There's some magic here that I'm missing :-)

> > 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.

Thanks. Much appreciated.

> > 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.

Ahhhh. That must have been the part I was missing. I need to re-read that
code one more time.

> 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.

I see you already use the same debugging technique that I have fallen back
to :-)

/D