GNOME Bugzilla – Bug 707728
More general bug fixes
Last modified: 2013-10-12 20:16:30 UTC
This started as "support translated stuff in appstream", and ended up in another round of miscellanous fixes. I believe the two about plugin priorities are particularly important, and probably Richard should give a look at them personally. The jhbuild plugin is just an experiment in having an other application backend. It works for removing (not yet installing or updating, since it's not clear if we should "build" or "buildone"), but it is terribly slow, because it forks to jhbuild for every package. It's not really for merging in master.
Created attachment 254410 [details] [review] appstream: add support for translations Recognize xml:lang tags in the appstream files and use them to provide translated names, summaries and descriptions.
Created attachment 254411 [details] [review] datadir-apps: use localized variants of Name and Comment This way the translated names are shown for installed apps, even if the appstream data does not provide translations.
Created attachment 254412 [details] [review] Fix plugin priorities We want to run appstream first, so we can translate package-names to app ids and viceversa. We want to run datadir-filename before desktopdb (so the .desktop files matches the one seen by the shell, in case of local overrides to system packages), and we want to run datadir-apps after datadir-filename and desktopdb (so the .desktop file name is already known). Note that desktopdb becomes effectively superflous in presence of appstream data. Also, datadir-filename must not check for the app name (since it runs after appstream now). It shouldn't anyway, the purpose of this plugin is to se the .desktop filename.
Created attachment 254413 [details] [review] packagekit-refine: don't override the state if already known The flow is like this: the GsShell calls get_popular(), hardcoded-popular replies with a list of GsApps without metadata. For each app, datadir-filename finds the .desktop file, in this case in a jhbuild directory; datadir-filename-local notes that the app is not from /usr, and marks it installed. appstream also knows about the app, and sets a package-name. Finally, packagekit-refine sees the app does not have a package-id (because it didn't originate from packagekit), so it searches for one. At that point, it would set the installed status according to the package, but that's wrong, because the app is installed locally.
Created attachment 254414 [details] [review] AppWidget: don't allow removing applications installed through jhbuild If the install-kind is local, assume that no plugin is capable of removing the app and make the widget insensitive. Note that before removing was actually possible, as packagekit-refine would set a package-id even on locally installed applications (and then packagekit would not check the install-kind)
Created attachment 254415 [details] [review] Fix the priority of the hardcoded-refine plugins We need to run these after appstream (to get the package-name to app ID conversion) and after datadir-apps (to override the kind set there). This mostly affects hardcoded-kind, but I changed the other for consistency too. Note that if we ever have real plugins for this data, we may have to revisit this.
Created attachment 254416 [details] [review] datadir-apps: ignore ENOENT reading the .desktop filename The desktopdb plugin can sometimes give us a non existing .desktop (for example if the cache was not regenerated after uninstalling the app), spare the warning in that case.
Created attachment 254417 [details] [review] ShellDetails: allow removing an application that can be updated There is not reason not to allow that.
Created attachment 254418 [details] [review] packagekit: don't try to remove applications that were not installed as packages Fail with NOT_SUPPORTED is install-kind != package, and let the other plugins handle it. Also, make sure that packagekit-refine also sets install-kind == package, to remove featured and popular apps.
Created attachment 254419 [details] [review] Add a jhbuild plugin Allows showing information (version, mostly) for jhbuilt applications, and removing them.
Review of attachment 254410 [details] [review]: ::: src/plugins/appstream-app.c @@ +104,3 @@ + app->names = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + app->summaries = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + app->descriptions = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); Do we really need to keep tables of translations ? I would expect us to just keep the 'best' locale around
Review of attachment 254411 [details] [review]: sure
(In reply to comment #11) > Review of attachment 254410 [details] [review]: > > ::: src/plugins/appstream-app.c > @@ +104,3 @@ > + app->names = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, > g_free); > + app->summaries = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, > g_free); > + app->descriptions = g_hash_table_new_full (g_str_hash, g_str_equal, > g_free, g_free); > > Do we really need to keep tables of translations ? I would expect us to just > keep the 'best' locale around Yeah, but we don't know the best locale until we read all of them
Review of attachment 254416 [details] [review]: makes sense
Review of attachment 254417 [details] [review]: I think this needs some clarifications on how the various GsApp states and kinds are defined and how they interact. I've been asking for this for a while...
Review of attachment 254419 [details] [review]: Frankly, I don't think jhbuild makes sense in this application. It is a distraction from what gnome-software is about, and muddies the waters when we haven't even gotten package-based applications fully done yet...
Created attachment 254489 [details] [review] appstream: add support for translations Recognize xml:lang tags in the appstream files and use them to provide translated names, summaries and descriptions.
(In reply to comment #17) > Recognize xml:lang tags in the appstream files and use them to > provide translated names, summaries and descriptions. This patch only, fixed up and merged, thanks!
I think we've got all the important bits from this patch, so I'll close it now. Thanks!