Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support use as a Windows program outside of Cygwin #4086

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rkitover
Copy link

I am currently being legally prevented by the US government from submitting any work anywhere, or even compiling and running any code.

I am posting this here to ask you if you would agree in principle to eventually merge this and to review my implementation if so. I can't be 100% sure all of this works right now, since I can't compile and run anything, or even ask my friends to compile and run anything, because then the police hack into their computer and break the code.

This is not ready to merged yet in any case, I have to get various other things patched first.

You told me previously you'd prefer support for closed-source operating systems to be contained as much as possible, I did this here and the changes to the app code are minimal.

So, let me explain my motives for this work.

The Cygwin port of tmux runs on the Cygwin virtual filesystem and does not behave like a Windows program. That is, the configuration is in your Cygwin home, files on the Cygwin virtual filesystem are used, etc.. By Cygwin here I mean both Cygwin and MSYS2, which uses the Cygwin runtime.

What this change does, is it allows the Cygwin port of tmux to behave like a Windows program when invoked outside of the Cygwin virtual filesystem. Configuration and other filesystem paths are used that would be used by a Windows program, e.g. your .tmux.conf is looked for in your profile directory, your profile temp directory is used for the socket, etc.. SHELL does not require being set, the parent process is looked up and used as the SHELL. So essentially, calling this tmux executable from PowerShell, cmd.exe, Git Bash, etc.. will work and work like a typical Windows program, with no requirements for any parts of a POSIX environment.

These functions first check for Cygwin and a Cygwin virtual filesystem and behave correctly for that case.

This will also allow creating a tmux package for winget and scoop packaged with the MSYS2 DLLs for Windows users to use as they would expect it to work on Windows with PowerShell or any shell of their choice, without requiring a Cygwin or MSYS2 installation. Currently using tmux with PowerShell requires invokking it from WSL with a special configuration.

If you'd like to try this, running this script from an MSYS2 installation will build everything:

https://gist.github.com/rkitover/62ce2b372dcd766dbf910b9837c8477e

, I'm fairly sure everything works already, but not 100% sure.

The commit message is below:

If getenv("SHELL") is NULL, use the function win32_setenv_shell()
added to osdep-win32-cpp.cpp to set it using the command line of the
parent process, which is retrieved using WMI WIN32 API.

This is a C++ file because the OLE/WMI API is only available for C++.
Adjust the autotools code to add this file and link the necessary
Windows DLLs.

Include some fixed/missing MINGW headers necessary to compile this file.
This will be fixed in the relevant places and they will be removed.

Add a new macro WIN32_PLATFORM for Windows specific functionality, currently
Cygwin and MSYS2, in the future for the native port as well.

When spawning commands using the shell, check for cmd.exe on Windows and
use the /c switch, otherwise use -c which works for PowerShell,
MSYS2, Cygwin and Git Bash etc..

Adjust code that uses /tmp/ to use
$env:USERPROFILE/AppData/Local/Temp/ outside of a Cygwin virtual
filesystem when /tmp/ is not available, add the function
win32_get_tmpdir() and related functions to osdep-win32.c for this.

Use getenv("USERPROFILE") when getenv("HOME") is NULL.

When outside of a Cygwin virtual filesystem, use
C:\ProgramData\tmux\tmux.conf:$USERPROFILE\.tmux.conf:$LOCALAPPDATA\tmux\tmux.conf
as the config search order.

Use the ncurses term-driver with TERM="#win32con" when a terminfo
database is not available. This will require patches to ncurses as well
as MSYS2 and Cygwin to work.

@rkitover rkitover marked this pull request as draft August 21, 2024 01:11
@rkitover rkitover force-pushed the windows-shell branch 2 times, most recently from 5713761 to 488b658 Compare August 24, 2024 05:44
@ysc3839
Copy link

ysc3839 commented Sep 19, 2024

You should not get "command line" of parent process. "command line" contains not only executable name, but also arguments. And the "executable name" in command line may not be absolute path.
You should use QueryFullProcessImageNameA to get executable file path.

By the way, Component Object Model (COM) is available in C. You call object method like this:

obj->lpVtbl->Method(obj, args);

You should not use $env:USERPROFILE/AppData/Local/Temp/ as temp directory. The path structure may change in the future. (In Windows XP, temp dir is C:\Documents and Settings\<username>\Local Settings\Temp)
You should use GetTempPathA.

@rkitover
Copy link
Author

@ysc3839 Thank you, I will make these changes.

On the issue of "command line" I am not sure. Given that the shell from which the process is invoked will always be an interactive shell, it may have necessary arguments like -l for Git Bash, or arguments the user prefers like -nologo for PowerShell. What are your thoughts on this?

@ysc3839
Copy link

ysc3839 commented Sep 19, 2024

The shell is pass to execl(), which can not contain arguments.

tmux/spawn.c

Line 466 in 64f1076

execl(new_wp->shell, argv0, (char *)NULL);

Did you check if it works?

AM_CPPFLAGS += -DTMUX_CONF='"$(sysconfdir)/tmux.conf:~/.tmux.conf:$$XDG_CONFIG_HOME/tmux/tmux.conf:~/.config/tmux/tmux.conf"'

if TARGET_WIN32
AM_CPPFLAGS += -DTMUX_CONF_WIN32='"/c/ProgramData/tmux/tmux.conf:~/.tmux.conf:$$LOCALAPPDATA/tmux/tmux.conf"'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use $ProgramData env var here instead of hard coding C: drive.

@rkitover
Copy link
Author

The shell is pass to execl(), which can not contain arguments.

Did you check if it works?

I can't do that right now, I can't write even a line of code without these government ****s breaking it.

Anyway, you are correct according to the specification of execl(), whether it works or not, and I will rework this code.

I still want to preserve the command line.

The SHELL is set to the default-shell option, perhaps it would be better to keep that as the executable and also set the default-command option unless it's overridden.

Another option would be to have default-shell always be cmd.exe and set default-command based on the parent process.

@rkitover
Copy link
Author

@ysc3839

Hi, sorry I haven't yet implemented your changes, I wil do that soon.

I got a chance to try execl() with a parameter in the Cygwin runtime, and it does not in fact work, so I will rework this code.

@nicm
Copy link
Member

nicm commented Oct 1, 2024

I had a quick look at this and a few comments:

  • You will need to rewrite the C++ parts in C.

  • You have the right idea about isolating stuff but I don't like all these games with macros. I think they should be functions in osdep-*.c. So for example instead of GETENV_HOME we could have osdep_get_home which for all platforms except Windows is simply return (getenv("HOME"));. It may be nice to add an osdep-unix.c for the stuff like this that is identical across all Unix platforms.

  • This includes your TTY_OVER_SOCKET changes as well, are they still needed?

  • I don't really want to import MinGW headers. Aren't these shipped with MinGW?

@rkitover
Copy link
Author

rkitover commented Oct 2, 2024

I had a quick look at this and a few comments:

Thank you for taking a look!

  • You will need to rewrite the C++ parts in C.

That will probably not be a problem, I didn't know you can use C for COM.

  • You have the right idea about isolating stuff but I don't like all these games with macros. I think they should be functions in osdep-*.c. So for example instead of GETENV_HOME we could have osdep_get_home which for all platforms except Windows is simply return (getenv("HOME"));. It may be nice to add an osdep-unix.c for the stuff like this that is identical across all Unix platforms.

Yeah, that's a better idea than what I did. I don't like over-reliance on CPP macros either.

  • This includes your TTY_OVER_SOCKET changes as well, are they still needed?

So I have my other PR open with this, this was just on the same branch that I had.

So here's the thing.

You are already including a change specifically for Cygwin, if you look at that function in server-client.c there is an #ifdef __CYGWIN__ there that opens the name of the terminal device passed in from the client. The code to send and receive file descriptors is a noop on Cygwin because Cygwin UNIX socket emulation does not support this. What they do is a localhost TCP socket to emulate UNIX sockets.

My solution does not require opening a device node from the client, is not specific to Cygwin and could be used in other scenarios where passing a file descriptor is not possible. It also does not require the recent changes to the Cygwin runtime and is a pretty small amount of code.

So it would be nice if my other PR was merged, I also include patches from the MSYS2 and Cygwin tmux packages there.

However, that is for you to decide.

  • I don't really want to import MinGW headers. Aren't these shipped with MinGW?

This was a temporary thing due to the MinGW header being buggy. I have since fixed the header with the help of one of the MinGW developers on the list, and the headers package in MSYS2 has been updated, so it will be removed.

@nicm
Copy link
Member

nicm commented Oct 2, 2024

What is the difference between your last PR and this one? Do they solve the same problem or will we need both?

@rkitover
Copy link
Author

rkitover commented Oct 2, 2024

What is the difference between your last PR and this one? Do they solve the same problem or will we need both?

The purpose of my last PR is to improve on the problem of Cygwin and possibly other scenarios where passing a file descriptor over a UNIX socket is not possible.

The point of the work I'm doing here, which is in very "rough draft" stage right now, is to support using tmux as a Windows programs that behaves correctly as a Windows program out of the box.

It's not a full native port, it relies on the Cygwin runtime for most of the functionality, but it would support behaving like a Windows program using Windows paths and launching the user's shell etc..

That's the idea.

@nicm
Copy link
Member

nicm commented Oct 2, 2024

OK, I see, thanks. What platforms are there that we might want to support that do not support file descriptor passing aside from Windows? Since it looks like they will still need a functioning pseudoterminal layer it is hard to see there could be many?

What I'm getting at is - do we actually need both these changes if this one will replace and improve on the previous one for Cgywin/Windows?

If `getenv("SHELL")` is `NULL`, use the function `win32_setenv_shell()`
added to `osdep-win32-cpp.cpp` to set it using the command line of the
parent process, which is retrieved using WMI WIN32 API.

This is a C++ file because the OLE/WMI API is only available for C++.
Adjust the autotools code to add this file and link the necessary
Windows DLLs.

Include some fixed/missing MINGW headers necessary to compile this file.
This will be fixed in the relevant places and they will be removed.

Add a new macro WIN32_PLATFORM for Windows specific functionality, currently
Cygwin and MSYS2, in the future for the native port as well.

When spawning commands using the shell, check for cmd.exe on Windows and
use the `/c` switch, otherwise use `-c` which works for PowerShell,
MSYS2, Cygwin and Git Bash etc..

Adjust code that uses `/tmp/` to use
`$env:USERPROFILE/AppData/Local/Temp/` outside of a Cygwin virtual
filesystem when `/tmp/` is not available, add the function
`win32_get_tmpdir()` and related functions to `osdep-win32.c` for this.

Use `getenv("USERPROFILE")` when `getenv("HOME")` is `NULL`.

When outside of a Cygwin virtual filesystem, use
`C:\ProgramData\tmux\tmux.conf:$USERPROFILE\.tmux.conf:$LOCALAPPDATA\tmux\tmux.conf`
as the config search order.

Use the ncurses term-driver with `TERM="#win32con"` when a terminfo
database is not available. This will require patches to ncurses as well
as MSYS2 and Cygwin to work.

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants