Skip to content
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.

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

certifi 2022.6.15.1 broken on Python 3.9 and 3.10 under third-party importers like PyOxidizer #203

Closed
tsibley opened this issue Sep 12, 2022 · 6 comments · Fixed by #204
Closed

Comments

@tsibley
Copy link
Contributor

tsibley commented Sep 12, 2022

The recently landed Python 3.11 deprecation warning fix released in 2022.6.15.1 on 9 Sept 2022 causes certifi to throw a ValueError on Python 3.10 and a TypeError on Python 3.9 when using an importer (like PyOxidizer's OxidizedImporter) that doesn't support the relatively-new "Traversable" / files() importlib API. This is because the full adapter layer for the older importlib resources API doesn't exist until Python 3.11.

On 3.10, the error is intentionally raised in the stdlib's DegenerateFiles class:

https://github.com/python/cpython/blob/3.10/Lib/importlib/_adapters.py#L53-L54

...
  File "certifi.core", line 37, in where
  File "contextlib", line 135, in __enter__
  File "importlib._common", line 89, in _tempfile
  File "importlib.abc", line 371, in read_bytes
  File "importlib._adapters", line 54, in open
ValueError

because the "adapter" in 3.10 exists only to fail, it seems.

On 3.9, the error is because the stdlib's files() function assumes that certifi.__file__ (as certifi.__spec__.origin) is not None:

https://github.com/python/cpython/blob/3.9/Lib/importlib/_common.py#L17-L18

…
  File "certifi.core", line 36, in where
  File "importlib.resources", line 147, in files
  File "importlib._common", line 14, in from_package
  File "importlib._common", line 18, in fallback_resources
  File "pathlib", line 1082, in __new__
  File "pathlib", line 707, in _from_parts
  File "pathlib", line 691, in _parse_args
TypeError: expected str, bytes or os.PathLike object, not NoneType

Two separate possible solutions come to mind:

  1. Gate the usage of the Traversable importlib API on Python ≥3.11 instead of ≥3.9, thus letting 3.9 and 3.10 use the older, more established API more likely to be supported by third-party importers.
  2. Gate the usage of the same on a feature-test for support by the actual certifi.__loader__ in addition to the Python version. I'm not sure how to actually do this at the moment, but I assume it's possible.

Option 1 certainly seems simpler to me.

@alex
Copy link
Member

alex commented Sep 12, 2022

Thank you for the detailed and well thought out report.

Unfortunately it fills me with exhaustion -- which isn't your fault. This package is trying to do the incredibly simple thing of taking a .pem file and shipping it with pip for python users. Doing so is producing an incredibly complicated edifice for properly interacting with the packaging-verse, none of which is stuff the maintainers actually care about, its all a tax to do the one thing we want: put a .pem on PyPI.

Anyhoo, that complaint aside: Your proposal (1) makes sense to me... but my question is, if we do that, who is going to complain that we broke something? It's impossible for me to reason about the backwards compatibility surfaces on any of this.

@tsibley
Copy link
Contributor Author

tsibley commented Sep 12, 2022

I can sympathize! I'm also exhausted from tracing this all down and it derailing the work I actually wanted to be doing. Thank you for considering a fix here, and regardless of anything else, thank you for all your hard work making certifi a reliable module that (nearly all the time!) Just Works.

I understand your hesitation to make a change, but I'd be hopeful that doing (1) wouldn't break anything for anyone given that

  • it essentially reverts that codepath for 3.9 and 3.10 back to a previous version, that presumably worked fine,
  • it still avoids any new deprecation warnings in 3.11 by using the new codepath there, and
  • the new codepath's only been in the wild for 3-4 days at this point.

@alex
Copy link
Member

alex commented Sep 12, 2022

Makes sense to me. I'll let @Lukasa chime in, but based on this I think I'd personally approve a PR to change the version check to 3.11

@tsibley
Copy link
Contributor Author

tsibley commented Sep 12, 2022

Digging into git history a little, I see that the codepath it'd be reverting to for 3.9 and 3.10 has existed in certifi since April 2020 (3fc8fec, #123), and it's the same code path that 3.7 and 3.8 still use.

@Lukasa
Copy link
Member

Lukasa commented Sep 13, 2022

I'm happy to follow (1).

tsibley added a commit to tsibley/python-certifi that referenced this issue Sep 13, 2022
…n ≥3.11

Using importlib.resource's files() API on 3.9 and 3.10 causes a
TypeError on 3.9 and a ValueError on 3.10 when running under a
third-party meta path importer (like PyOxidizer's OxidizedImporter) that
doesn't support the relatively-new API.  This is because the full
adapter layer (importlib.resources._adapters) for the older importlib
resources API doesn't exist until Python 3.11.

The older resources API is now used by 3.7–3.10, as it was prior to the
certifi 2022.06.15.1 release.  This codepath has existed in certifi
since April 2020 (3fc8fec).

An alternative to this change would be testing the actual importer in
use at runtime (e.g. certifi.__loader__) for files() support, but that
seemed more complex than reverting to the previous codepath here.

Resolves: certifi#203
Related-to: certifi#199
Related-to: certifi#123
@tsibley
Copy link
Contributor Author

tsibley commented Sep 13, 2022

@alex @Lukasa Submitted #204 to implement (1).

Lukasa pushed a commit that referenced this issue Sep 13, 2022
…n ≥3.11 (#204)

Using importlib.resource's files() API on 3.9 and 3.10 causes a
TypeError on 3.9 and a ValueError on 3.10 when running under a
third-party meta path importer (like PyOxidizer's OxidizedImporter) that
doesn't support the relatively-new API.  This is because the full
adapter layer (importlib.resources._adapters) for the older importlib
resources API doesn't exist until Python 3.11.

The older resources API is now used by 3.7–3.10, as it was prior to the
certifi 2022.06.15.1 release.  This codepath has existed in certifi
since April 2020 (3fc8fec).

An alternative to this change would be testing the actual importer in
use at runtime (e.g. certifi.__loader__) for files() support, but that
seemed more complex than reverting to the previous codepath here.

Resolves: #203
Related-to: #199
Related-to: #123
tsibley added a commit to nextstrain/cli that referenced this issue Sep 13, 2022
This sidesteps issues with the relatively-new files() / Traversable API
of importlib.resources that is present in 3.9 and 3.10 but not supported
by PyOxidizer's OxidizedImporter.  It only supports the original
importlib.resources API and importlib.resources doesn't implement a
backwards compatibility layer/adapter until Python 3.11 (not yet
released).  By using 3.8, code which tests for the presence of the new
API in importlib.resources or tests the Python version will not detect
stdlib support for the new API and thus will generally fall back to the
API supported by PyOxidizer.

Related-to: <certifi/python-certifi#203>
tsibley added a commit to nextstrain/cli that referenced this issue Sep 23, 2022
This sidesteps issues with the relatively-new files() / Traversable API
of importlib.resources that is present in 3.9 and 3.10 but not supported
by PyOxidizer's OxidizedImporter.  It only supports the original
importlib.resources API and importlib.resources doesn't implement a
backwards compatibility layer/adapter until Python 3.11 (not yet
released).  By using 3.8, code which tests for the presence of the new
API in importlib.resources or tests the Python version will not detect
stdlib support for the new API and thus will generally fall back to the
API supported by PyOxidizer.

Related-to: <certifi/python-certifi#203>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants