Suppress socket imports that might not exist on some systems

[CoolCat467]

This pull request adds import exception suppression to a few imports from the socket library that apparently might not exist on all systems. Fixes #2915.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6547313) 99.64% compared to head (6d035ef) 99.64%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2916   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files         116      116           
  Lines       17477    17479    +2     
  Branches     3133     3134    +1     
=======================================
+ Hits        17415    17417    +2     
  Misses         43       43           
  Partials       19       19           

Files	Coverage Δ
src/trio/socket.py	100.00% <100.00%> (ø)

[Cheaterman]

LGTM 👍 thanks!

EDIT: OTOH I can't help but notice the code uses as to re-export using the same name, this can probably be omitted for clarity?

[jakkdl]

EDIT: OTOH I can't help but notice the code uses as to re-export using the same name, this can probably be omitted for clarity?

this is for pyright --verifytypes reasons. See #2625
(there's a comment to that effect further up in the file, but its not immediately visible in the diff view)

[jakkdl]

This is a partial revert of #2887 which broke these out.
The logic behind the change seems to be that support for these were added to windows in 3.8, and with dropping <3.8 it was now only pypy that didn't support them.

If we want to be extra thorough it'd perhaps be nice if we could add a conditional that is only true for the failing android API. @Cheaterman do you know of any sys.something variables that would be useful? The documentation says it's unavailable on Emscriptem and WASI: https://docs.python.org/3/library/socket.html#socket.if_indextoname - so maybe that's relevant? Otherwise I'm quite curious why they are missing.

I suppose building on android could maybe be considered for #2885

@Cheaterman have you tried building against this branch, or with local changes fixing this import, to verify these are the only problematic imports on your system? Does the test suite run correctly otherwise?

API 24 was released in august 2016 - which is very old by python standards: python3.8 was released in 2019. So I'm a bit wary that even if we fixed this import there'd be a lot of additional problems hiding.

[A5rocks]

This makes sense to me and while I would prefer to gate this rather than suppress import errors (so we have documentation for exactly what platforms fail in form of code), this works fine too.

That other approach might be too much work for something so small!

[Cheaterman]

@jakkdl Thanks for all the comments! I'm not aware of a sys module variable or anything else that would indicate the minimum supported Android API ; in fact that's why I was surprised to see this error happen in the first place, it should largely not be visible from Python.

I didn't get to test against this branch just yet - bumping minimum API is acceptable for my current application and other users I interacted with, in fact we might also bump the default in P4A to be thorough.

The reason it does matter to support such an old API version is simply because this isn't the target API but the minimum supported one, which means (AIUI at least) devices older than that will not be able to run the app. In practice I agree the ever-shrinking pool of 7+ year old devices isn't a huge concern, but it matters to some users ; and given we're actively recommending people switch to trio these days so they avoid all the threading footguns, we'd like to make their experience as smooth as possible :-).

[jakkdl]

@Cheaterman
Ah, P4A seems relevant. Is this possibly a bug on their side then? https://github.com/kivy/python-for-android
Glancing through the docs I don't see any reason they wouldn't support these socket imports - though they state the minimum version is 3.7 on PyPI and the github README, but 3.8 in the docs, so maybe there's something there.
Does it change if explicitly setting the python version in the build options?

[A5rocks]

-- snip --

OK, I found where all 3 attributes here are defined and commit date (to add if_nameindex) seems to point to January 2016. Checking Wikipedia's list of Android API versions seems to say that the next-released API version was 24, in August 2016.

Makes sense to me.

[Cheaterman]

Hi @A5rocks and thanks for your comments, I think you are entirely correct (only if_nameindex was introduced in 24) :-)

Hi @jakkdl and thanks for your work, P4A is indeed very relevant but the reason we were caught offguard by this is that the name will be unavailable on even Python 3.11 if building against minAPI < 24. Either way, I completely forgot to mention that yes, this is indeed being addressed on the P4A side, simply by bumping up the default minAPI to 24 ; but as you might guess, this isn't an ideal fix for people wanting to use recent trio versions while targeting older Android devices, that's why IMHO both are necessary.

[A5rocks]

Just a nitpick. I'm fine whether or not you update the comment.

[A5rocks]

For what it's worth: as far as I can tell, this is just a CPython thing. Though that's a guess based on how CPython is pulling it in (see https://bugs.python.org/msg279495). I kinda assume this is just building CPython for Android, no specific changes.

[CoolCat467]

I think 9dda073 should be sufficient