Bug 224669 - AutoInstaller should name directory based on ABI
Summary: AutoInstaller should name directory based on ABI
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-16 06:08 PDT by Sam Sneddon [:gsnedders]
Modified: 2023-10-03 18:35 PDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Sneddon [:gsnedders] 2021-04-16 06:08:56 PDT
Currently we install Python libraries in python-2 and python-3, based on:

Tools/Scripts/webkitpy/__init__.py
34:AutoInstall.set_directory(os.path.join(libraries, 'autoinstalled', 'python-{}'.format(sys.version_info[0])))

However, this is really an oversimplification of Python ABIs. Notably, while there's generally ABI compatibility between bug fix releases, there aren't between minor releases (e.g., Python 3.8 and Python 3.9).

Basing something on PEP 3147 (https://www.python.org/dev/peps/pep-3147/) probably makes a fair bit of sense. Essentially, on Python 3, we should at least use sys.implementation.cache_tag instead.

Given we do care about ABI here, we probably should even care about ABI detail; CPython uses SOABI (accessible via sysconfig.get_config_var("SOABI")) for this, but this only exists on POSIX-like systems. In reality, we probably don't need to store the platform in the string, so we can probably just use sys.implementation.cache_tag + sys.abiflags?

Opinions?
Comment 1 Sam Sneddon [:gsnedders] 2021-04-16 06:09:58 PDT
Oh, I meant to link PEP 3149 (https://www.python.org/dev/peps/pep-3149/) which covers ABI version tagged .so files.
Comment 2 Radar WebKit Bug Importer 2021-04-23 13:09:02 PDT
<rdar://problem/77083105>
Comment 3 Sam Sneddon [:gsnedders] 2023-08-21 15:30:00 PDT
Pull request: https://github.com/WebKit/WebKit/pull/16903
Comment 4 Sam Sneddon [:gsnedders] 2023-08-29 06:11:33 PDT
Because I was getting confused in what situation this wasn't working properly, to quote my comment from the PR:

> Pushing this because having it locally helps nobody. Keeping this as a draft because I'm now somewhat confused by the necessity of this—[PEP 3149](https://peps.python.org/pep-3149/) should make it safe, provided we shard the top-level directory well enough (which we do except in the face of OS upgrades). And yet I've frequently had things fail on Arch Linux when running webkitpy against differing CPython 3 versions, which… shouldn't happen. Should check what's going on there, and decide what to do based on that.

Running:

```
git clean -Xfd Tools/Scripts
python3.9 Tools/Scripts/test-webkitpy --all
python3.10 Tools/Scripts/test-webkitpy --all
```

fails with:

```
    File "/Volumes/gsnedders/projects/Safari/OpenSource/Tools/Scripts/libraries/autoinstalled/python-3-arm64/lupa/__init__.py", line 32, in <module>
      from lupa._lupa import *
  ModuleNotFoundError: No module named 'lupa._lupa'
```

And this happens because:

```
% find Tools/Scripts/libraries/autoinstalled/python-3-arm64 -name '_lupa.*'
Tools/Scripts/libraries/autoinstalled/python-3-arm64/lupa/_lupa.cpython-39-darwin.so
```

And because CPython 3.10 doesn't implement the CPython 3.9 ABI, this fails.

There's a variety of extension modules that get auto-installed, but most of these are optional "speedup" modules (or using the "Stable ABI"), hence most of the time this succeeds outside of our tests:

```
% find Tools/Scripts/libraries/autoinstalled/python-3-arm64 -name '*.so'   
Tools/Scripts/libraries/autoinstalled/python-3-arm64/selenium/webdriver/firefox/amd64/x_ignore_nofocus.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/selenium/webdriver/firefox/x86/x_ignore_nofocus.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/markupsafe/_speedups.cpython-39-darwin.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/cryptography/hazmat/bindings/_openssl.abi3.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/cryptography/hazmat/bindings/_rust.abi3.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/hiredis/hiredis.cpython-39-darwin.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/websockets/speedups.cpython-39-darwin.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/lupa/_lupa.cpython-39-darwin.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/_cffi_backend.cpython-39-darwin.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/coverage/tracer.cpython-39-darwin.so
Tools/Scripts/libraries/autoinstalled/python-3-arm64/zope/interface/_zope_interface_coptimizations.cpython-39-darwin.so
```
Comment 5 EWS 2023-09-20 09:55:30 PDT
Committed 268205@main (17ed49517923): <https://commits.webkit.org/268205@main>

Reviewed commits have been landed. Closing PR #16903 and removing active labels.
Comment 6 Jonathan Bedard 2023-09-20 20:23:23 PDT
Reverted by https://github.com/WebKit/WebKit/pull/18004
Comment 7 Jonathan Bedard 2023-09-20 20:23:25 PDT
Re-opening for pull request https://github.com/WebKit/WebKit/pull/18004
Comment 8 EWS 2023-09-20 20:38:25 PDT
Committed 268230@main (01641ad43827): <https://commits.webkit.org/268230@main>

Reviewed commits have been landed. Closing PR #18004 and removing active labels.
Comment 9 Sam Sneddon [:gsnedders] 2023-10-03 18:35:12 PDT
Reopening because of revert.