Merge lp:~michihenning/thumbnailer/snap into lp:thumbnailer/devel

Proposed by Michi Henning
Status: Merged
Approved by: James Henstridge
Approved revision: 371
Merged at revision: 369
Proposed branch: lp:~michihenning/thumbnailer/snap
Merge into: lp:thumbnailer/devel
Diff against target: 127 lines (+90/-5)
4 files modified
.bzrignore (+5/-1)
snapcraft.yaml (+80/-0)
tests/copyright/check_copyright.sh (+1/-1)
tests/whitespace/CMakeLists.txt (+4/-3)
To merge this branch: bzr merge lp:~michihenning/thumbnailer/snap
Reviewer Review Type Date Requested Status
James Henstridge Approve
unity-api-1-bot continuous-integration Approve
Review via email: mp+315538@code.launchpad.net

Commit message

First stab at snapcraft.yaml.

Description of the change

First stab at snapcraft.yaml.

It's not clear yet whether we really want to use the generic DBus interface or add our own interface to snapd. For now, we use the generic one; this works, in that thumbnailer-admin can talk to the (hand-started) thumbnailer-service. Presumably, that's because both are in the same snap. Next step will be to try with thumbnailer-admin in a separate snap.

To try this (from the top-level dir):

$ snapcraft
$ thumbnailer.thumbnailer-service &
$ thumbnailer.thumbnailer-admin stats

To post a comment you must log in.
Revision history for this message
James Henstridge (jamesh) wrote :

One small problem, and a few questions.

review: Needs Fixing
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :

Replied inline, thanks!

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

Looks good. I am a bit concerned about the "control" slot you're exposing though: the dbus interface auto connects, and I don't think we want this one auto-connecting.

Also, we don't ever acquire the name "com.canonical.ThumbnailerAdmin", so that doesn't sound quite right either.

I wonder if there is anything to allow snaps to communicate with other binaries from the same snap via dbus? If so, then we wouldn't even need this second slot.

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

I've added a few inline comments.

These are the changes I made while testing out the snap: http://paste.ubuntu.com/23873664/. With those changes, I tried using my second snap to access the thumbnailer:

  $ sudo snap install --dangerous thumbnailer-admin-test_0.1_amd64.snap
  $ sudo connect thumbnailer-admin-test:thumbnailer thumbnailer:thumbnailer
  $ thumbnailer-admin-test.admin get-album Radiohead 'Ok Computer'
  thumbnailer-admin: Handler::createFinished(): could not get thumbnail for album: Radiohead/Ok Computer (0,0): ERROR

On the service side, I got the following:

  $ thumbnailer.thumbnailer-service
  thumbnailer-service: [15:57:10.144] Initializing
  thumbnailer-service: [15:57:10.440] image cache: 0 entries, 0 bytes, hit rate 0.00 (0/0), avg hit run 0.00, avg miss run 0.00
  thumbnailer-service: [15:57:10.440] thumbnail cache: 0 entries, 0 bytes, hit rate 0.00 (0/0), avg hit run 0.00, avg miss run 0.00
  thumbnailer-service: [15:57:10.440] failure cache: 0 entries, 0 bytes, hit rate 0.00 (0/0), avg hit run 0.00, avg miss run 0.00

  (process:13511): GdkPixbuf-WARNING **: Cannot open pixbuf loader module file '/home/james/snap/thumbnailer/x1/.cache/gdk-pixbuf-loaders.cache': No such file or directory

  This likely means that your installation is broken.
  Try running the command
    gdk-pixbuf-query-loaders > /home/james/snap/thumbnailer/x1/.cache/gdk-pixbuf-loaders.cache
to make things work again for the time being.

  (process:13511): GdkPixbuf-WARNING **: Cannot open pixbuf loader module file '/home/james/snap/thumbnailer/x1/.cache/gdk-pixbuf-loaders.cache': No such file or directory

  This likely means that your installation is broken.
  Try running the command
    gdk-pixbuf-query-loaders > /home/james/snap/thumbnailer/x1/.cache/gdk-pixbuf-loaders.cache
to make things work again for the time being.
  thumbnailer-service: [15:57:30.227] Warning: "Handler::createFinished(): could not get thumbnail for album: Radiohead/Ok Computer (0,0): ERROR"

So it is still not quite right. It looks like there is some support for gdk-pixbuf in the desktop-launch script, but it only comes into play if we ship gdk-pixbuf plus loaders as part of our snap.

review: Needs Fixing
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:371
https://jenkins.canonical.com/unity-api-1/job/lp-thumbnailer-ci/19/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1535
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1542
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1320
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1320/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1320
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1320/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1320
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1320/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1320
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1320/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1320
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1320/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1320
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1320/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-thumbnailer-ci/19/rebuild

review: Approve (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

Okay, I'm now able to build a thumbnailer snap that can talk to the thumbnailer-admin-test snap without any post install fixups.

To make sure the new version of the part is used, I rebuilt with:

    snapcraft clean
    snapcraft update
    snapcraft

(The "update" operation downloads a new copy of parts.yaml).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '.bzrignore'
--- .bzrignore 2013-09-20 14:31:57 +0000
+++ .bzrignore 2017-01-30 03:06:38 +0000
@@ -1,4 +1,8 @@
1build1./build
2.project2.project
3.cproject3.cproject
4.settings4.settings
5./parts
6./prime
7./stage
8./thumbnailer*.snap
59
=== added file 'snapcraft.yaml'
--- snapcraft.yaml 1970-01-01 00:00:00 +0000
+++ snapcraft.yaml 2017-01-30 03:06:38 +0000
@@ -0,0 +1,80 @@
1name: thumbnailer
2version: '2.4'
3summary: Daemon and admin tool to create and cache image thumbnails for media files
4description: >
5 This snap provides thumbnailer-service, a DBus daemon that extracts thumbnails from
6 local media files and retrieves music album artwork from dash.ubuntu.com. It also
7 includes thumbnailer-admin, a command-line utility that allows for basic cache
8 maintenance and testing. See lp:thumbnailer for more detail.
9grade: devel
10confinement: strict
11
12slots:
13 thumbnailer:
14 interface: dbus
15 name: com.canonical.Thumbnailer
16 bus: session
17
18plugs:
19 platform:
20 interface: content
21 content: ubuntu-app-platform1
22 target: ubuntu-app-platform
23 default-provider: ubuntu-app-platform
24 network-manager:
25 interface: network-manager
26 thumbnails:
27 interface: dbus
28 name: com.canonical.Thumbnailer
29 bus: session
30
31apps:
32 thumbnailer-service:
33 command: desktop-launch $SNAP/lib/thumbnailer/thumbnailer-service
34 # TODO: Currently cannot use a simple daemon with the session bus. For now,
35 # the service must be started by hand. See https://github.com/snapcore/snapd/pull/2592
36 # daemon: simple
37 plugs:
38 - platform
39 - network
40 # TODO: This grants more privileges than we really need but, without this, apparmor
41 # denies access.
42 - network-manager
43 # TODO: The gsettings interface currently does not permit reading.
44 # See https://bugs.launchpad.net/snappy/+bug/1600154
45 # For now, the service uses default settings because it's gsettings lookups fail.
46 - gsettings
47 slots:
48 - thumbnailer
49 thumbnailer-admin:
50 command: desktop-launch $SNAP/bin/thumbnailer-admin
51 plugs:
52 - platform
53 - thumbnails
54
55parts:
56 thumbnailer:
57 plugin: cmake
58 source: .
59 after:
60 - desktop-ubuntu-app-platform
61 filesets:
62 binaries:
63 - bin/*
64 - lib/thumbnailer/*
65 man:
66 - share/man/*
67 etc:
68 - etc/*
69 share:
70 # TODO: This does not seem to have any effect. It is unclear right now how gsettings
71 # schemas need to be installed.
72 - share/glib-2.0/*
73 prime:
74 - $binaries
75 - $man
76 - $etc
77 - $share
78 - ubuntu-app-platform
79 install: |
80 mkdir $SNAPCRAFT_PART_INSTALL/ubuntu-app-platform
081
=== modified file 'tests/copyright/check_copyright.sh'
--- tests/copyright/check_copyright.sh 2016-05-30 09:44:33 +0000
+++ tests/copyright/check_copyright.sh 2017-01-30 03:06:38 +0000
@@ -31,7 +31,7 @@
31[ $# -lt 1 ] && usage31[ $# -lt 1 ] && usage
32[ $# -gt 2 ] && usage32[ $# -gt 2 ] && usage
3333
34ignore_pat="\\.sci$|\\.swp$|\\.bzr|debian|qmldir|HACKING|tsan-suppress|valgrind-suppress|testsong_ogg|\\.txt$|\\.xml$|\\.in$|\\.dox$|\\.html$|\\.map$|\\.aiff$|\\.gif$|\\.jpg$|\\.JPG$|\\.mp2$|\\.mp3$|\\.m4a$|\\.m4v$|\\.flac$|\\.oga$|\\.ogg$|\\.opus$|\\.png$|\\.svg$|\\.wav$|UseGSettings.cmake$"34ignore_pat="\\.sci$|\\.swp$|\\.bzr|debian|qmldir|HACKING|tsan-suppress|valgrind-suppress|testsong_ogg|\\.txt$|\\.xml$|\\.in$|\\.dox$|\\.html$|\\.map$|\\.aiff$|\\.gif$|\\.jpg$|\\.JPG$|\\.mp2$|\\.mp3$|\\.m4a$|\\.m4v$|\\.flac$|\\.oga$|\\.ogg$|\\.opus$|\\.png$|\\.svg$|\\.wav$|UseGSettings.cmake$|\\.yaml$|parts|prime|stage|~[0-9]+~$"
3535
36#36#
37# We don't use the -i option of licensecheck to add ignore_dir to the pattern because Jenkins creates directories37# We don't use the -i option of licensecheck to add ignore_dir to the pattern because Jenkins creates directories
3838
=== modified file 'tests/whitespace/CMakeLists.txt'
--- tests/whitespace/CMakeLists.txt 2015-05-22 03:13:47 +0000
+++ tests/whitespace/CMakeLists.txt 2017-01-30 03:06:38 +0000
@@ -3,7 +3,8 @@
3#3#
44
5add_test(whitespace5add_test(whitespace
6 ${CMAKE_CURRENT_SOURCE_DIR}/check_whitespace.py6 ${CMAKE_CURRENT_SOURCE_DIR}/check_whitespace.py ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}
7 ${CMAKE_SOURCE_DIR}7 ${CMAKE_SOURCE_DIR}/parts
8 ${CMAKE_BINARY_DIR}8 ${CMAKE_SOURCE_DIR}/stage
9 ${CMAKE_BINARY_DIR}/prime
9)10)

Subscribers

People subscribed via source and target branches

to all changes: