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

[Colomban Wendling]

@b4n requested changes on this pull request.

This has a few issues IMO:
* The way you modify the file *changes the whole line*, meaning that if that rule and the source file are not in sync, it's super hard to diagnose weird content in the installed file (as it ignores the content of the source file).
* I don't like modifying a file at install time.  Not only this means you get another set of files before and after installation, this also means that for a lot of people it'll modify build files *as root* (if they install as root), which is awfully annoying (you get root-owned files you can't modify in a user directory).
* The [documentation](https://www.gnu.org/software/automake/manual/automake.html#Extending-Installation) doesn't actually say whether or not `install-data-local` is performed before or after the Automake rules, so it's not guaranteed modifying a file will install it.  In practice it's probably the case if it currently works (and I guess the implementation is straightforward and guarantees it), but it's undocumented.

I'll abuse of my dictatorship here and say that if we want this we'll use my set of changes (https://github.com/b4n/geany/commit/884309bd6e7969f7b2d01592fa6741cd76b1580b), unless of course anybody has specific concerns about it.



-- 
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-570468082
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20210118/db713756/attachment.htm>