Skip to content

windows: Implement socket/ssl modules #12810

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

windows: Implement socket/ssl modules #12810

wants to merge 9 commits into from
+402 −50

Conversation

stinos
Copy link
Contributor

@stinos stinos commented Oct 26, 2023 •
edited
Loading

Socket module is enabled for mingw and msvc builds, mbedtls only for the former; for no particular reason, I can add it there as well if that makes more sense.

As for testing I manually ran the relevant multi_net/net_inet/net_hosted tests and tried the requests module both with and without SSL, and all of that works fine so it's likely that everything is done correctly.

The new multi_net/tcp_recv_timeout.py fails sometimes on ci, likely because one of the recv() calls errors out because the remote disconnected already, which I thought multitest.next() would make sure didn't happen but I probably misunderstood what it does. Someone has an idea for that? I just want the recv calls on both ends to time out.

The test fixes with the numeric values I'm not super happy about, but the only proper way of doing this is to provide common functions to check these errors in a test_helpers module or something like that, and then perhaps implementing a complete errno module for windows which does include all WSAXXX error code, which is a bit much maybe?

@codecov
Copy link

codecov bot commented Oct 26, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.43%. Comparing base (185116e) to head (1f4266d).

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #12810   +/-   ##
=======================================
  Coverage   98.43%   98.43%           
=======================================
  Files         163      163           
  Lines       21290    21290           
=======================================
  Hits        20956    20956           
  Misses        334      334

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Gadgetoid Gadgetoid mentioned this pull request Feb 29, 2024
Merged
@projectgus
Copy link
Contributor

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

@stinos stinos mentioned this pull request Mar 24, 2024
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 25, 2024 •
edited
Loading

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@stinos stinos force-pushed the winsock branch 4 times, most recently from 0aae1b9 to dc44e7c Compare March 25, 2024 13:48
@stinos
Copy link
Contributor Author

stinos commented Mar 25, 2024

Apart from the STATIC changes, this went rather stale all recent mbedtls changes as well, and the switch from Appveyor to Github Actions; I updated everything now. The 2 mingw standard variant builds currently fail, but there's something weird going on with how Github Actions runs these builds and I'm still looking into that.

@stinos
Copy link
Contributor Author

stinos commented Mar 26, 2024

The 2 mingw standard variant builds currently fail, but there's something weird going on with how Github Actions runs these builds and I'm still looking into that.

Fix in #14188

stinos added 9 commits August 20, 2024 15:31
@stinos
tests: Make socket tests compatible with CPython on windows.
341ca23 
Note these are mostly the 'bare' socket tests, not the ssl/tls ones
for instance: most of these don't run on CPython because of
incompatible wrap_socket() arguments.
The change mostly consists of checking the WSA error codes next to
the errno ones and these are written as numeric values because the
names (like WSAEAGAIN) are only available in CPython and not in
micropython.

Signed-off-by: stijn <stijn@ignitron.net>
@stinos
tests: Add socket timeout tests for connect and recv.
e18693d 
Signed-off-by: stijn <stijn@ignitron.net>
@stinos
unix/modsocket: Introduce typedefs and macros.
a0e15f1 
Upcoming commits are going to port this file to work with sockets
on windows so prepare for that with the platform-specific bits.

Signed-off-by: stijn <stijn@ignitron.net>
@stinos
unix/modsocket: Remove obsolete comment.
fd60536 
Signed-off-by: stijn <stijn@ignitron.net>
@stinos
windows: Implement socket module.
fc6a121 
Signed-off-by: stijn <stijn@ignitron.net>
@stinos
windows/msvc: Add a build target for updating submodules.
1580f44 
This is more consistent with the Makefile/CMake-based builds.

Signed-off-by: stijn <stijn@ignitron.net>
@stinos
windows/msvc: Introduce an msbuild variable for third-party sources.
8275f32 
This is consistent with extmod.mk's split between SRC_EXTMOD_C and
SRC_THIRDPARTY_C, the latter specifying source files which get
compiled but not included in qstr generation.

Signed-off-by: stijn <stijn@ignitron.net>
@stinos
windows/msvc: Fix preprocessing command for quoted definitions.
249b903 
Supports preprocessor definitions like /Dval="quotedvalue" by turning
that into /Dval=\"quotedvalue\".

Signed-off-by: stijn <stijn@ignitron.net>
@stinos
windows/msvc: Add ssl module using mbedtls in standard variant.
1f4266d 
Signed-off-by: stijn <stijn@ignitron.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
port-windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stinos @projectgus @dpgeorge