cuviper/ssh-pageant

Compatibility with MSYS2

Closed this issue · 13 comments

Would it be possible to make ssh-pageant compatible with MSYS2 environment http://sourceforge.net/projects/msys2/? MSYS2 is an independent rewrite of MSYS, based on modern Cygwin (POSIX compatibility layer) and MinGW-w64 with the aim of better interoperability with native Windows software. I tried compiling the ssh-pageant source code, but I got many compilation errors due to missing include files:

sys/un.h
err.h
sys/cygwin.h
sys/select.h
sys/socket.h
sys/wait.h
arpa/inet.h

It would be great to be able to use ssh-pageant together with the openssh of MSYS2.

It seems sourceforge is having an outage, so I can't try it myself right now...

With MSYS, there are two build environments you can enter: MINGW for native Windows builds, and MSYS for building on that POSIX layer. If you are missing so many headers, I suspect you're in MINGW mode, which won't work. For example, you'd use msys.bat MSYS as described on the MinGW wiki, but you may have to discover MSYS2's equivalent. But at the very least, sys/un.h is fundamental to AF_UNIX sockets like SSH_AUTH_SOCK.

The second thing is ssh-pageant's compat.h. If MSYS2 is really based on modern Cygwin, then it won't need most of those workarounds. They are guarded by #ifdef __MSYS__, so if MSYS2 also sets this, we'll need to find a way to distinguish them. I'd like to avoid a whole autoconf-style setup, if simple checks will suffice. :)

Actually, you might be able to use the plain cygwin builds, if MSYS2 provides a compatible cygwin1.dll. Did you try this? (But I'd still like to make it possible to build directly too.)

Thank you for your quick reply. I just tried the Cygwin binaries (32-bit and 64-bit). They give me the same result as the MSYS binary:

$ ./ssh-pageant ssh-add -l
ssh-pageant.exe: error while loading shared libraries: ?: cannot open shared object file: No such file or directory

Indeed, I tried to compile ssh-pageant using MinGW-W64 gcc 4.9.2 from a shell launched via "C:\msys64\mingw64_shell.bat" and thus ran into the issue with the missing headers. I now tried using the msys/gcc version instead and the missing header errors are gone, but new errors caused by definitions in compat.h arise:

$ make
gcc -O2 -Werror -Wall -Wextra -MMD   -c -o main.o main.c
In file included from main.c:11:0:
compat.h:38:14: error: static declaration of ‘program_invocation_short_name’ follows non-static declaration
 static char *program_invocation_short_name = ssh_pageant_name;
              ^
In file included from /usr/include/errno.h:9:0,
                 from compat.h:24,
                 from main.c:11:
/usr/include/sys/errno.h:26:23: note: previous declaration of ‘program_invocation_short_name’ was here
 extern __IMPORT char *program_invocation_short_name;
                       ^
In file included from main.c:11:0:
compat.h:53:1: error: static declaration of ‘mkdtemp’ follows non-static declaration
 mkdtemp(char *template)
 ^
In file included from /usr/include/string.h:10:0,
                 from /usr/include/sys/un.h:14,
                 from compat.h:16,
                 from main.c:11:
/usr/include/stdlib.h:110:8: note: previous declaration of ‘mkdtemp’ was here
 char * _EXFUN(mkdtemp,(char *));
        ^
In file included from main.c:11:0:
compat.h:63:1: error: static declaration of ‘strlcpy’ follows non-static declaration
 strlcpy(char *dst, const char *src, size_t size)
 ^
In file included from /usr/include/string.h:10:0,
                 from /usr/include/sys/un.h:14,
                 from compat.h:16,
                 from main.c:11:
/usr/include/string.h:119:8: note: previous declaration of ‘strlcpy’ was here
 size_t _EXFUN(strlcpy,(char *, const char *, size_t));
        ^
In file included from main.c:11:0:
compat.h:85:1: error: static declaration of ‘accept4’ follows non-static declaration
 accept4(int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags)
 ^
In file included from compat.h:30:0,
                 from main.c:11:
/usr/include/sys/socket.h:26:7: note: previous declaration of ‘accept4’ was here
   int accept4 (int, struct sockaddr *__peer, socklen_t *, int flags);
       ^
In file included from main.c:11:0:
compat.h:104:1: error: static declaration of ‘cygwin_conv_path’ follows non-static declaration
 cygwin_conv_path (unsigned what, const void *from, void *to, size_t size)
 ^
In file included from compat.h:29:0,
                 from main.c:11:
/usr/include/sys/cygwin.h:74:16: note: previous declaration of ‘cygwin_conv_path’ was here
 extern ssize_t cygwin_conv_path (cygwin_conv_path_t what, const void *from,
                ^
In file included from main.c:11:0:
compat.h: In function ‘cygwin_conv_path’:
compat.h:111:5: error: implicit declaration of function ‘cygwin_conv_to_posix_path’ [-Werror=implicit-function-declaration]
     if (((what & CCP_RELATIVE) ? cygwin_conv_to_posix_path(from, posix)
     ^
compat.h:112:17: error: implicit declaration of function ‘cygwin_conv_to_full_posix_path’ [-Werror=implicit-function-declaration]
                 : cygwin_conv_to_full_posix_path(from, posix)) != 0)
                 ^
In file included from main.c:29:0:
winpgntc.h: At top level:
winpgntc.h:21:23: error: conflicting types for ‘uint32_t’
 typedef unsigned long uint32_t;
                       ^
In file included from /usr/lib/gcc/x86_64-pc-msys/4.9.2/include/stdint.h:9:0,
                 from /usr/include/cygwin/socket.h:19,
                 from /usr/include/sys/un.h:15,
                 from compat.h:16,
                 from main.c:11:
/usr/include/stdint.h:34:22: note: previous declaration of ‘uint32_t’ was here
 typedef unsigned int uint32_t;
                      ^
cc1: all warnings being treated as errors
<builtin>: recipe for target 'main.o' failed
make: *** [main.o] Error 1

MSYS2's gcc pre-defines the following Cygwin- and MSYS-related macros:

$ gcc   -dM -E -x c /dev/null | grep CYGWIN
#define __CYGWIN__ 1
$ gcc   -dM -E -x c /dev/null | grep MSYS
#define __MSYS__ 1

Unfortunately, there doesn't seem to be a dedicated MSYS2 or similar. If MSYS did not define CYGWIN, this could be a way to distinguish MSYS2 from MSYS. Unfortunately, I don't have access to an MSYS installation to test it. Maybe it could be also exploited that MSYS2 uses a much more recent gcc version (currently 4.9.2) than MSYS (which is stuck with gcc 3.x).

As a side note, charade, which also failed to compile using MinGW-W64/gcc does compile using MSYS2/gcc. Unfortunately, charade requires the package "keychain", which isn't packaged for MSYS2. Thus, it is not usable.

Oops, I thought even the old MSYS kept the same cygwin1.dll runtime name, but even that has msys-1.0.dll instead. Oh well, let's get it compiling properly.

Testing __GNUC__ == 3 might be ok, but how about __NEWLIB__? That's actually nice because it's really newlib that's supposed to be providing all those functions. In current Cygwin, /usr/include/sys/features.h has:

#define __NEWLIB__              2
#define __NEWLIB_MINOR__        2

In MSYS, these are totally missing. Actually newlib's ChangeLog says they were only added on 2014-09-17. If MSYS2 does have them, then maybe we should test defined(__MSYS__) && !defined(__NEWLIB__) to guard the compatibility stuff.

All this for functionality that's not even strictly necessary, as compat.h demonstrates. But it makes the main code nicer IMO. I think we're close though.

In case we need another option, #ifndef SOCK_CLOEXEC might be a sufficient canary, though it's annoyingly indirect.

I can confirm that MSYS2 has __NEWLIB__ defined in the same way than Cygwin does.

Using the proposed defined(__MSYS__) && !defined(__NEWLIB__) in compat.h reduces the amount of compilation errors to:

$ make
gcc -O2 -Werror -Wall -Wextra -MMD   -c -o main.o main.c
In file included from main.c:29:0:
winpgntc.h:21:23: error: conflicting types for ‘uint32_t’
 typedef unsigned long uint32_t;
                       ^
In file included from /usr/lib/gcc/x86_64-pc-msys/4.9.2/include/stdint.h:9:0,
                 from /usr/include/cygwin/socket.h:19,
                 from /usr/include/sys/un.h:15,
                 from compat.h:16,
                 from main.c:11:
/usr/include/stdint.h:34:22: note: previous declaration of ‘uint32_t’ was here
 typedef unsigned int uint32_t;
                      ^
<builtin>: recipe for target 'main.o' failed
make: *** [main.o] Error 1

Ah, there's a small MSYS conditional in winpgntc.h too - that can get the same update.

Indeed, fixing winpgntc.h did the trick and now ssh-pageant works nicely! Thanks for the development of this very helpful tool and your support in resolving the compilation issue!

Thanks for the PR -- next time I do a release, I'll try to make MSYS2 builds too.

Sorry for commenting after this bug report has closed. I can also confirm success compiling and using ssh-pageant under MSYS2. The essential condition to build under MSYS2 is setting MSYSTEM=MSYS before launching bash (which is done by bundled msys2_shell.bat), and install the necessary variant of header package (mostly msys2-runtime-devel).

Have been attempting mingw64 build but no luck so far 😞

Just as a by-the-way, ssh-pageant works well when I am interacting via the msys2 shell, but not when I use the mingw-w64-64 bit shell -- I assume it requires using the cygwin dll