[Github-comments] [geany/geany] write absolute path to geany bin in desktop file (#2728)

[Colomban Wendling]

@b4n requested changes on this pull request.

I am not sure we do want this, but maybe.  I don't see much downside, but perhaps when building with `--enable-binreloc` (gives `Exec=//bin/geany` which is… well, both right an wrong), yet I don't know how much sense a .desktop makes for a relocatable installation.
So let's say we're happy with it.

However, I don't like the implementation.  IMO, we should rather do something like https://github.com/b4n/geany/commit/884309bd6e7969f7b2d01592fa6741cd76b1580b -- and AFAIK it's how other do.

This said, full path is not a popular trend in my Debian stable, which kind of makes me wonder if there is a downside I can't think about:
```shell
$ rgrep 'Exec *= */' /usr/share/applications/ | wc -l # absolute paths
27
$ rgrep 'Exec *= *[^/]' /usr/share/applications/ | wc -l # non-absolute paths
233
```

> @@ -11,6 +11,18 @@ AC_DEFUN([GEANY_PREFIX],
 	if test "x$exec_prefix" = xNONE; then
 		exec_prefix=$prefix
 	fi
+
+	# The $bindir variable is equal to the literal string "${exec_dir}/bin"
+	# rather than the actual path. pkgconfig (.pc) files can use that ok, but
+	# other files written out by configure require the literal path.
+	#
+	# If the --bindir argument is given to configure, then $bindir will already
+	# be equal to an absolute path.

Actually no, AFAIK it's valid to pass `--bindir` with placeholders.  The only "reliable" way of expanding one of the dir variables is to eval it until it stops changing -- which is both annoying and tricky.  IIRC there's a 3rd party macro to do so, but in any case it's not great.

The best solution usually is to perform these replacements at make time.

> @@ -9,6 +9,9 @@ AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([1.11 -Wall parallel-tests subdir-objects])
 AC_CONFIG_HEADERS([config.h])
 
+PKG_DESC="A fast and lightweight IDE using GTK+"

I think that's kind of over-engineering the thing, and IMO should at least not be part of this change.

> +Name: @PACKAGE_NAME@
+Description: @PKG_DESC@

This is unrelated to the proposed change

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2728#pullrequestreview-569935949
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20210116/a2627175/attachment.htm>