Cleanup wrapper script that launches IDE #1039
Conversation
|
Oops, branched from an old repository, I'll rebase on current master. |
This is a shell anti-pattern and hinders integration into normal Linux desktop usage. Only daemon launchers should background a job, and even then only on request. Instead we actually want to `exec` it, which has the effect of cleaning up the wrapper script processes, making sure all the signals are connected to the _actual_ foreground process, and passing exit codes and such through when all is said and done.
For some distros (e.g. Arch Linux) packaging binaries compiled elsewhere
is a big no-no, and the more so when they are duplicates of something
the system has anyway. The current Arch Linux packaging for example has
to remove the generated binaries from the installation, then patch this
wrapper script to use the system Lua executable.
This change will make that process easier. First it will be possible to
run with a different Lua executable by setting an environment variable:
LUA_EXECUTABLE=/usr/bin/lua zbstudio
Second, the coding style in the script will make it much easier to patch
to make the that choice stick.
alerque
force-pushed
the
exec-not-background
branch
from
8c43e96
to
cb0a998
Compare
|
Rebased. Note the difference was that in 78229a9 you suppressed warnings. My rebase also reverts that. Suppressing everything on STDERR is also an anti-pattern. If launched from a |
I agree, but it's done for practical purposes, as there are some messages from libraries that wxwidgets using, which clutter the output unnecessarily. Those of us who run debug builds and need to see assertions and other related messages, can easily tweak the launch script, but for everyone else I don't see a reason to get those messages. I like the idea of providing lua executable path to launch with, but I think it needs to be specific to the IDE (in name), as there is a version dependency on (included) libraries. |
|
@alerque, how is |
|
@alerque, just to get back to my earlier question: do you need specifically |
|
I believe this is because In your case your CMakeLists.txt is already using finding Lua and setting the value in |
See commit comments.
Eventually a configure/build time flag should be added to set a system supplied Lua without having to patch at all, but this step in that direction at least makes things easier to start with.