Menubar loads desktop files incorrectly

[nenadalm]

Output of awesome --version:

awesome v3.5.9 (Mighty Ravendark) • Build: Mar 7 2016 18:22:52 for x86_64 by gcc version 6.0.0 (mockbuild@) • Compiled against Lua 5.3.2 (running with Lua 5.3) • D-Bus support: ✔

awesome v4.0 (Harder, Better, Faster, Stronger) • Compiled against Lua 5.1.5 (running with Lua 5.1) • D-Bus support: ✔ • execinfo support: ✔ • RandR 1.5 support: ✘ • LGI version: 0.9.0

How to reproduce the issue: assuming following config:

menubar.menu_gen.all_menu_dirs = { "/usr/share/applications", "/usr/local/share/applications" }

• install application (e.g. google-chrome)
• open menubar (super+p)
• start writing google - there is one entry :+1:
• $ cp /usr/share/applications/google-chrome.desktop /usr/local/share/applications/
• restart awesome
• open menubar (super+p)
• start writing google - there are two entries :-1:

If multiple files have the same desktop file ID, the first one in the $XDG_DATA_DIRS precedence order is used.

(https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#extra-actions-implementation-notes)

[vilev86]

Similar awesome WM issue:

https://wiki.archlinux.org/index.php/awesome#Duplicate_menu-entries_generated_by_Xdg-menu

https://bugs.gentoo.org/646230

[fellowseb]

Hi,

I was wondering what you guys thought about making awesomewm's menubar follow the Desktop Menu Spec (https://specifications.freedesktop.org/menu-spec/latest/index.html) ? I would like to give it a try (propose a PR) if nobody has an objection.

The idea would be to use .menu and .directory files instead of having the categories hard coded in a .lua script. It would be easier to reuse a same menu description between DEs and WMs. Plus it would make us get rid of this bug in particular.

[Elv13]

@fellowseb All for it! Please also note that there is a couple of interesting pull requests open to improve some aspects such as the performance. Someone who would tie all the work together, create some unit tests and ensure compliance with the spec would do a great service to the AwesomeWM community. Another little thing I would do is move this into gears.desktopmenu or something like this so it isn't tied with the menubar library anymore. This way Debian can drop their package patches that add a very old code to add the menu to the top-left. Oh, and as always, if parts of it can be async, I would be very happy, but that's asking too much.

[psychon]

Note that we already have the code that follows the desktop menu spec. It is menubar.icon_theme and was deprecated in 0b0b466705b8f8e4f4cc3a2a4ad353fc0e544b82 because for some reason that no one understands, this was only used to look up the icons for categories and yet, just these few lookups made awesome come to a crawling halt.

The problem is not to implement the spec, but to implement the spec in a clever way, because all those file system lookups are too slow (see the commit message of 0b0b466705b8f8e4f4cc3a2a4ad353fc0e544b82).

I did some baby steps to fix our naive implementation.

• The spec suggests to just recursively scan the icon directory and cache the result, instead of sending individual lookups to the file system. Some code that was meant for this is in #2113.
• Since all of this is still too slow, GTK came up with icon_theme.cache files and, as far as I know, these days everything uses these files. A parser for these files is in #2090.

Since the existing menubar.icon_theme cannot easily be changed to make use of these, nothing more happened.

[psychon]

Wait... I think I confused specs here. You were talking about .desktop files and not icon themes. Sorry! But now I am confused about what you actually want to do here... What are .menu and .directory files and where do they come from? On my current system, I only have google-chrome.menu and not a single .directory file in /usr/.

[fellowseb]

Thanks for the details @Elv13 !

I am indeed talking about the Menu Spec (and the associated Desktop Entry Spec). I am bringing this up because I too stumbled upon this same bug and I thought it would be a good occasion to work towards a complete compliance. The spec uses .menu XML files to describe the menu itself, to reference the directories to be used when searching for the .desktop files and .directory files to categorise the programs in the menu. It allows users to easily override default menus and entries by adding files in their ~/.local/share/. This also means it would be easier for a user to keep consistent menus between different DEs/WMs.

In awesomewm, I see two use cases where this spec would make sense: the menubar and awful.menus (in particular, the mymainmenu instance in the default version of awesomerc.lua)

Here are a few distinct steps I would follow:

• Create gears.desktop_menu (and maybe gears.desktop_entry), add lots of unit tests and use async computation when possible.
• Make menubar call the previously created utilities to construct its data structure, without modifying its behavior and interface. Create a default /usr/local/share/menus/awesome-applications.menu to replace the current static Categories table.
• The mymainmenu case: Add a way to construct a awful.menu (or a part of its items) from the previously created utilities. I think it's achievable to have the entire menu translated into .menu/.directory/.desktop files by using DBus calls to replace the awesome-related entries; but I'm not sure how valuable that would be (We could keep part of the menu coded in the lua file and just let people add items from .desktop/.menu/.directory files).

Your comments relative to the Icon Theme Spec are interesting, but that's another subject :).

Let me know if I'm totally wrong about all this; I don't want to hijack this thread! It would be my first contribution in Lua.

[actionless]

btw here is a short example how to use menubar internals to generate an awful.menu: https://github.com/actionless/awesome_config/blob/devel/actionless/util/menugen.lua

[Elv13]

and btw one using the async APIs https://github.com/Elv13/awesome-configs/blob/master/customMenu/appmenu.lua

[psychon]

Quick & untested hack that might or might not work and that might or might not encourage someone to work on this properly:

diff --git a/lib/menubar/menu_gen.lua b/lib/menubar/menu_gen.lua
index 0be0b696..a40dd97d 100644
--- a/lib/menubar/menu_gen.lua
+++ b/lib/menubar/menu_gen.lua
@@ -10,6 +10,7 @@
 local gtable = require("gears.table")
 local gfilesystem = require("gears.filesystem")
 local utils = require("menubar.utils")
+local gio = require("lgi").Gio
 local pairs = pairs
 local ipairs = ipairs
 local table = table
@@ -82,17 +83,21 @@ function menu_gen.generate(callback)
     menu_gen.lookup_category_icons()

     local result = {}
-    local unique_entries = {}
+    local desktop_file_ids = {}
     local dirs_parsed = 0

+    -- TODO: Turn this into a protected call so that it does not crash awesome, similar to what is done in in utils.parse_dir
+    gio.Async.start(function()
+        -- TODO: Fix the indentation, but I wanted to keep the diff small
     for _, dir in ipairs(menu_gen.all_menu_dirs) do
-        utils.parse_dir(dir, function(entries)
+        utils.async_parse_dir(dir, function(entries)
             entries = entries or {}
             for _, entry in ipairs(entries) do
                 -- Check whether to include program in the menu
                 if entry.show and entry.Name and entry.cmdline then
-                    local unique_key = entry.Name .. '\0' .. entry.cmdline
-                    if not unique_entries[unique_key] then
+                    if not desktop_file_ids[entry.desktop_file_id] then
+                        desktop_file_ids[entry.desktop_file_id] = true
+
                         local target_category = nil
                         -- Check if the program falls into at least one of the
                         -- usable categories. Set target_category to be the id
@@ -115,7 +120,6 @@ function menu_gen.generate(callback)
                                      cmdline = cmdline,
                                      icon = icon,
                                      category = target_category })
-                        unique_entries[unique_key] = true
                     end
                 end
             end
@@ -125,6 +129,7 @@ function menu_gen.generate(callback)
             end
         end)
     end
+end)
 end

 return menu_gen
diff --git a/lib/menubar/utils.lua b/lib/menubar/utils.lua
index d0b9e5e0..1a819be9 100644
--- a/lib/menubar/utils.lua
+++ b/lib/menubar/utils.lua
@@ -407,12 +407,12 @@ function utils.parse_desktop_file(file)
     return program
 end

---- Parse a directory with .desktop files recursively.
+--- Parse a directory with .desktop files recursively in an LGI asynchronous context.
 -- @tparam string dir_path The directory path.
 -- @tparam function callback Will be fired when all the files were parsed
 -- with the resulting list of menu entries as argument.
 -- @tparam table callback.programs Paths of found .desktop files.
-function utils.parse_dir(dir_path, callback)
+function utils.async_parse_dir(dir_path, callback)

     local function get_readable_path(file)
         return file:get_path() or file:get_uri()
@@ -443,6 +443,7 @@ function utils.parse_dir(dir_path, callback)
                         if not success then
                             gdebug.print_error("Error while reading '" .. path .. "': " .. program)
                         elseif program then
+                            program.desktop_file_id = make_relative_path(dir_path, path)
                             table.insert(programs, program)
                         end
                     end
@@ -457,11 +458,18 @@ function utils.parse_dir(dir_path, callback)
         enum:async_close()
     end

-    gio.Async.start(do_protected_call)(function()
-        local result = {}
-        parser(gio.File.new_for_path(dir_path), result)
-        call_callback(callback, result)
-    end)
+    local result = {}
+    parser(gio.File.new_for_path(dir_path), result)
+    call_callback(callback, result)
+end
+
+--- Parse a directory with .desktop files recursively.
+-- @tparam string dir_path The directory path.
+-- @tparam function callback Will be fired when all the files were parsed
+-- with the resulting list of menu entries as argument.
+-- @tparam table callback.programs Paths of found .desktop files.
+function utils.parse_dir(dir_path, callback)
+    gio.Async.start(do_protected_call)(utils.async_parse_dir, dir_path, callback)
 end

 function utils.compute_textbox_width(textbox, s)

[dyfrgi]

Thanks for the starting point, @psychon. I used it as the basis for #3822, which fixes this.