ros-visualization / python_qt_binding

http://wiki.ros.org/python_qt_binding
BSD 3-Clause "New" or "Revised" License
34 stars 54 forks source link

Modifying sip_configure for improper lib names #53

Closed brawner closed 6 years ago

brawner commented 6 years ago

This came up when building qt_gui_cpp. When I dropped the boost library find_package, it resolved variables to white space. In turn sip_configure.py added an unnecessary -l to the makefile LIBs command causing make issues.

This adds a check to sip_configure.py for whether the lib name is None, an empty string or whitespace.

For example, when I build qt_gui_cpp this is the diff for the qt_gui_cpp_sip.so Makefile if I ask CMakeLists to find_package(Boost...) (even if the code doesn't use it) and if I don't.

< With find_package(Boost)
> Without find_package(Boost)
< LIBS = -Flib -Llib -F/usr/local/Cellar/qt/5.11.2/lib -L/usr/local/Cellar/qt/5.11.2/lib /usr/local/lib/libboost_filesystem-mt.dylib /usr/local/lib/libboost_system-mt.dylib -framework QtCore -framework DiskArbitration -framework IOKit -framework QtGui -F$$[QT_INSTALL_LIBS] -framework QtCore -framework DiskArbitration -framework IOKit -framework QtWidgets -framework QtPrintSupport
---
> LIBS = -Flib -Llib -F/usr/local/Cellar/qt/5.11.2/lib -L/usr/local/Cellar/qt/5.11.2/lib -l -framework QtCore -framework DiskArbitration -framework IOKit -framework QtGui -F$$[QT_INSTALL_LIBS] -framework QtCore -framework DiskArbitration -framework IOKit -framework QtWidgets -framework QtPrintSupport

Or condensed:

< LIBS = -Flib -Llib ... libboost_system-mt.dylib -framework QtCore ...
---
> LIBS = -Flib -Llib ... qt/5.11.2/lib -l -framework QtCore...

And this is the line from the Makefile with this bug fix

< LIBS = -Flib -Llib ... qt/5.11.2/lib -framework QtCore ...
dirk-thomas commented 6 years ago

When I dropped the boost library find_package, it resolved variables to white space.

Can you clarify which variable you are referring to? The current patch looks more like fixing a symptom rather the actual problem.

brawner commented 6 years ago

@dirk-thomas After removing all the boost dependencies, removing this line in qt_gui_cpp/CMakeLists.txt causes the issue above. I found this on mac, linux and the CI machines.

https://github.com/ros-visualization/qt_gui_core/pull/133#discussion_r229824783

I've confirmed that the LIBS variable is empty in the sip_helper.cmake with status messages https://github.com/ros-visualization/python_qt_binding/blob/49031b313269ff7f2bc5dd6e2bbc1438a68e1c5d/cmake/sip_helper.cmake#L207

And even in sip_configure.py, the libs variable is empty here until after generate. https://github.com/ros-visualization/python_qt_binding/blob/49031b313269ff7f2bc5dd6e2bbc1438a68e1c5d/cmake/sip_configure.py#L136

I couldn't think of anywhere else to introduce this fix.

brawner commented 6 years ago

Closing to retarget crystal