Merge lp:~sam-sgrs/ubuntu-filemanager-app/fix-1579487 into lp:ubuntu-filemanager-app

Proposed by Sam Segers
Status: Needs review
Proposed branch: lp:~sam-sgrs/ubuntu-filemanager-app/fix-1579487
Merge into: lp:ubuntu-filemanager-app
Diff against target: 99 lines (+42/-9)
2 files modified
CMakeLists.txt (+26/-2)
src/plugin/folderlistmodel/CMakeLists.txt (+16/-7)
To merge this branch: bzr merge lp:~sam-sgrs/ubuntu-filemanager-app/fix-1579487
Reviewer Review Type Date Requested Status
Carlos Jose Mazieri Needs Information
Review via email: mp+303622@code.launchpad.net

Description of the change

Fix when the samba libs are installed on the host.

The samba libs use the rpath when compiled so the host libs are looked for fist before the bundled ones.
They can be stripped from the rpath using 'chrpath -d'.
The folderlistmodel also needs to be build using the dev packages from the bundled apps.

To post a comment you must log in.
Revision history for this message
Carlos Jose Mazieri (carlos-mazieri) wrote :

Hello Sam,

It is OK to me, just a question:

   Have you tested building for armf on device and building for armf on qtcreator using a sdk like click-ubuntu-sdk-13.10-armhf?

This build was done by Alan Pope and Stefano Verzegnassi to prevent a bug that did not allow to install libsmbclien-dev in armf sdk, see https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287.

review: Needs Information
Revision history for this message
Sam Segers (sam-sgrs) wrote :

No. I didn't.
I'll check this later today.

Op wo 24 aug. 2016 00:49 schreef Carlos Jose Mazieri <
<email address hidden>>:

> Review: Needs Information
>
> Hello Sam,
>
> It is OK to me, just a question:
>
> Have you tested building for armf on device and building for armf on
> qtcreator using a sdk like click-ubuntu-sdk-13.10-armhf?
>
> This build was done by Alan Pope and Stefano Verzegnassi to prevent a bug
> that did not allow to install libsmbclien-dev in armf sdk, see
> https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287
> .
> --
>
> https://code.launchpad.net/~sam-sgrs/ubuntu-filemanager-app/fix-1579487/+merge/303622
> You are the owner of lp:~sam-sgrs/ubuntu-filemanager-app/fix-1579487.
>

Revision history for this message
Sam Segers (sam-sgrs) wrote :

This *should* fix having to remove the libs on the system itself discussed
in the link you gave.
But I can't test this on a device (that's why I fixed it for the desktop so
I could try the content store in Unity8).
So maybe it's better if someone else tries it?

Op wo 24 aug. 2016 06:39 schreef Sam Segers <email address hidden>:

> No. I didn't.
> I'll check this later today.
>
> Op wo 24 aug. 2016 00:49 schreef Carlos Jose Mazieri <
> <email address hidden>>:
>
> > Review: Needs Information
> >
> > Hello Sam,
> >
> > It is OK to me, just a question:
> >
> > Have you tested building for armf on device and building for armf on
> > qtcreator using a sdk like click-ubuntu-sdk-13.10-armhf?
> >
> > This build was done by Alan Pope and Stefano Verzegnassi to prevent a bug
> > that did not allow to install libsmbclien-dev in armf sdk, see
> >
> https://code.launchpad.net/~popey/ubuntu-filemanager-app/add-click-deps/+merge/270287
> > .
> > --
> >
> >
> https://code.launchpad.net/~sam-sgrs/ubuntu-filemanager-app/fix-1579487/+merge/303622
> > You are the owner of lp:~sam-sgrs/ubuntu-filemanager-app/fix-1579487.
> >
>
> --
>
> https://code.launchpad.net/~sam-sgrs/ubuntu-filemanager-app/fix-1579487/+merge/303622
> You are the owner of lp:~sam-sgrs/ubuntu-filemanager-app/fix-1579487.
>

Revision history for this message
Sam Segers (sam-sgrs) wrote :

It builds fine in the 15.04 armhf kit but don't have a device for tests.

Revision history for this message
Arto Jalkanen (ajalkane) wrote :

I'll try to find the time to build and test this on device before weekend is over.

Thank you in advance anyway for doing these fixes for desktop. It does look like it shouldn't cause problems, but there was such many iterations to get these samba libs working on phone devices that I wouldn't be too surprised if something would break :).

Revision history for this message
Carlos Jose Mazieri (carlos-mazieri) wrote :

Hello Arto and Sam,

The "apt-get install libsmbclient-dev" used to fail on any armf kit, so it was impossible to generate the click package in the qtcreator.

The build on device used to work because "apt-get install libsmbclient-dev" on device used to work.

That is the reason why Alan Pope and Stefano Verzegnassi came with solution of archiving samba libraries in the project.

Revision history for this message
Sam Segers (sam-sgrs) wrote :

But there are no samba headers in the project, so how is it building now if
thee Dev package can't be installed?
This change also downloads the development files the same way as the
included libs.

For the desktop, the main issue is the plugin loads libsmbclient, which in
turn loads numerous smb libs. Because libsmbclient is compiled with rpath
hard coded, it was loading the system smb libs which are xenial and the
bundled vivid.

So for the desktop only the rpath change is needed. Feel free to discard
the rest.

Op vr 26 aug. 2016 21:02 schreef Carlos Jose Mazieri <
<email address hidden>>:

> Hello Arto and Sam,
>
> The "apt-get install libsmbclient-dev" used to fail on any armf kit, so it
> was impossible to generate the click package in the qtcreator.
>
> The build on device used to work because "apt-get install
> libsmbclient-dev" on device used to work.
>
> That is the reason why Alan Pope and Stefano Verzegnassi came with
> solution of archiving samba libraries in the project.
>
> --
>
> https://code.launchpad.net/~sam-sgrs/ubuntu-filemanager-app/fix-1579487/+merge/303622
> You are the owner of lp:~sam-sgrs/ubuntu-filemanager-app/fix-1579487.
>

Unmerged revisions

568. By Sam Segers

Fix shared libraries on the desktop
The plugin uses the vivid samba dev packages as headers and the packaged libs to link to when in CLICK_MODE.
All the packaged libs also get stripped from the rpath with chrpath so that LD_LIBRARY_PATH works and the system libs don't get used.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt 2016-07-14 15:04:49 +0000
+++ CMakeLists.txt 2016-08-22 19:49:52 +0000
@@ -73,12 +73,16 @@
7373
74 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/manifest.json filemanager.apparmor ${CONTENT_HUB_JSON} DESTINATION ${CMAKE_INSTALL_PREFIX})74 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/manifest.json filemanager.apparmor ${CONTENT_HUB_JSON} DESTINATION ${CMAKE_INSTALL_PREFIX})
7575
76 set(UPSTREAM_LIBS_DIR ${CMAKE_BINARY_DIR}/upstream-libs)76 set(UPSTREAM_LIBS_DIR ${CMAKE_BINARY_DIR}/upstream-libs)
77 set(UPSTREAM_DEV_LIBS_DIR ${CMAKE_BINARY_DIR}/upstream-libs-dev)
7778
78 #grab dependencies79 #grab dependencies
79 set(GET_CLICK_DEPS_TOOL ${CMAKE_SOURCE_DIR}/get-click-deps)80 set(GET_CLICK_DEPS_TOOL ${CMAKE_SOURCE_DIR}/get-click-deps)
80 set(DEPS_MANIFEST ${CMAKE_CURRENT_SOURCE_DIR}/filemanager-libs.json)81 set(DEPS_MANIFEST ${CMAKE_CURRENT_SOURCE_DIR}/filemanager-libs.json)
82 set(DEV_DEPS_MANIFEST ${CMAKE_CURRENT_SOURCE_DIR}/filemanager-libs-dev.json)
81 MESSAGE("Grabbing upstream libs to ${UPSTREAM_LIBS_DIR}")83 MESSAGE("Grabbing upstream libs to ${UPSTREAM_LIBS_DIR}")
84 # DEV packages
85 MESSAGE("Grabbing upstream dev libs to ${UPSTREAM_DEV_LIBS_DIR}")
8286
83 get_filename_component(BLD_CONFIGURATION_NAME ${CMAKE_BINARY_DIR} NAME)87 get_filename_component(BLD_CONFIGURATION_NAME ${CMAKE_BINARY_DIR} NAME)
84 set(UPSTREAM_CACHE $ENV{HOME}/dev/upstream-libs-filemanager/${BLD_CONFIGURATION_NAME})88 set(UPSTREAM_CACHE $ENV{HOME}/dev/upstream-libs-filemanager/${BLD_CONFIGURATION_NAME})
@@ -87,21 +91,41 @@
87 if(EXISTS "${UPSTREAM_CACHE}")91 if(EXISTS "${UPSTREAM_CACHE}")
88 MESSAGE("Upstream libs cache exists.")92 MESSAGE("Upstream libs cache exists.")
89 file(COPY ${UPSTREAM_CACHE}/upstream-libs/ DESTINATION ${UPSTREAM_LIBS_DIR} PATTERN * )93 file(COPY ${UPSTREAM_CACHE}/upstream-libs/ DESTINATION ${UPSTREAM_LIBS_DIR} PATTERN * )
94 file(COPY ${UPSTREAM_CACHE}/upstream-libs-dev/ DESTINATION ${UPSTREAM_DEV_LIBS_DIR} PATTERN * )
90 else()95 else()
91 MESSAGE("Cache miss, downloading from network.")96 MESSAGE("Cache miss, downloading from network.")
92 file(MAKE_DIRECTORY ${UPSTREAM_LIBS_DIR})97 file(MAKE_DIRECTORY ${UPSTREAM_LIBS_DIR})
93 file(MAKE_DIRECTORY ${UPSTREAM_CACHE})98 file(MAKE_DIRECTORY ${UPSTREAM_CACHE})
99 file(MAKE_DIRECTORY ${UPSTREAM_DEV_LIBS_DIR})
100 file(MAKE_DIRECTORY ${UPSTREAM_CACHE})
94 execute_process(101 execute_process(
95 COMMAND ${GET_CLICK_DEPS_TOOL} -d ${DEPS_MANIFEST} ${CLICK_ARCH} ${UPSTREAM_LIBS_DIR}102 COMMAND ${GET_CLICK_DEPS_TOOL} -d ${DEPS_MANIFEST} ${CLICK_ARCH} ${UPSTREAM_LIBS_DIR}
96 )103 )
104 execute_process(
105 COMMAND ${GET_CLICK_DEPS_TOOL} -d ${DEV_DEPS_MANIFEST} ${CLICK_ARCH} ${UPSTREAM_DEV_LIBS_DIR}
106 )
107 # Copy the symlinks from the deb packages
108 file(COPY ${UPSTREAM_DEV_LIBS_DIR}/usr/lib/ DESTINATION ${UPSTREAM_LIBS_DIR}/usr/lib PATTERN * )
109
110 # Strip the rpath so the shared libraries don't load the desktop libraries
111 file(GLOB_RECURSE TO_STRIP "${CMAKE_CURRENT_BINARY_DIR}/upstream-libs/usr/lib/${ARCH_TRIPLET}/*")
112 foreach(ITEM ${TO_STRIP})
113 # Strip the rpath so the shared libraries don't load the desktop libraries but the packaged ones.
114 execute_process(
115 COMMAND "chrpath" -d ${ITEM}
116 )
117 endforeach()
97 # Cache for next usage118 # Cache for next usage
98 file(COPY ${UPSTREAM_LIBS_DIR} DESTINATION ${UPSTREAM_CACHE} )119 file(COPY ${UPSTREAM_LIBS_DIR} DESTINATION ${UPSTREAM_CACHE} )
120 file(COPY ${UPSTREAM_DEV_LIBS_DIR} DESTINATION ${UPSTREAM_CACHE} )
99 endif()121 endif()
122
100 MESSAGE("Installing upstream libs from ${CMAKE_CURRENT_BINARY_DIR}/upstream-libs/usr/lib/${ARCH_TRIPLET}/ to ${DATA_DIR}lib/${ARCH_TRIPLET}")123 MESSAGE("Installing upstream libs from ${CMAKE_CURRENT_BINARY_DIR}/upstream-libs/usr/lib/${ARCH_TRIPLET}/ to ${DATA_DIR}lib/${ARCH_TRIPLET}")
101 file(GLOB_RECURSE UPSTREAM_LIBS "${CMAKE_CURRENT_BINARY_DIR}/upstream-libs/usr/lib/${ARCH_TRIPLET}/*")124 file(GLOB_RECURSE UPSTREAM_LIBS "${CMAKE_CURRENT_BINARY_DIR}/upstream-libs/usr/lib/${ARCH_TRIPLET}/*")
102 foreach(ITEM ${UPSTREAM_LIBS})125 foreach(ITEM ${UPSTREAM_LIBS})
103 IF( IS_DIRECTORY "${ITEM}" )126 IF( IS_DIRECTORY "${ITEM}" )
104 LIST( APPEND DIRS_TO_DEPLOY "${ITEM}" )127 # There are non because of the GLOB_RECURES. All are get coppied in the main dir.
128 # LIST( APPEND DIRS_TO_DEPLOY "${ITEM}" )
105 ELSE()129 ELSE()
106 LIST( APPEND FILES_TO_DEPLOY "${ITEM}" )130 LIST( APPEND FILES_TO_DEPLOY "${ITEM}" )
107 ENDIF()131 ENDIF()
108132
=== modified file 'src/plugin/folderlistmodel/CMakeLists.txt'
--- src/plugin/folderlistmodel/CMakeLists.txt 2015-12-12 14:40:30 +0000
+++ src/plugin/folderlistmodel/CMakeLists.txt 2016-08-22 19:49:52 +0000
@@ -105,14 +105,23 @@
105105
106qt5_use_modules(nemofolderlistmodel Gui Qml Quick Widgets)106qt5_use_modules(nemofolderlistmodel Gui Qml Quick Widgets)
107107
108## samba requires libsmbclient108if(CLICK_MODE)
109find_path(SAMBA_INCLUDE_DIR 109 set(SAMBA_INCLUDE_DIR ${UPSTREAM_DEV_LIBS_DIR}/usr/include/samba-4.0)
110 NAMES libsmbclient.h 110 find_library(SAMBA_LIBRARIES
111 HINTS /usr/include/smbclient /usr/include/samba /usr/include/samba-3.0 /usr/include/samba-4.0111 NAMES smbclient
112 )112 PATHS ${UPSTREAM_LIBS_DIR}/usr/lib/${ARCH_TRIPLET}
113find_library(SAMBA_LIBRARIES NAMES smbclient )113 NO_DEFAULT_PATH)
114else(CLICK_MODE)
115 ## samba requires libsmbclient
116 find_path(SAMBA_INCLUDE_DIR
117 NAMES libsmbclient.h
118 HINTS /usr/include/smbclient /usr/include/samba /usr/include/samba-3.0 /usr/include/samba-4.0
119 )
120 find_library(SAMBA_LIBRARIES NAMES smbclient)
121endif(CLICK_MODE)
122
114message(STATUS "samba include=${SAMBA_INCLUDE_DIR}")123message(STATUS "samba include=${SAMBA_INCLUDE_DIR}")
115message(STATUS "samba lib=${SAMBA_LIBRARIES}=${SAMBA_LIBRARIES}")124message(STATUS "samba lib=${SAMBA_LIBRARIES}")
116125
117if(SAMBA_INCLUDE_DIR AND SAMBA_LIBRARIES)126if(SAMBA_INCLUDE_DIR AND SAMBA_LIBRARIES)
118 message(STATUS "Found samba: include=${SAMBA_INCLUDE_DIR} library=${SAMBA_LIBRARIES}")127 message(STATUS "Found samba: include=${SAMBA_INCLUDE_DIR} library=${SAMBA_LIBRARIES}")

Subscribers

People subscribed via source and target branches