Skip to content

gh-77589: Add unix domain socket for Windows#137420

Open
aisk wants to merge 15 commits into
python:mainfrom
aisk:windows-unix-socket
Open

gh-77589: Add unix domain socket for Windows#137420
aisk wants to merge 15 commits into
python:mainfrom
aisk:windows-unix-socket

Conversation

@aisk
Copy link
Copy Markdown
Member

@aisk aisk commented Aug 5, 2025

Didn't add asyncio support in this PR to avoid too many code changes and keep the review process simpler.

Comment thread PC/pyconfig.h Outdated
@aisk aisk changed the title gh-77589: Windows unix socket gh-77589: Add unix domain socket for Windows Aug 5, 2025
@aisk aisk requested a review from gpshead as a code owner August 5, 2025 16:12
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Instead of skipping some tests, would not be worth to test a Windows specific behavior?

Some tests can currently be skipped on Windows, but they may work after adding support of AF_UNIX. Please check all currently skipped tests for socket and multiprocessing.

Comment thread Lib/test/test_socket.py
Comment thread Lib/test/test_socket.py
Comment thread Lib/test/test_pathlib/test_pathlib.py Outdated
Comment thread Lib/test/test_stat.py
@serhiy-storchaka serhiy-storchaka requested a review from zooba August 5, 2025 18:54
@serhiy-storchaka
Copy link
Copy Markdown
Member

It seems that the previous attempt did have more changes in the socket and socketserver modules. Are they no longer relevant?

@aisk
Copy link
Copy Markdown
Member Author

aisk commented Aug 6, 2025

It seems that the previous attempt did have more changes in the socket and socketserver modules. Are they no longer relevant?

The previous attempt modified the socketpair function in the socket module to use AF_UNIX by default on Windows, so it would introduce more changes. I'd like to keep the current behavior to reduce code changes in one PR and compatibility break risk, and we can change it in the future.

The change in socketserver in the previous attempt is mainly because in asyncio's test case, it tries to create an HTTP server over unix domain sockets. On Windows, currently AF_UNIX doesn't support reuse_address, so there needs to be this modification. I changed the test to disable reuse_address on Windows with AF_UNIX. But I think the previous attempt is the right way because users can encounter this issue too. Will update this later today. Thank you for the review!

@aisk aisk requested a review from vsajip as a code owner August 7, 2025 15:54
@aisk aisk marked this pull request as draft August 9, 2025 15:21
@aisk aisk marked this pull request as ready for review August 11, 2025 15:40
@aisk
Copy link
Copy Markdown
Member Author

aisk commented Aug 11, 2025

Instead of skipping some tests, would not be worth to test a Windows specific behavior?

Some tests can currently be skipped on Windows, but they may work after adding support of AF_UNIX. Please check all currently skipped tests for socket and multiprocessing.

Hi, I updated the test to test Windows-specific behaviors, and checked current tests which can be enabled after we added AF_UNIX, but not finding one. Most tests are skipped by some stuff like 'skipUnless(hasattr(socket, "AF_UNIX))'.

There is a noticeable remark for the reviewer: The PC\pyconfig.h is maintained by hand, so we don't have a configure step to detect if current build environment have AF_UNIX support (have afunix.h). So I introduced __has_include which haven't seen in current repository. Please let me know if there are better way to do this.

@github-actions
Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 27, 2026
@abel1502
Copy link
Copy Markdown
Contributor

What's the status of this PR? It seems ready for merge, are there any standing issues?

@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants