hw_X11 and hw_xft don't work as expected in some cases about twin HOT 11 CLOSED

I agree that twattach behavior is unexpected in the cases you describe.
As you point out, in most (all?) cases, the solution is to use twdisplay instead, which is much more predictable.

It may help considering that twattach does "almost" nothing: it connects to an existing twin server, without passing its environment variables ($DISPLAY etc.), and merely asks the twin server to start a new display driver (the --hw=... argument specified in twattach command line). There is a little magic to determine twattach current's controlling tty if you specify --hw=tty, but that's all.

This means the environment where you execute twattach (inside a different X11 session, on a different machine, with different environment variables, etc.) has no effect at all: only the environment where you started the twin server is used.

Trying to fix twattach behavior by propagating its environment to twin server would cause more issues than it solves:
the server's idea of $DISPLAY may change - without updating the corresponding file ~/.Xauthority - which would prevent any direct connection from twin server to surrounding X11 or VNC servers. And similarly for the rest of the environment variables.

Again, the solution is to use twdisplay, which is designed exactly for complex situations, where the enviroment at the moment twin's server was started and the current environment where twdisplay is executed are different.

If you prefer an analogy, you can consider twattach as a close-to-metal tool to send raw commands to twin's server, and twdisplay as the equivalent of a VNC viewer.

from twin.

the server's idea of $DISPLAY may change - without updating the corresponding file ~/.Xauthority - which would prevent any direct connection from twin server to surrounding X11 or VNC servers.

That behavior is expected.

And similarly for the rest of the environment variables.

I just mentioned passing $DISPLAY, nothing else. That would be unexpected.

OK. Challenge accepted! I'll give it a whirl. I think it will work well.

from twin.

One very simple and very effective thing you (or I) can do is to modify twattach source as follows:

If it receives a command line argument --hw=X... or --hw=X11... or --hw=xft... which does not contain @SOME_X11_DISPLAY, it can make a copy of such argument and modify it to contain @MY_DISPLAY before sending it to twin server, where MY_DISPLAY is the value of environment variable $DISPLAY.

Doing so will instruct twin server to open an X11 window on the $DISPLAY indicated by the environment variables of twattach .

As you pointed out, it's a more intuitive behavior than using twin server's environment variable $DISPLAY

from twin.

Already done. Seems to work well. ;-) Needs more testing.

from twin.

Good :)
When you have tested it, feel free to create a pull request containing your changes - this solution does not nees to modify twin server's environment variables, so I have no objections in merging it

from twin.

Another things to consider while rejecting this idea. ;-)

• User George logs in to X. Starts a TWIN session detaches and logs out of X. Jane logs in to X on the same machine and also starts a TWIN session. $DISPLAY in the first session is no longer valid.

The point is that $DISPLAY is highly volatile. There really is no point in hanging on to it. Might even be a good idea to expunge it from TWIN's environment... But somebody may have a use for it... starting X apps from TWIN? It will work in limited scenarios where there is one and only one user and the X server is always running and logged in as that one user. At least logged in when the user wants to start something X from TWIN.

from twin.

Exactly.

Keeping the original value of $DISPLAY in twin server seems the least surprising behavior to me.

After the patch we are currently discussing is merged, twin server's environment variable $DISPLAY will be used in very few cases - for example if you open a builtin terminal in twin and then you try to launch some X11 program from it. Or if you launch an X11 program from twin's builtin menu File|Execute.

And anyway, if the user logged in changes, X11 server's .Xauthority changes too, so it will reject connections from clients launched by the previous user, as their .Xauthority is no longer valid.

And if someone uses xhost + or xhost +local: or something similar that disables .Xauthority check, they will get what they asked for: an insecure configuration that grants X11 access to other users.

from twin.

Exactly.... and since we have a solution we can both agree on: enough said! :-D I'll be building packages shortly and then distribute it to my GUI systems. I'll re-run my twattach series and try to spot any "blunders" I've made. If all goes well you should have my patch tomorrow.

In general its against my religion to do anything M$. So I'll just submit the patch here. Its pretty small...

from twin.

Well its working for me. Here's what I did:

diff --git a/clients/attach.c b/clients/attach.c
index 22fce5f..27d18e9 100644
--- a/clients/attach.c
+++ b/clients/attach.c
@@ -101,6 +101,20 @@ static void InitSignals(void) {
 #endif
 }
 
+static char *Xfix(char *arg) {
+  char *ps, *pd, *r, *DISPLAY;
+  int l;
+
+  DISPLAY = getenv("DISPLAY");
+  if(!DISPLAY || !*DISPLAY) return strdup(arg); // nothing to do w/o $DISPLAY
+  pd = r = malloc(strlen(arg)+strlen(DISPLAY)+2);
+  for(ps=arg; *ps && *ps!=','; *pd++ = *ps++);
+  *pd++ = '@';
+  strcpy(pd, DISPLAY);
+  if(*ps==',') strcat(pd, ps);
+  return r;
+}
+
 TW_DECL_MAGIC(attach_magic);
 
 int main(int argc, char *argv[]) {
@@ -186,6 +200,11 @@ int main(int argc, char *argv[]) {
           sprintf(arg, "-hw=tty%s", s);
         } else
           arg = strdup(*argv);
+      } else if(!strncmp(*argv + 4, "X", 1) && (!(*argv)[5] || (*argv)[5]==',')) {
+        arg = Xfix(*argv);
+      } else if((!strncmp(*argv + 4, "xft", 3) || !strncmp(*argv + 4, "X11", 3))
+               && (!(*argv)[7] || (*argv)[7]==',')) {
+        arg = Xfix(*argv);
       } else if ((*argv)[4])
         arg = strdup(*argv);
       else {

from twin.

You approach is correct, but once again I ended up modifying your patch - see commit 8f4b7d1

As a side note, using twattach --hw={X,X11,xft} causes my twin server to crash if it already has an open X11 driver - I'll investigate it

from twin.

You approach is correct, but once again I ended up modifying your patch

A shame. My solution was KISS++ and DRY++. It took care of all of the situations you mentioned in your email and the future potential or hacker shenanigans of hw driver names starting with the same prefix.

FYI: I did test all three driver names, with and without @ and with and without arguments. Some 12 test cases. Which used two different X servers on the same machine and were repeated with TWIN sessions started in and out of an X environment. Very tedious.

But at least others get the fix too.

from twin.