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

Proposed by Sam Segers on 2016-08-22
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 2016-08-22 Needs Information on 2016-08-23
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.

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

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

Sam Segers (sam-sgrs) wrote :

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

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

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.

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 on 2016-08-22

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
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-07-14 15:04:49 +0000
3+++ CMakeLists.txt 2016-08-22 19:49:52 +0000
4@@ -73,12 +73,16 @@
5
6 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/manifest.json filemanager.apparmor ${CONTENT_HUB_JSON} DESTINATION ${CMAKE_INSTALL_PREFIX})
7
8- set(UPSTREAM_LIBS_DIR ${CMAKE_BINARY_DIR}/upstream-libs)
9+ set(UPSTREAM_LIBS_DIR ${CMAKE_BINARY_DIR}/upstream-libs)
10+ set(UPSTREAM_DEV_LIBS_DIR ${CMAKE_BINARY_DIR}/upstream-libs-dev)
11
12 #grab dependencies
13 set(GET_CLICK_DEPS_TOOL ${CMAKE_SOURCE_DIR}/get-click-deps)
14 set(DEPS_MANIFEST ${CMAKE_CURRENT_SOURCE_DIR}/filemanager-libs.json)
15+ set(DEV_DEPS_MANIFEST ${CMAKE_CURRENT_SOURCE_DIR}/filemanager-libs-dev.json)
16 MESSAGE("Grabbing upstream libs to ${UPSTREAM_LIBS_DIR}")
17+ # DEV packages
18+ MESSAGE("Grabbing upstream dev libs to ${UPSTREAM_DEV_LIBS_DIR}")
19
20 get_filename_component(BLD_CONFIGURATION_NAME ${CMAKE_BINARY_DIR} NAME)
21 set(UPSTREAM_CACHE $ENV{HOME}/dev/upstream-libs-filemanager/${BLD_CONFIGURATION_NAME})
22@@ -87,21 +91,41 @@
23 if(EXISTS "${UPSTREAM_CACHE}")
24 MESSAGE("Upstream libs cache exists.")
25 file(COPY ${UPSTREAM_CACHE}/upstream-libs/ DESTINATION ${UPSTREAM_LIBS_DIR} PATTERN * )
26+ file(COPY ${UPSTREAM_CACHE}/upstream-libs-dev/ DESTINATION ${UPSTREAM_DEV_LIBS_DIR} PATTERN * )
27 else()
28 MESSAGE("Cache miss, downloading from network.")
29 file(MAKE_DIRECTORY ${UPSTREAM_LIBS_DIR})
30 file(MAKE_DIRECTORY ${UPSTREAM_CACHE})
31+ file(MAKE_DIRECTORY ${UPSTREAM_DEV_LIBS_DIR})
32+ file(MAKE_DIRECTORY ${UPSTREAM_CACHE})
33 execute_process(
34 COMMAND ${GET_CLICK_DEPS_TOOL} -d ${DEPS_MANIFEST} ${CLICK_ARCH} ${UPSTREAM_LIBS_DIR}
35 )
36+ execute_process(
37+ COMMAND ${GET_CLICK_DEPS_TOOL} -d ${DEV_DEPS_MANIFEST} ${CLICK_ARCH} ${UPSTREAM_DEV_LIBS_DIR}
38+ )
39+ # Copy the symlinks from the deb packages
40+ file(COPY ${UPSTREAM_DEV_LIBS_DIR}/usr/lib/ DESTINATION ${UPSTREAM_LIBS_DIR}/usr/lib PATTERN * )
41+
42+ # Strip the rpath so the shared libraries don't load the desktop libraries
43+ file(GLOB_RECURSE TO_STRIP "${CMAKE_CURRENT_BINARY_DIR}/upstream-libs/usr/lib/${ARCH_TRIPLET}/*")
44+ foreach(ITEM ${TO_STRIP})
45+ # Strip the rpath so the shared libraries don't load the desktop libraries but the packaged ones.
46+ execute_process(
47+ COMMAND "chrpath" -d ${ITEM}
48+ )
49+ endforeach()
50 # Cache for next usage
51 file(COPY ${UPSTREAM_LIBS_DIR} DESTINATION ${UPSTREAM_CACHE} )
52+ file(COPY ${UPSTREAM_DEV_LIBS_DIR} DESTINATION ${UPSTREAM_CACHE} )
53 endif()
54+
55 MESSAGE("Installing upstream libs from ${CMAKE_CURRENT_BINARY_DIR}/upstream-libs/usr/lib/${ARCH_TRIPLET}/ to ${DATA_DIR}lib/${ARCH_TRIPLET}")
56 file(GLOB_RECURSE UPSTREAM_LIBS "${CMAKE_CURRENT_BINARY_DIR}/upstream-libs/usr/lib/${ARCH_TRIPLET}/*")
57 foreach(ITEM ${UPSTREAM_LIBS})
58 IF( IS_DIRECTORY "${ITEM}" )
59- LIST( APPEND DIRS_TO_DEPLOY "${ITEM}" )
60+ # There are non because of the GLOB_RECURES. All are get coppied in the main dir.
61+ # LIST( APPEND DIRS_TO_DEPLOY "${ITEM}" )
62 ELSE()
63 LIST( APPEND FILES_TO_DEPLOY "${ITEM}" )
64 ENDIF()
65
66=== modified file 'src/plugin/folderlistmodel/CMakeLists.txt'
67--- src/plugin/folderlistmodel/CMakeLists.txt 2015-12-12 14:40:30 +0000
68+++ src/plugin/folderlistmodel/CMakeLists.txt 2016-08-22 19:49:52 +0000
69@@ -105,14 +105,23 @@
70
71 qt5_use_modules(nemofolderlistmodel Gui Qml Quick Widgets)
72
73-## samba requires libsmbclient
74-find_path(SAMBA_INCLUDE_DIR
75- NAMES libsmbclient.h
76- HINTS /usr/include/smbclient /usr/include/samba /usr/include/samba-3.0 /usr/include/samba-4.0
77- )
78-find_library(SAMBA_LIBRARIES NAMES smbclient )
79+if(CLICK_MODE)
80+ set(SAMBA_INCLUDE_DIR ${UPSTREAM_DEV_LIBS_DIR}/usr/include/samba-4.0)
81+ find_library(SAMBA_LIBRARIES
82+ NAMES smbclient
83+ PATHS ${UPSTREAM_LIBS_DIR}/usr/lib/${ARCH_TRIPLET}
84+ NO_DEFAULT_PATH)
85+else(CLICK_MODE)
86+ ## samba requires libsmbclient
87+ find_path(SAMBA_INCLUDE_DIR
88+ NAMES libsmbclient.h
89+ HINTS /usr/include/smbclient /usr/include/samba /usr/include/samba-3.0 /usr/include/samba-4.0
90+ )
91+ find_library(SAMBA_LIBRARIES NAMES smbclient)
92+endif(CLICK_MODE)
93+
94 message(STATUS "samba include=${SAMBA_INCLUDE_DIR}")
95-message(STATUS "samba lib=${SAMBA_LIBRARIES}=${SAMBA_LIBRARIES}")
96+message(STATUS "samba lib=${SAMBA_LIBRARIES}")
97
98 if(SAMBA_INCLUDE_DIR AND SAMBA_LIBRARIES)
99 message(STATUS "Found samba: include=${SAMBA_INCLUDE_DIR} library=${SAMBA_LIBRARIES}")

Subscribers

People subscribed via source and target branches