Unity Media Scanner Scope

Merge lp://qastaging/~jamesh/unity-scope-mediascanner/click-support into lp://qastaging/unity-scope-mediascanner

click-support
Merge into trunk

Proposed by James Henstridge on 2014-10-09

Status:

Merged

Approved by:

Paweł Stołowski on 2014-10-10

Approved revision:

193

Merged at revision:

196

Proposed branch:

lp://qastaging/~jamesh/unity-scope-mediascanner/click-support

Merge into:

lp://qastaging/unity-scope-mediascanner

Diff against target:

1076 lines (+452/-170)

38 files modified

CMakeLists.txt (+16/-10)
cmake/ClickBuild.cmake (+43/-0)
cmake/ClickScope.cmake (+33/-0)
cmake/Intltool.cmake (+17/-0)
config.h.in (+3/-0)
data/CMakeLists.txt (+0/-49)
po/CMakeLists.txt (+20/-10)
src/CMakeLists.txt (+0/-50)
src/musicaggregator/CMakeLists.txt (+39/-0)
src/musicaggregator/apparmor.json (+5/-0)
src/musicaggregator/manifest.json.in (+16/-0)
src/musicaggregator/musicaggregator.ini.in (+3/-7)
src/musicaggregator/musicaggregatorquery.cpp (+3/-3)
src/musicaggregator/musicaggregatorscope.cpp (+7/-1)
src/musicaggregator/onlinemusicresultforwarder.h (+1/-1)
src/mymusic/CMakeLists.txt (+29/-0)
src/mymusic/apparmor.json (+5/-0)
src/mymusic/manifest.json.in (+16/-0)
src/mymusic/mediascanner-music.ini.in (+2/-6)
src/mymusic/music-scope.cpp (+2/-2)
src/myvideos/CMakeLists.txt (+29/-0)
src/myvideos/apparmor.json (+5/-0)
src/myvideos/manifest.json.in (+16/-0)
src/myvideos/mediascanner-video.ini.in (+2/-5)
src/myvideos/video-scope.cpp (+2/-2)
src/utils/CMakeLists.txt (+11/-0)
src/utils/i18n.cpp (+32/-0)
src/utils/i18n.h (+8/-0)
src/videoaggregator/CMakeLists.txt (+37/-0)
src/videoaggregator/apparmor.json (+5/-0)
src/videoaggregator/manifest.json.in (+16/-0)
src/videoaggregator/videoaggregator.ini.in (+3/-6)
src/videoaggregator/videoaggregatorquery.cpp (+3/-3)
src/videoaggregator/videoaggregatorscope.cpp (+7/-1)
tests/CMakeLists.txt (+9/-9)
tests/test-music-scope.cpp (+2/-1)
tests/test-result-forwarder.cpp (+3/-3)
tests/test-video-scope.cpp (+2/-1)

To merge this branch:

bzr merge lp://qastaging/~jamesh/unity-scope-mediascanner/click-support

Undecided

Invalid

Link a bug report

Reviewer	Review Type	Date Requested	Status
Paweł Stołowski (community)		2014-10-09	Approve on 2014-10-10
PS Jenkins bot (community)	continuous-integration		Approve on 2014-10-10
Review via email: mp+237731@code.qastaging.launchpad.net

Commit message

Reorganise package and update build rules to support building scopes as click packages. Also update icons to the latest versions from design.

Description of the change

Reorganise package to allow for building as click packages.

By default, it will build as before (as is happening in Jenkins). The click packages can be built with something like:

    mkdir build
    cd build
    cmake .. -DCMAKE_INSTALL_PREFIX=`pwd`/install -DCLICK_MODE=ON
    make
    make install
    make click

This branch moves the data files under $libdir even for the non-click builds, since that reduced the delta between the two builds.

The icons for some of the scopes have also been updated to match the ones from design (previously it was using the same icons for the local media and aggregator scopes, for instance).

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-09:

FAILED: Continuous integration, rev:191
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~jamesh/unity-scope-mediascanner/click-support/+merge/237731/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-ci/154/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-amd64-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-armhf-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-i386-ci/56

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-mediascanner-ci/154/rebuild

review: Needs Fixing (continuous-integration)

lp://qastaging/~jamesh/unity-scope-mediascanner/click-support updated on 2014-10-09

192. By James Henstridge on 2014-10-09: Fix up Icon setting for "My Music" scope.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-09:

FAILED: Continuous integration, rev:192
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~jamesh/unity-scope-mediascanner/click-support/+merge/237731/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-ci/155/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-amd64-ci/57
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-armhf-ci/57
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-i386-ci/57

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-mediascanner-ci/155/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2014-10-09:

Looks good overall and the scopes work when installed alongside existing ones on the phone, however a couple of comments:

488 +#ifdef CLICK_MODE
489 +const std::string MusicAggregatorScope::LOCALSCOPE = "com.ubuntu.scopes.mymusic_mymusic";
490 +#else
491 const std::string MusicAggregatorScope::LOCALSCOPE = "mediascanner-music";
492 +#endif

I think we can't do this, we won't be able to address this scope from the outside - favorites come to mind. Aggregator ids needs to be updated in unity-schemas package (com.canonical.Unity.gschema.xml). One potential problem we will have with this is if user updated his favorites (very likely to happen), in which case he will have own copy of this dconf key.

The ids also affect results invalidation when mediascanner reports new content: the following code needs to be adjusted in unity-scopes-shell:

void Scopes::invalidateScopeResults(QString const& scopeName)
{
    // HACK! mediascanner invalidates local media scopes, but those are aggregated, so let's "forward" the call
    if (scopeName == "mediascanner-music") {
        invalidateScopeResults("musicaggregator");
    } else if (scopeName == "mediascanner-video") {
        invalidateScopeResults("videoaggregator");
    } else if (scopeName == "smart-scopes") {

So, landing of this change should be accompanied by and synchronised with unity-scopes-shell and unity-schemas MPs, but I'm open to a different plan for that migration.

Also, while you are at it - could you please update the screenshots? They show very outdated look of both media and video scopes. Thanks!

review: Needs Fixing

Revision history for this message

James Henstridge (jamesh) wrote on 2014-10-10:

Note that the default build here is with CLICK_MODE=OFF, which is used to build a Debian package with the old scope names. I agree that we will need to make more changes to actually migrate over to using click packaged versions of the scopes in the image, but they aren't a prerequisite for merging this branch. This MP is a 1000+ lines of diff, and I'd prefer to get this merged sooner rather than later.

You can see in the build logs that the package is being built with the old names here: https://jenkins.qa.ubuntu.com/job/unity-scope-mediascanner-utopic-armhf-ci/57/console

Also, we can't avoid changing the scope IDs while migrating to click packages: the scope IDs use a package name as a prefix, after all.

I'll redo the screenshots as requested.

lp://qastaging/~jamesh/unity-scope-mediascanner/click-support updated on 2014-10-10