Skip to content

Conversation

Vizonex
Copy link

@Vizonex Vizonex commented Aug 5, 2025

Changes

Fixes #959

This attempts to add winloop support to anyio. And is my second attempt. This time I not hook pre-commit so that stressful things don't happen again. I am going to be doing these steps one at a time so that I can be guided in the right direction instead of screwing up.

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

  • You've added tests (in tests/) added which would fail without your patch
  • You've updated the documentation (in docs/, in case of behavior changes or new
    features)
  • You've added a new changelog entry (in docs/versionhistory.rst).

If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.

Updating the changelog

If there are no entries after the last release, use **UNRELEASED** as the version.
If, say, your patch fixes issue #123, the entry should look like this:

- Fix big bad boo-boo in task groups
  (`#123 <https://github.com/agronholm/anyio/issues/123>`_; PR by @yourgithubaccount)

If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.

@agronholm
Copy link
Owner

I specifically asked on Gitter to use use_uvloop. Why did you change your implementation?

@agronholm
Copy link
Owner

I also mentioned augmenting the uvloop extra in pyproject.toml to install winloop on Windows.

@Vizonex
Copy link
Author

Vizonex commented Aug 5, 2025

@agronholm I'm fixing the issue. one moment I added winloop to pyproject.toml I just forgot to register it. As for the flag I'm reverting that idea.

@Vizonex
Copy link
Author

Vizonex commented Aug 5, 2025

@agronholm I'll make sure to grab what I previously had for documentation about winloop real quick.

@agronholm
Copy link
Owner

Do you intend to finish this PR?

@Vizonex
Copy link
Author

Vizonex commented Aug 18, 2025

Do you intend to finish this PR?

Yeah I was waiting on you to review or see if changes were required seems 3.9 & 3.13 are failing for some unknown reason.

@Vizonex Vizonex requested a review from agronholm August 18, 2025 18:09
@agronholm
Copy link
Owner

There's no point in asking for review if the tests don't pass. We already discussed the necessary changes on Gitter / Matrix. The DNS resolution needs the Windows special casing turned off when run with winloop, and as for the extra_groups and umask kwargs to asyncio.open_subprocess() are concerned, you can either choose to implement them (essentially declare and then ignore as they don't apply to Windows AFAIK) or adjust the tests to ignore them like we do with uvloop.

@agronholm
Copy link
Owner

I will approve this once you change the test skipping to use the same method as with uvloop. It's okay to add a utility function in the test suite to check for winloop/uvloop as you would now need to duplicate the same nontrivial logic.

@agronholm
Copy link
Owner

Oh, and do add that changelog entry which you already had in your previous PR.

@Vizonex Vizonex changed the title add winloop flag for windows support add winloop for windows if use_uvloop is used on windows Aug 19, 2025
@Vizonex Vizonex requested a review from agronholm August 19, 2025 23:34
@Vizonex
Copy link
Author

Vizonex commented Aug 22, 2025

is there anything left or is this mergeable?

@Vizonex Vizonex requested a review from agronholm August 23, 2025 15:51
@agronholm
Copy link
Owner

Why is there an added winloop skip in test_run_process()? It makes no sense. Does running subprocesses really not work on winloop?

@agronholm
Copy link
Owner

is there anything left or is this mergeable?

I need the above question resolved before I merge.

@Vizonex
Copy link
Author

Vizonex commented Aug 24, 2025

Why is there an added winloop skip in test_run_process()? It makes no sense. Does running subprocesses really not work on winloop?

@agronholm there seems to be a bug that doesn't want it to pass and seems to be a problem with the library itself maybe. I'll see about making a duplicate of the test but basically the one that is failing is tests/test_subprocesses.py::test_run_process[asyncio+uvloop-shell] for some strange reason. I'll see about patching it in a later update.

@agronholm
Copy link
Owner

Why is there an added winloop skip in test_run_process()? It makes no sense. Does running subprocesses really not work on winloop?

@agronholm there seems to be a bug that doesn't want it to pass and seems to be a problem with the library itself maybe. I'll see about making a duplicate of the test but basically the one that is failing is tests/test_subprocesses.py::test_run_process[asyncio+uvloop-shell] for some strange reason. I'll see about patching it in a later update.

I'd really like to see this fixed in winloop before this PR is merged. The next release of AnyIO is some time away given how v4.10.0 was just released, so there's plenty of time.

@Vizonex Vizonex mentioned this pull request Aug 25, 2025
5 tasks
@Vizonex
Copy link
Author

Vizonex commented Aug 25, 2025

@agronholm I have opened a draft in my repository for fixing subprocess related bugs. I will be working it on/off depending on my part-time job's schedule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants