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

[Michael Terry]

Only run mediascanner in unity8.

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.

[unity-api-1-bot (unity-api-1-bot)]

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)

[Michael Terry (mterry)]

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.

[Pete Woods (pete-woods)]

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

[Pete Woods (pete-woods)]

review: Approve

=== modified file 'src/daemon/scannerdaemon.cc'
--- src/daemon/scannerdaemon.cc	2016-08-30 09:58:38 +0000
+++ src/daemon/scannerdaemon.cc	2017-03-21 15:00:44 +0000
@@ -202,6 +202,30 @@
     }
 }
 
+static void validate_desktop() {
+    // Only set manually
+    const gchar *ms_run_env = g_getenv("MEDIASCANNER_RUN");
+    if (g_strcmp0(ms_run_env, "1") == 0)
+        return;
+
+    // Only set properly in 17.04 and onward
+    const gchar *desktop_env = g_getenv("XDG_CURRENT_DESKTOP");
+    if (g_regex_match_simple("(^|:)Unity8(:|$)", desktop_env,
+                             (GRegexCompileFlags)0, (GRegexMatchFlags)0))
+        return;
+
+    // Shouldn't rely on this, but we only need to use it for 16.04 - 17.04
+    const gchar *session_env = g_getenv("XDG_SESSION_DESKTOP");
+    if (g_strcmp0("unity8", session_env) == 0)
+        return;
+
+    // We shouldn't run if we weren't asked for; we can confuse some desktops
+    // (like unity7) with our scanning of mounted drives and the like.
+    printf("Mediascanner service not starting due to unsupported desktop environment.\n");
+    printf("Set MEDIASCANNER_RUN=1 to override this.\n");
+    exit(0);
+}
+
 static void print_banner() {
     char timestr[200];
     time_t t;
@@ -223,6 +247,7 @@
 }
 
 int main(int /*argc*/, char **/*argv*/) {
+    validate_desktop();
     print_banner();
 
     try {