deiv commented on pull request #2235: URL: https://github.com/apache/thrift/pull/2235#issuecomment-696055493
> > I added a TODO, to take into account, that windows has support to unix sockets, but the code is not prepared for that. At least it needs to include the windows headers for unix sockets. See [af_unix-comes-to-windows](https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/) > > I have PR #2127 that adds support for Unix sockets under Windows. Generally its positive to have separate PRs for separate functionality, but in this case its not necessarily better that you need to guard _against_ supporting Unix sockets only for me to later add the support in a subsequent PR. So please feel free to cannibalize #2127 and take everything you need to support the Unix sockets on MSVC? Hi @emmenlau, I don't have a windows development enviroment, and because, to add support for Unix sockets, is needed a new header (and the checks for it), better to have it in another separate PR. And is needed to remove other windows conditional compilation in sockets sources files, too. Eg: ``` #ifndef _WIN32 socklen_t structlen = fillUnixSocketAddr(address, path_); ``` This one only fix abstract namespace, so better if you can do another PR removing the conditional compilation for windows. Seems more logical to have the new windows unix socket support separated. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
