Merge lp://staging/~mterry/mediascanner2/only-in-u8 into lp://staging/mediascanner2

Proposed by Michael Terry
Status: Merged
Approved by: Pete Woods
Approved revision: 343
Merged at revision: 343
Proposed branch: lp://staging/~mterry/mediascanner2/only-in-u8
Merge into: lp://staging/mediascanner2
Diff against target: 42 lines (+25/-0)
1 file modified
src/daemon/scannerdaemon.cc (+25/-0)
To merge this branch: bzr merge lp://staging/~mterry/mediascanner2/only-in-u8
Reviewer Review Type Date Requested Status
Pete Woods (community) Approve
unity-api-1-bot continuous-integration Approve
Review via email: mp+320505@code.staging.launchpad.net

Commit message

Only run mediascanner in unity8.

Description of the change

As part of the MIR for mediascanner2 (bug 1669546), it was noted that mediascanner2 caused some issues with unity7 (scanning thumb drives when the user is trying to eject and some such). So to avoid that for now and to get mediascanner2 into main for zesty, here's a patch to only run mediascanner2 in unity8.

I've chosen to use a runtime function to check the environment because (A) the environment checking is not trivial and (B) systemd units don't seem to be as easy to say "only run in unity7" as upstart jobs.

I've created the new environment env MEDIASCANNER_RUN=1 if someone would want to manually override this check.

I've only guarded the main scanner daemon, since that's the bit that is unconditionally run and likely to conflict with thumb drives. The extractor and dbus interface both seem harmless (and only activated upon request). Is that accurate?

I didn't add any tests, since none of the current tests try running the daemon itself. But I manually tested.

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:343
https://jenkins.canonical.com/unity-api-1/job/lp-mediascanner2-ci/20/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1851
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1858
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1634
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1634/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1634
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1634/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1634
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1634/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1634
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1634/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1634
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1634/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1634
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1634/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote :

After writing this, I was pointed at the systemd key ExecStartPre=, which could be used to move this check into our systemd unit file, if that is viewed as cleaner.

I still like this in-code method better, because it allows the logic to be consolidated (between upstart and systemd) and more verbose/better commented.

Plus, for visual cleanliness reasons, I'd be tempted to simplify the potential systemd logic to not be so strict about matching Unity8 exactly in XDG_CURRENT_DESKTOP.

But if you like that approach better, I can retool this MP.

Revision history for this message
Pete Woods (pete-woods) wrote :

Definitely agree with codifying stuff like that, as you can add unit tests for it.

Revision history for this message
Pete Woods (pete-woods) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches