Merge lp:~ted/ubuntu-app-launch/reenable-only-show-in into lp:ubuntu-app-launch

[Ted Gould]

Reenable OnlyShowIn checking for Desktop files

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:230
http://jenkins.qa.ubuntu.com/job/ubuntu-app-launch-ci/51/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-app-launch-wily-amd64-ci/51
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-app-launch-wily-armhf-ci/51
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-app-launch-wily-i386-ci/51

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-app-launch-ci/51/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:231
http://jenkins.qa.ubuntu.com/job/ubuntu-app-launch-ci/54/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-app-launch-wily-amd64-ci/54
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-app-launch-wily-armhf-ci/54
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-app-launch-wily-i386-ci/54

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-app-launch-ci/54/rebuild

review: Approve (continuous-integration)

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

FAILED: Continuous integration, rev:231
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/3/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/111/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/118/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/3/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:231
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/5/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/113/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/120
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/72
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial+overlay/72
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/72
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/55/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/55/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/55/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/55/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/55
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/55/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/55
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/55/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/55/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/55/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/55/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/5/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:232
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/22/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/283/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/289
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/223
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial+overlay/223
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/223
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/152/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/152
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/152/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/152
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/152/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/152/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/152/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/152/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/152/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/152
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/152/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/152
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/152/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/22/rebuild

review: Needs Fixing (continuous-integration)

[Larry Price (larryprice)]

hooray! looks good to me

review: Approve

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

FAILED: Continuous integration, rev:233
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/103/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/674/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/680/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/103/rebuild

review: Needs Fixing (continuous-integration)

[Iain Lane (laney)]

You need to split the XDG_CURRENT_DESKTOP value on ":" and look at each part separately, I'm afraid.

See https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html "OnlyShowIn".

review: Needs Fixing

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

FAILED: Continuous integration, rev:233
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/109/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1008/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1015/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/109/rebuild

review: Needs Fixing (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

Text conflict in tests/application-info-desktop.cpp

review: Needs Fixing

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

FAILED: Continuous integration, rev:233
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/113/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1021/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1028/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/113/rebuild

review: Needs Fixing (continuous-integration)

[lp:~ted/ubuntu-app-launch/reenable-only-show-in]

234. By Ted Gould on 2017-01-12

Merge trunk

235. By Ted Gould on 2017-01-12

Update to new Desktop object

236. By Ted Gould on 2017-01-12

Try to split out CURRENT_DESKTOP

237. By Ted Gould on 2017-01-12

First pass doing set comparisons of the lists

238. By Ted Gould on 2017-01-12

Switch to find_first_of

239. By Ted Gould on 2017-01-12

Typo on name

240. By Ted Gould on 2017-01-12

Fix variable name

241. By Ted Gould on 2017-01-13

Add some more tests for sets

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

FAILED: Continuous integration, rev:241
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/166/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1363/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1370
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1148/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1148
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1148/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1148
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1148/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1148
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1148/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1148
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1148/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1148
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1148/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/166/rebuild

review: Needs Fixing (continuous-integration)

[Charles Kerr (charlesk)]

Minor suggestions & random drunken comments inline

review: Approve

=== modified file 'libubuntu-app-launch/application-info-desktop.cpp'
--- libubuntu-app-launch/application-info-desktop.cpp	2016-09-14 16:42:20 +0000
+++ libubuntu-app-launch/application-info-desktop.cpp	2017-01-13 03:03:05 +0000
@@ -20,6 +20,7 @@
 #include "application-info-desktop.h"
 #include "application-icon-finder.h"
 #include "registry-impl.h"
+#include <algorithm>
 #include <cstdlib>
 
 namespace ubuntu
@@ -30,6 +31,25 @@
 {
 namespace
 {
+
+static std::set<std::string> strvToSet(gchar** strv)
+{
+    std::set<std::string> retval;
+
+    if (strv != nullptr)
+    {
+        for (int i = 0; strv[i] != nullptr; i++)
+        {
+            if (strv[i][0] != '\0')
+            {
+                retval.emplace(strv[i]);
+            }
+        }
+    }
+
+    return retval;
+}
+
 constexpr const char* DESKTOP_GROUP = "Desktop Entry";
 
 struct TypeTag;
@@ -224,6 +244,21 @@
     return result;
 }
 
+std::set<std::string> stringlistFromKeyfileSet(const std::shared_ptr<GKeyFile>& keyfile, const gchar* key)
+{
+    GError* error = nullptr;
+    auto results = g_key_file_get_string_list(keyfile.get(), DESKTOP_GROUP, key, nullptr, &error);
+    if (error != nullptr)
+    {
+        g_error_free(error);
+        return {};
+    }
+
+    auto retval = strvToSet(results);
+    g_strfreev(results);
+    return retval;
+}
+
 Desktop::Desktop(const std::shared_ptr<GKeyFile>& keyfile,
                  const std::string& basePath,
                  const std::string& rootDir,
@@ -250,12 +285,21 @@
         auto xdg_current_desktop = getenv("XDG_CURRENT_DESKTOP");
         if (xdg_current_desktop != nullptr)
         {
-            if (stringlistFromKeyfileContains(keyfile, "NotShowIn", xdg_current_desktop, false) ||
-                !stringlistFromKeyfileContains(keyfile, "OnlyShowIn", xdg_current_desktop, true))
+            /* Split the CURRENT_DESKTOP by colons if there are multiple */
+            auto current_desktops = g_strsplit(xdg_current_desktop, ":", -1);
+            auto cdesktops = strvToSet(current_desktops);
+            g_strfreev(current_desktops);
+
+            auto onlyshowin = stringlistFromKeyfileSet(keyfile, "OnlyShowIn");
+            auto noshowin = stringlistFromKeyfileSet(keyfile, "NotShowIn");
+
+            auto hasAnyOf = [](std::set<std::string>& a, std::set<std::string>& b) {
+                return std::find_first_of(a.cbegin(), a.cend(), b.cbegin(), b.cend()) != a.cend();
+            };
+
+            if ((!hasAnyOf(onlyshowin, cdesktops) && !onlyshowin.empty()) || hasAnyOf(noshowin, cdesktops))
             {
-                g_warning("Application is not shown in Unity");
-                // Exception removed for OTA11 as a temporary fix
-                // throw std::runtime_error("Application is not shown in Unity");
+                throw std::runtime_error("Application is not shown in '" + std::string{xdg_current_desktop} + "'");
             }
         }
 

=== modified file 'tests/application-info-desktop.cpp'
--- tests/application-info-desktop.cpp	2016-09-14 16:43:36 +0000
+++ tests/application-info-desktop.cpp	2017-01-13 03:03:05 +0000
@@ -30,13 +30,13 @@
 {
 protected:
     ApplicationInfoDesktop()
-        : test_dekstop_env("SomeFreeDesktop")
+        : test_desktop_env("SomeFreeDesktop")
     {
     }
 
     virtual void SetUp()
     {
-        setenv("XDG_CURRENT_DESKTOP", test_dekstop_env.c_str(), true);
+        setenv("XDG_CURRENT_DESKTOP", test_desktop_env.c_str(), true);
     }
 
     virtual void TearDown()
@@ -53,7 +53,7 @@
         return keyfile;
     }
 
-    const std::string test_dekstop_env;
+    const std::string test_desktop_env;
 };
 
 TEST_F(ApplicationInfoDesktop, DefaultState)
@@ -120,18 +120,19 @@
                                                        ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr),
                  std::runtime_error);
 
-/* Disabling for OTA11 */
-#if 0
     // not shown in Unity
     auto notshowin = defaultKeyfile();
-    g_key_file_set_string(notshowin.get(), DESKTOP, "NotShowIn", ("Gnome;" + test_dekstop_env + ";").c_str());
-    EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(notshowin, "/"), std::runtime_error);
+    g_key_file_set_string(notshowin.get(), DESKTOP, "NotShowIn", ("Gnome;" + test_desktop_env + ";").c_str());
+    EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(notshowin, "/", {},
+                                                       ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr),
+                 std::runtime_error);
 
     // only show in not Unity
     auto onlyshowin = defaultKeyfile();
     g_key_file_set_string(onlyshowin.get(), DESKTOP, "OnlyShowIn", "KDE;Gnome;");
-    EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(onlyshowin, "/"), std::runtime_error);
-#endif
+    EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(onlyshowin, "/", {},
+                                                       ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr),
+                 std::runtime_error);
 }
 
 TEST_F(ApplicationInfoDesktop, KeyfileIconPatterns)
@@ -191,16 +192,49 @@
 
     // Appearing explicitly in only show list
     auto onlyshowin = defaultKeyfile();
-    g_key_file_set_string(onlyshowin.get(), DESKTOP, "OnlyShowIn", (test_dekstop_env + ";Gnome;").c_str());
+    g_key_file_set_string(onlyshowin.get(), DESKTOP, "OnlyShowIn", (test_desktop_env + ";Gnome;").c_str());
     EXPECT_NO_THROW(ubuntu::app_launch::app_info::Desktop(onlyshowin, "/", {},
                                                           ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr));
 
     // Appearing explicitly in only show list not first
     auto onlyshowinmiddle = defaultKeyfile();
     g_key_file_set_string(onlyshowinmiddle.get(), DESKTOP, "OnlyShowIn",
-                          ("Gnome;" + test_dekstop_env + ";KDE;").c_str());
+                          ("Gnome;" + test_desktop_env + ";KDE;").c_str());
     EXPECT_NO_THROW(ubuntu::app_launch::app_info::Desktop(onlyshowinmiddle, "/", {},
                                                           ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr));
+
+    // Chance current to be a list
+    setenv("XDG_CURRENT_DESKTOP", ("notafreedesktop:" + test_desktop_env + "::someotherdesktop").c_str(), true);
+
+    // Make sure we can parse it and just not blow up
+    auto base = defaultKeyfile();
+    EXPECT_NO_THROW(ubuntu::app_launch::app_info::Desktop(base, "/", {},
+                                                          ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr));
+
+    // Put in both, make sure we reject
+    auto everything = defaultKeyfile();
+    g_key_file_set_string(everything.get(), DESKTOP, "OnlyShowIn", ("Gnome;" + test_desktop_env + ";KDE;").c_str());
+    g_key_file_set_string(everything.get(), DESKTOP, "NotShowIn", ("Gnome;" + test_desktop_env + ";").c_str());
+    EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(everything, "/", {},
+                                                       ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr),
+                 std::runtime_error);
+
+    // Reject us
+    auto notlist = defaultKeyfile();
+    g_key_file_set_string(notlist.get(), DESKTOP, "NotShowIn", ("Gnome;Foo;" + test_desktop_env + ";KDE;").c_str());
+    EXPECT_THROW(ubuntu::app_launch::app_info::Desktop(notlist, "/", {},
+                                                       ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr),
+                 std::runtime_error);
+
+    // Only Show us
+    g_key_file_set_string(onlyshowin.get(), DESKTOP, "OnlyShowIn", (test_desktop_env + ";Gnome;").c_str());
+    EXPECT_NO_THROW(ubuntu::app_launch::app_info::Desktop(onlyshowin, "/", {},
+                                                          ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr));
+
+    // Make sure we can still go with nothing set
+    auto notset = defaultKeyfile();
+    EXPECT_NO_THROW(ubuntu::app_launch::app_info::Desktop(notset, "/", {},
+                                                          ubuntu::app_launch::app_info::DesktopFlags::NONE, nullptr));
 }
 
 TEST_F(ApplicationInfoDesktop, Orientations)