Package: x2goagent Version: 2:3.5.0.29-0x2go1~git20150113.557+wheezy.heuler.1 A user-local ~/.nx/config/keystrokes.cfg (as described on <http://wiki.x2go.org/doku.php/wiki:advanced:nx-keyboard-shortcuts>, which, btw, still talks about the singular file name "keystroke.cfg") is not read by nxagent, incapacitating users from overriding system-wide defaults. The core reason is the way x2goagent communicates the custom /etc/x2go/keystrokes.cfg path to nxagent: NXAGENT_KEYSTROKEFILE=/etc/x2go/keystrokes.cfg export NXAGENT_KEYSTROKEFILE In nxagent's keystroke-file search order, this environment variable is tested for before ~/.nx/config/keystrokes.cfg is looked at. As a simple remedy, I suggest imitating nxagent's search order in the x2goagent wrapper script: --- debian/wrappers/x2goagent | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/debian/wrappers/x2goagent b/debian/wrappers/x2goagent index 129c7ca..750c651 100644 --- a/debian/wrappers/x2goagent +++ b/debian/wrappers/x2goagent @@ -29,7 +29,15 @@ test -x $NX_LIBS/../x2go/bin/$NXAPP && export NX_LIBS=$NX_LIBS export LD_LIBRARY_PATH -NXAGENT_KEYSTROKEFILE=/etc/x2go/keystrokes.cfg -export NXAGENT_KEYSTROKEFILE +for CONFIG in ~/.x2go/config/keystrokes.cfg ~/.nx/config/keystrokes.cfg \ + /etc/x2go/keystrokes.cfg /etc/nxagent/keystrokes.cfg +do + if [ -r "$CONFIG" ] + then + NXAGENT_KEYSTROKEFILE=$CONFIG + export NXAGENT_KEYSTROKEFILE + break + fi +done exec $NX_LIBS/../x2go/bin/$NXAPP "$@" -- PGP-Key 0xD40E0E7A
On 15.01.2015 04:10 PM, Horst Schirmeier wrote:
debian/wrappers/x2goagent | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/debian/wrappers/x2goagent b/debian/wrappers/x2goagent index 129c7ca..750c651 100644 --- a/debian/wrappers/x2goagent +++ b/debian/wrappers/x2goagent @@ -29,7 +29,15 @@ test -x $NX_LIBS/../x2go/bin/$NXAPP && export NX_LIBS=$NX_LIBS
export LD_LIBRARY_PATH
-NXAGENT_KEYSTROKEFILE=/etc/x2go/keystrokes.cfg -export NXAGENT_KEYSTROKEFILE +for CONFIG in ~/.x2go/config/keystrokes.cfg ~/.nx/config/keystrokes.cfg \
/etc/x2go/keystrokes.cfg /etc/nxagent/keystrokes.cfg
Do we actually WANT to make ~/.nx/foo override /etc/x2go/foo *for x2goagent*? Or do we rather want to get rid of ~/.nx and /etc/nxagent entirely within x2go components (which, really, would make some sort of sense, especially if $SOMETHING created ~/.nx/config/keystrokes.cfg and users wonder why /etc/x2go/keystrokes.cfg does not take any effect.) Actually, I see another problem there. Wouldn't it be smarter to consider both ~/.x2go/foo and /etc/x2go/keystrokes.cfg (if existent), with values in ~/.x2go/foo overriding those of the global configuration file? A priority-based merge would really be the thing we're looking for. I've got something like that lying around. It's not exactly small, though. And would benefit from being shared code, as it could (and also would) be used in both x2goagent and x2goserver. Mihai
On Thu, 15 Jan 2015, Mihai Moldovan wrote:
Do we actually WANT to make ~/.nx/foo override /etc/x2go/foo *for x2goagent*? Or do we rather want to get rid of ~/.nx and /etc/nxagent entirely within x2go components (which, really, would make some sort of sense, especially if $SOMETHING created ~/.nx/config/keystrokes.cfg and users wonder why /etc/x2go/keystrokes.cfg does not take any effect.)
I have no objection removing ~/.nx and /etc/nxagent from the file list in my patch, keeping ~/.x2go/config/keystrokes.cfg and /etc/x2go/keystrokes.cfg. As ~/.nx does not work at all at the moment, this wouldn't even break existing setups (but would require updating the documentation in the wiki).
Actually, I see another problem there. Wouldn't it be smarter to consider both ~/.x2go/foo and /etc/x2go/keystrokes.cfg (if existent), with values in ~/.x2go/foo overriding those of the global configuration file? A priority-based merge would really be the thing we're looking for. I've got something like that lying around.
It's not exactly small, though. And would benefit from being shared code, as it could (and also would) be used in both x2goagent and x2goserver.
This may or may not be a good idea. For example, IMHO it'd require introducing a way to completely remove/disable /etc/x2go/keystrokes.cfg definitions via directives in user-local files, instead of only overriding key definitions. Also, it has the potential of not being compatible to existing setups. Nevertheless, I suggest opening a new bug for this, as #744 needs to be fixed either way.
Horst
-- PGP-Key 0xD40E0E7A
Control: tag -1 patch Hi Horst, hi Mihai, On Do 15 Jan 2015 21:57:27 CET, Mihai Moldovan wrote:
On 15.01.2015 04:10 PM, Horst Schirmeier wrote:
debian/wrappers/x2goagent | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/debian/wrappers/x2goagent b/debian/wrappers/x2goagent index 129c7ca..750c651 100644 --- a/debian/wrappers/x2goagent +++ b/debian/wrappers/x2goagent @@ -29,7 +29,15 @@ test -x $NX_LIBS/../x2go/bin/$NXAPP && export NX_LIBS=$NX_LIBS
export LD_LIBRARY_PATH
-NXAGENT_KEYSTROKEFILE=/etc/x2go/keystrokes.cfg -export NXAGENT_KEYSTROKEFILE +for CONFIG in ~/.x2go/config/keystrokes.cfg ~/.nx/config/keystrokes.cfg \
/etc/x2go/keystrokes.cfg /etc/nxagent/keystrokes.cfg
Thanks for your patch, Horst.
Do we actually WANT to make ~/.nx/foo override /etc/x2go/foo *for
First: We want to allow users to override system-wide settings by user settings.
x2goagent*? Or do we rather want to get rid of ~/.nx and /etc/nxagent entirely within x2go components (which, really, would make some sort of sense, especially if $SOMETHING created ~/.nx/config/keystrokes.cfg and users wonder why /etc/x2go/keystrokes.cfg does not take any effect.)
This indeed is a bit of a drama, I agree. 1. We want to provide NX-X11 to people who still use NX (e.g. FreeNX). To provide this, we have to think generically here. Paths like ~/.nx or /etc/nxagent/ are hard-coded in NX-X11 and nxagent and I think that is ok. All X2Go'ish paths should be overridden via env vars or otherwise. In nxagent, there is some code that checks ARGV[0] (== nxagent? == x2goagent?) and triggers the branding of start-up screens of desktop sessions (the gray X2GO logo). Maybe for setting paths (esp. ~/.x2go/keystrokes.cfg), some similar mechanism should be used?
Actually, I see another problem there. Wouldn't it be smarter to consider both ~/.x2go/foo and /etc/x2go/keystrokes.cfg (if existent), with values in ~/.x2go/foo overriding those of the global configuration file? A priority-based merge would really be the thing we're looking for. I've got something like that lying around.
It would be smarter, indeed. But one step at a time. Let's get this issue solved first cleanly (it obviously is a namespace issue for NX configuration files). Once that is sorted out, you should bring your merging code into the game. I'd suggest you file it as a wishlist bug + patch (or without patch) for now...
It's not exactly small, though. And would benefit from being shared code, as it could (and also would) be used in both x2goagent and x2goserver.
Ok.... So we would bring another dependency into the game that no already existing library or whatsoever could already cover in functionality? (Mike is scared of to many upstream projects having to be maintained inside X2Go). Mike -- DAS-NETZWERKTEAM mike gabriel, herweg 7, 24357 fleckeby fon: +49 (1520) 1976 148 GnuPG Key ID 0x25771B31 mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de freeBusy: https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xf...
On Thu, 15 Jan 2015, Mike Gabriel wrote:
Do we actually WANT to make ~/.nx/foo override /etc/x2go/foo *for
First: We want to allow users to override system-wide settings by user settings.
x2goagent*? Or do we rather want to get rid of ~/.nx and /etc/nxagent entirely within x2go components (which, really, would make some sort of sense, especially if $SOMETHING created ~/.nx/config/keystrokes.cfg and users wonder why /etc/x2go/keystrokes.cfg does not take any effect.)
This indeed is a bit of a drama, I agree.
- We want to provide NX-X11 to people who still use NX (e.g. FreeNX).
To provide this, we have to think generically here. Paths like ~/.nx or /etc/nxagent/ are hard-coded in NX-X11 and nxagent and I think that is ok. All X2Go'ish paths should be overridden via env vars or otherwise.
In nxagent, there is some code that checks ARGV[0] (== nxagent? == x2goagent?) and triggers the branding of start-up screens of desktop sessions (the gray X2GO logo).
Maybe for setting paths (esp. ~/.x2go/keystrokes.cfg), some similar mechanism should be used?
How about this variant? patch for x2go-specific keystroke configuration files, enable user-local keystrokes.cfg --- ...agent_x2go-specific-keystroke-config.full.patch | 22 ++++++++++++++++++++++ debian/patches/series | 1 + debian/wrappers/x2goagent | 3 --- 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch diff --git a/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch b/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch new file mode 100644 index 0000000..8ab4f93 --- /dev/null +++ b/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch @@ -0,0 +1,22 @@ +--- a/nx-X11/programs/Xserver/hw/nxagent/Keystroke.c ++++ b/nx-X11/programs/Xserver/hw/nxagent/Keystroke.c +@@ -27,6 +27,7 @@ + #include "Options.h" + #include "Keystroke.h" + #include "Drawable.h" ++#include "Init.h" /* extern int nxagentX2go */ + + #include <unistd.h> + +@@ -261,6 +262,11 @@ static void parse_keystroke_file(void) + char *homefile = "/.nx/config/keystrokes.cfg"; + char *etcfile = "/etc/nxagent/keystrokes.cfg"; + ++ if (nxagentX2go) { ++ homefile = "/.x2go/config/keystrokes.cfg"; ++ etcfile = "/etc/x2go/keystrokes.cfg"; ++ } ++ + if (nxagentKeystrokeFile != NULL && access(nxagentKeystrokeFile, R_OK) == 0) + { + filename = strdup(nxagentKeystrokeFile); diff --git a/debian/patches/series b/debian/patches/series index 98d3e5e..c65f645 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -61,6 +61,7 @@ 301_nx-X11_use-shared-libs.full.patch 302_nx-X11_xkbbasedir-detection.full.patch 320_nxagent_configurable-keystrokes.full.patch +321_nxagent_x2go-specific-keystroke-config.full.patch 400_nxcomp-version.full+lite.patch #401_nxcomp_bigrequests-and-genericevent-extensions.full+lite.patch 600_nx-X11+nxcompext+nxcompshad_unique-libnames.full.patch diff --git a/debian/wrappers/x2goagent b/debian/wrappers/x2goagent index 129c7ca..ba07a3f 100644 --- a/debian/wrappers/x2goagent +++ b/debian/wrappers/x2goagent @@ -29,7 +29,4 @@ test -x $NX_LIBS/../x2go/bin/$NXAPP && export NX_LIBS=$NX_LIBS export LD_LIBRARY_PATH -NXAGENT_KEYSTROKEFILE=/etc/x2go/keystrokes.cfg -export NXAGENT_KEYSTROKEFILE - exec $NX_LIBS/../x2go/bin/$NXAPP "$@" -- PGP-Key 0xD40E0E7A
On 16.01.2015 03:11 PM, Horst Schirmeier wrote:
How about this variant?
patch for x2go-specific keystroke configuration files, enable user-local keystrokes.cfg
...agent_x2go-specific-keystroke-config.full.patch | 22 ++++++++++++++++++++++ debian/patches/series | 1 + debian/wrappers/x2goagent | 3 --- 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch
diff --git a/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch b/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch new file mode 100644 index 0000000..8ab4f93 --- /dev/null +++ b/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch @@ -0,0 +1,22 @@ +--- a/nx-X11/programs/Xserver/hw/nxagent/Keystroke.c ++++ b/nx-X11/programs/Xserver/hw/nxagent/Keystroke.c +@@ -27,6 +27,7 @@
- #include "Options.h"
- #include "Keystroke.h"
- #include "Drawable.h" ++#include "Init.h" /* extern int nxagentX2go */
- #include <unistd.h>
- +@@ -261,6 +262,11 @@ static void parse_keystroke_file(void)
- char *homefile = "/.nx/config/keystrokes.cfg";
- char *etcfile = "/etc/nxagent/keystrokes.cfg";
- ++ if (nxagentX2go) { ++ homefile = "/.x2go/config/keystrokes.cfg"; ++ etcfile = "/etc/x2go/keystrokes.cfg"; ++ } ++
- if (nxagentKeystrokeFile != NULL && access(nxagentKeystrokeFile, R_OK) == 0)
- {
filename = strdup(nxagentKeystrokeFile);export LD_LIBRARY_PATH
-NXAGENT_KEYSTROKEFILE=/etc/x2go/keystrokes.cfg -export NXAGENT_KEYSTROKEFILE
exec $NX_LIBS/../x2go/bin/$NXAPP "$@"
LGTM. Thank you! Mihai
Control: tag -1 patch
Hi Horst,
On Fr 16 Jan 2015 15:11:05 CET, Horst Schirmeier wrote:
On Thu, 15 Jan 2015, Mike Gabriel wrote:
Do we actually WANT to make ~/.nx/foo override /etc/x2go/foo *for
First: We want to allow users to override system-wide settings by user settings.
x2goagent*? Or do we rather want to get rid of ~/.nx and /etc/nxagent entirely within x2go components (which, really, would make some sort of sense, especially if $SOMETHING created ~/.nx/config/keystrokes.cfg and users wonder why /etc/x2go/keystrokes.cfg does not take any effect.)
This indeed is a bit of a drama, I agree.
- We want to provide NX-X11 to people who still use NX (e.g. FreeNX).
To provide this, we have to think generically here. Paths like ~/.nx or /etc/nxagent/ are hard-coded in NX-X11 and nxagent and I think that is ok. All X2Go'ish paths should be overridden via env vars or otherwise.
In nxagent, there is some code that checks ARGV[0] (== nxagent? == x2goagent?) and triggers the branding of start-up screens of desktop sessions (the gray X2GO logo).
Maybe for setting paths (esp. ~/.x2go/keystrokes.cfg), some similar mechanism should be used?
How about this variant?
patch for x2go-specific keystroke configuration files, enable user-local keystrokes.cfg
...agent_x2go-specific-keystroke-config.full.patch | 22
++++++++++++++++++++++ debian/patches/series | 1 + debian/wrappers/x2goagent | 3 --- 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644
debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patchdiff --git
a/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch
b/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch new file mode 100644 index 0000000..8ab4f93 --- /dev/null +++ b/debian/patches/321_nxagent_x2go-specific-keystroke-config.full.patch
[...]
The patch looks ok. However, two things...
(1) can you re-send that patch as an attachment (not as an inline
text)? (I have been struggling with extracting inline patches from
mail bodies in the past with a lot of hassle on my side, so I am
prophylactically asking without even having tried to apply your
sent-in (inline-text) patch).
(2) I am tempted to merge that patch into patch 320. Would that be ok
with you?
Mike
--
DAS-NETZWERKTEAM mike gabriel, herweg 7, 24357 fleckeby fon: +49 (1520) 1976 148
GnuPG Key ID 0x25771B31 mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de
freeBusy: https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xf...
Hi Mike,
On Mon, 19 Jan 2015, Mike Gabriel wrote:
The patch looks ok. However, two things...
(1) can you re-send that patch as an attachment (not as an inline text)? (I have been struggling with extracting inline patches from mail bodies in the past with a lot of hassle on my side, so I am prophylactically asking without even having tried to apply your sent-in (inline-text) patch).
Sure.
(2) I am tempted to merge that patch into patch 320. Would that be ok with you?
320 is generally useful for nxagent and could be merged into its codebase one day. Mine is x2go specific. This objection aside, feel free to merge the patches, sure! :)
Horst
-- PGP-Key 0xD40E0E7A
On 15.01.2015 11:56 PM, Mike Gabriel wrote:
Thanks for your patch, Horst.
I totally forgot about that. Thanks.
Do we actually WANT to make ~/.nx/foo override /etc/x2go/foo *for
First: We want to allow users to override system-wide settings by user settings.
Yes, my point was explicitly about ~/.nx vs. /etc/x2go. ~/.x2go overriding /etc/x2go is naturally OK.
x2goagent*? Or do we rather want to get rid of ~/.nx and /etc/nxagent entirely within x2go components (which, really, would make some sort of sense, especially if $SOMETHING created ~/.nx/config/keystrokes.cfg and users wonder why /etc/x2go/keystrokes.cfg does not take any effect.)
This indeed is a bit of a drama, I agree.
- We want to provide NX-X11 to people who still use NX (e.g. FreeNX).
To provide this, we have to think generically here. Paths like ~/.nx or /etc/nxagent/ are hard-coded in NX-X11 and nxagent and I think that is ok. All X2Go'ish paths should be overridden via env vars or otherwise.
And that's fine. nxagent (itself) should rely on ~/.nx and /etc/nxagent.
I'm just saying that for x2goagent, it would be smarter to ignore ~/.nx and /etc/nxagent, thus only taking ~/.x2go and /etc/x2go into consideration.
In nxagent, there is some code that checks ARGV[0] (== nxagent? == x2goagent?) and triggers the branding of start-up screens of desktop sessions (the gray X2GO logo).
Well, if we have that anyway, we can expand on that. Horst's new patch sounds great by making use of exactly that.
Maybe for setting paths (esp. ~/.x2go/keystrokes.cfg), some similar mechanism should be used?
Wait... there's ~/.x2go/config/keystrokes.cfg and ~/.x2go/keystrokes.cfg? What's the difference between those?
Actually, I see another problem there. Wouldn't it be smarter to consider both ~/.x2go/foo and /etc/x2go/keystrokes.cfg (if existent), with values in ~/.x2go/foo overriding those of the global configuration file? A priority-based merge would really be the thing we're looking for. I've got something like that lying around.
It would be smarter, indeed. But one step at a time.
On 15.01.2015 10:27 PM, Horst Schirmeier wrote:
For example, IMHO it'd require introducing a way to completely remove/disable /etc/x2go/keystrokes.cfg definitions via directives in user-local files, instead of only overriding key definitions.
I'm not so sure anymore. As pointed out, merging means that you cannot (easily) delete keys in the global config file. I didn't think of that one.
Mihai
On Fri, 16 Jan 2015, Mihai Moldovan wrote:
Maybe for setting paths (esp. ~/.x2go/keystrokes.cfg), some similar mechanism should be used?
Wait... there's ~/.x2go/config/keystrokes.cfg and ~/.x2go/keystrokes.cfg? What's the difference between those?
Currently, there "is" neither. That's why I filed the bug report in the first place. ;-) We still can decide what path it should be. I favor ~/.x2go/config/keystrokes.cfg for being closest to the original ~/.nx/config/keystrokes.cfg.
Horst
-- PGP-Key 0xD40E0E7A
Processing control commands:
tag -1 patch Bug #744 [x2goagent] user-local keystrokes.cfg does not work Added tag(s) patch.
-- 744: http://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=744 X2Go Bug Tracking System Contact owner@bugs.x2go.org with problems
Processing control commands:
tag -1 patch Bug #744 [x2goagent] user-local keystrokes.cfg does not work Ignoring request to alter tags of bug #744 to the same tags previously set
-- 744: http://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=744 X2Go Bug Tracking System Contact owner@bugs.x2go.org with problems
Processing control commands:
clone -1 -2 Bug #744 [x2goagent] user-local keystrokes.cfg does not work Bug 744 cloned as bug 750 reassign -2 wiki.x2go.org Bug #750 [x2goagent] user-local keystrokes.cfg does not work Bug reassigned from package 'x2goagent' to 'wiki.x2go.org'. No longer marked as found in versions 2:3.5.0.29-0x2go1~git20150113.557+wheezy.heuler.1. No longer marked as fixed in versions 2:3.5.0.29. retitle -2 Update keystrokes.cfg docs for nx-libs 3.5.0.29 Bug #750 [wiki.x2go.org] user-local keystrokes.cfg does not work Changed Bug title to 'Update keystrokes.cfg docs for nx-libs 3.5.0.29' from 'user-local keystrokes.cfg does not work' tag -2 - pending Bug #750 [wiki.x2go.org] Update keystrokes.cfg docs for nx-libs 3.5.0.29 Removed tag(s) pending.
-- 744: http://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=744 750: http://bugs.x2go.org/cgi-bin/bugreport.cgi?bug=750 X2Go Bug Tracking System Contact owner@bugs.x2go.org with problems
-
Horst Schirmeier -
Mihai Moldovan -
Mike Gabriel -
owner＠bugs.x2go.org