bbidulock / icewm

A window manager designed for speed, usability, and consistency
Other
571 stars 97 forks source link

Absolute paths in *.desktop files #684

Closed danfe closed 1 year ago

danfe commented 1 year ago

Two files, lib/icewm-session.desktop and lib/icewm.desktop have TryExec and Exec entries which contain absolute paths to corresponding executables which assume that IceWM is installed under /usr prefix, which is not necessarily true (./configure script correctly respects custom prefix). Consider dropping full paths altogether and just rely on $PATH search, as their names are unique enough to not require absolute paths. While 3rd-party software typically installed under /usr in GNU/Linux, other systems use different prefix, e.g. /usr/local or /usr/pkg.

gijsbers commented 1 year ago

The git history shows that this used to be the case. However, there are environments which fail to start icewm at login, unless the Exec paths are absolute. One could modify the paths to a specific prefix at install. If you know how to change Makefile.am to do this, tell us.

danfe commented 1 year ago

If you know how to change Makefile.am to do this, tell us.

You just need to amend AC_CONFIG_FILES macro in the configure.ac file like this pseudo-patch below:

@@ -521,6 +521,8 @@ AC_CONFIG_FILES([Makefile
         doc/Makefile
         man/Makefile
         lib/Makefile
+        lib/icewm-session.desktop
+        lib/icewm.desktop
         lib/keys
         lib/menu
         lib/programs

Then, rename both *.desktop files to *.desktop.in and replace those /usr parts of the paths with @prefix@ so it will be expanded correctly as configured.

gijsbers commented 1 year ago

That would add two new files. How about this hack to avoid that:

--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -5,6 +5,11 @@ dist_xsession_DATA = \
        icewm.desktop \
        icewm-session.desktop

+install-data-hook:
+       sed -i 's|Exec=/.*/|Exec=$(bindir)/|' \
+           $(DESTDIR)$(xsessiondir)/icewm.desktop \
+           $(DESTDIR)$(xsessiondir)/icewm-session.desktop
+
danfe commented 1 year ago

That would add two new files.

Well, not really add, just rename two existing ones, giving them .in suffix; the number of files does not change. The same mechanism is already used to prepare keys, menu, program, etc. Frankly, I don't see how adding two more to the list would hurt. Computers are good at processing files, let them do their job. :-)

How about this hack to avoid that

It's essentially the same thing that ./configure would do, only done manually. We call it, setting the Sun by hand, could not find suitable English metaphor. It also creates another editing point in addition to already used (and expected) AC_CONFIG_FILES mechanism.

That said, I'd accept any working solution, this is your code after all, but IMHO two extra .*in files that could be processed in a standard way, coherent with other *.in files you have there already, do not warrant employing a hack. Hacks are generally bad and create needless maintenance burden which clean solutions do not entail.