Merge lp:~cr3/checkbox/940627 into lp:checkbox

[Marc Tardif]

When reviewing, please make sure that the changes to plugins/backend_info.py are sane. Thanks!

[Daniel Manrique (roadmr)]

Thanks for pointing out the dubious section!

Without the pointer to the desktop file, bug 776712 will resurface. So in any case they should point to the checkbox-qt file:

prefix = ["kdesudo", "--desktop", "/usr/share/applications/checkbox-qt.desktop", "--"]
prefix = ["gksu", "--description", "/usr/share/applications/checkbox-qt.desktop", "--"]

Check this merge request for a brief discussion on this topic:

https://code.launchpad.net/~carl-milette/checkbox/776712/+merge/62193

[Marc Tardif (cr3)]

But the checkbox-qt.desktop file will not exist in the case where only checkbox-gtk is installed. We could either check for the existence of the file and only pass it as argument when it is available. Or, we could always pass the checkbox-qt.desktop file as argument which seems to work with gksu and kdesudo if the file doesn't exist. Which would you prefer?

review: Needs Information

[Daniel Manrique (roadmr)]

Urgh :(

I see two solutions:

1- Add some logic to look for either checkbox-qt.desktop or checkbox-gtk.desktop and use the first one available.

2- Move to a single checkbox.desktop file. This would mean that checkbox-qt and checkbox-gtk would be mutually exclusive so this solution potentially sucks :(

3- make checkbox.desktop a symlink to the correct one, and have each package update the symlink to its .desktop file upon installation (does update-alternatives apply to .desktop files?)

OK, three solutions :)

Which one sounds easier to implement and more future-proof? (i'd say #1).

review: Needs Information

[Marc Tardif (cr3)]

Of the three solutions you propose, I would also lean towards the first one. However, the only problem is that the checkbox-gtk.desktop file will never exist. The point of this merge request is to prevent the confusion of potentially having two Desktop testing applications after upgrading from Oneiric to Precise. Since we agreed that checkbox-gtk should be upgraded rather than deprecated with a transitional package, we need to workaround the confusion caused by the two desktop files.

Since checkbox-gtk and checkbox-qt should be able to coexist, this also poses a problem for your second solution.

As for your third solution, my brain hurts just trying to think about it :(

[Marc Tardif (cr3)]

How about we revert this discussion to the suggestion you made ealier about always using the checkbox-qt.desktop file? In the most likely cases, this file will exist:

1. When installing Precise, only checkbox-qt will exist so the checkbox-qt.desktop file will be present.
2. When upgrading from Oneiric to Precise, or from Lucid to Precise for that matter, checkbox-qt will also exist so the checkbox-qt.desktop file will also be present. Even if running checkbox-gtk will effectively use the checkbox-qt.desktop file, that shouldn't make a difference.

The only corner case where bug 776712 would resurface is when someone explicitly installs checkbox-gtk and uninstalls checkbox-qt, implying also uninstalling ubuntu-desktop. I'd be comfortable with that.

[Daniel Manrique (roadmr)]

I think always using checkbox-qt is a reasonable behavior. In addition to what you mention in the last comment, I also checked the behavior for gksu, and if the --description points to a nonexistent file, it simply quotes that name.

So if checkbox-qt.desktop exists, it nicely says "The application named 'System Testing' wants privileges". If it doesn't exist, it says "The application named '/usr/share/applications/checkbox-qt.desktop' wants privileges".

This is potentially not entirely confusing to someone who, as you mention, removed checkbox-qt and ubuntu-desktop by hand.

So my suggestion, sneakily based mostly on *your* arguments, is to just default to checkbox-qt.desktop.

As per our new and fancy review process, since this implies some agreed-upon code changes, I'll set to "Needs Fixing".

review: Needs Fixing

[Marc Tardif (cr3)]

Ok, defaulting to checkbox-qt.desktop.

review: Needs Resubmitting

[lp:~cr3/checkbox/940627]

1282. By Marc Tardif on 2012-03-05

Merged from trunk.

1283. By Marc Tardif on 2012-03-05

Reverted to just using /usr/share/applications/checkbox-qt.desktop when calling kdesudo or gksu.

[Daniel Manrique (roadmr)]

Awesome, thanks!

Merging.

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-03-05 19:10:06 +0000
+++ debian/changelog	2012-03-05 19:54:20 +0000
@@ -30,6 +30,7 @@
   * Tidied up logic for determining DISK device product and vendor 
     (LP: #942548)
   * Fixed filename matching expression for local jobs (LP: #942273)
+  * Fixed duplicate System Testing applications after upgrade (LP: #940627)
 
   [Aurelien Gateau]
   * lib/template.py, lib/template_i18n.py, plugins/jobs_info.py,

=== modified file 'debian/checkbox-gtk.install'
--- debian/checkbox-gtk.install	2011-12-05 15:25:22 +0000
+++ debian/checkbox-gtk.install	2012-03-05 19:54:20 +0000
@@ -1,5 +1,4 @@
 usr/bin/checkbox-gtk
-usr/share/applications/checkbox-gtk.desktop
 usr/share/checkbox/gtk/*
 usr/share/checkbox/examples/checkbox-gtk.ini
 usr/lib/python*/*-packages/checkbox_gtk/*

=== removed file 'gtk/checkbox-gtk.desktop.in'
--- gtk/checkbox-gtk.desktop.in	2011-09-16 19:40:25 +0000
+++ gtk/checkbox-gtk.desktop.in	1970-01-01 00:00:00 +0000
@@ -1,11 +0,0 @@
-[Desktop Entry]
-_Name=System Testing
-_Comment=Test your system and submit results to the Ubuntu Friendly project
-Encoding=UTF-8
-Exec=/usr/bin/checkbox-gtk
-Terminal=false
-Type=Application
-Icon=checkbox
-StartupNotify=true
-Categories=GTK;GNOME;Application;System;Settings;
-X-Ubuntu-Gettext-Domain=checkbox

=== modified file 'plugins/backend_info.py'
--- plugins/backend_info.py	2011-12-08 22:31:36 +0000
+++ plugins/backend_info.py	2012-03-05 19:54:20 +0000
@@ -62,13 +62,13 @@
                     stdout=PIPE, stderr=PIPE) == 0 and \
                 call(["pgrep", "-x", "-u", str(uid), "ksmserver"],
                     stdout=PIPE, stderr=PIPE) == 0:
-            prefix = ["kdesudo", "--desktop", "/usr/share/applications/checkbox-gtk.desktop", "--"]
+            prefix = ["kdesudo", "--desktop", "/usr/share/applications/checkbox-qt.desktop", "--"]
         elif os.getenv("DISPLAY") and \
                 call(["which", "gksu"],
                     stdout=PIPE, stderr=PIPE) == 0 and \
                 call(["pgrep", "-x", "-u", str(uid), "gnome-panel|gconfd-2"],
                     stdout=PIPE, stderr=PIPE) == 0:
-            prefix = ["gksu", "--description", "/usr/share/applications/checkbox-gtk.desktop", "--"]
+            prefix = ["gksu", "--description", "/usr/share/applications/checkbox-qt.desktop", "--"]
         else:
             prefix = ["sudo"]
 

=== modified file 'po/POTFILES.in'
--- po/POTFILES.in	2011-12-15 15:54:31 +0000
+++ po/POTFILES.in	2012-03-05 19:54:20 +0000
@@ -1,6 +1,6 @@
 [encoding: UTF-8]
 [type: gettext/glade] gtk/checkbox-gtk.ui
-gtk/checkbox-gtk.desktop.in
+qt/checkbox-qt.desktop.in
 [type: gettext/rfc822deb] jobs/audio.txt.in
 [type: gettext/rfc822deb] jobs/autotest.txt.in
 [type: gettext/rfc822deb] jobs/bluetooth.txt.in

=== modified file 'setup.cfg'
--- setup.cfg	2011-12-05 15:25:22 +0000
+++ setup.cfg	2012-03-05 19:54:20 +0000
@@ -4,7 +4,7 @@
 [build_i18n]
 domain=checkbox
 desktop_files=[
-    ("share/applications", ("gtk/checkbox-gtk.desktop.in","qt/checkbox-qt.desktop.in"))]
+    ("share/applications", ("qt/checkbox-qt.desktop.in"))]
 rfc822deb_files=[
     ("share/checkbox/jobs",
         ("jobs/audio.txt.in",