Summary: | Ensure webkitpy secret storage works successfully on Linux using only autoinstalled libraries | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||||||
Component: | Tools / Tests | Assignee: | Michael Catanzaro <mcatanzaro> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | ews-watchlist, glenn, jbedard, mcatanzaro, pnormand, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | Linux | ||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=220736 https://bugs.webkit.org/show_bug.cgi?id=221029 https://bugs.webkit.org/show_bug.cgi?id=230540 https://bugs.webkit.org/show_bug.cgi?id=234499 |
||||||||||||||
Attachments: |
|
Description
Michael Catanzaro
2021-10-07 11:40:17 PDT
Created attachment 440525 [details]
Patch
I think it's https://bugs.webkit.org/show_bug.cgi?id=231055 (In reply to Jonathan Bedard from comment #2) > I think it's https://bugs.webkit.org/show_bug.cgi?id=231055 Never mind, thought that change modified libraries, it does not. (In reply to Jonathan Bedard from comment #3) > Never mind, thought that change modified libraries, it does not. First problem is that python-secretstorage is not being autoinstalled, it needs to be registered as an implicit dep of python-keyring when running on Linux. But that's not enough, something else is wrong too. I will try to debug. Next problem is I tried updating to the latest python-keyring 23.2.1 (only when called from python3, since it is incompatible with python2), but it throws an exception whenever using any of the public APIs. I've tracked it down to _load_plugins in backend.py which is calling importlib.metadata.entry_points() https://docs.python.org/3.10/library/importlib.metadata.html#entry-points using the `group` keyword argument, which is documented to exist in python 3.10, but webkitpy's autoinstaller overrides the stdlib version with version 1.7.0 from pypy, which has an incompatible API. Not sure how hard it is to upgrade that yet. When I downgrade to python-keyring 22.3.0, the last version compatible with our importlib.metadata, then I get past that error and uncover another exception, which I will need to investigate tomorrow. (I also tried not updating anything, but that didn't work either, and it feels more productive to debug the newer versions rather than older versions.) Seems like I will need to fix out the bug where a version like 1.2.0 matches 1.2.1 (In reply to Jonathan Bedard from comment #6) > Seems like I will need to fix out the bug where a version like 1.2.0 matches > 1.2.1 Um... I didn't know about this? What bug is this? (In reply to Michael Catanzaro from comment #7) > (In reply to Jonathan Bedard from comment #6) > > Seems like I will need to fix out the bug where a version like 1.2.0 matches > > 1.2.1 > > Um... I didn't know about this? What bug is this? This bug: https://bugs.webkit.org/show_bug.cgi?id=230540, looking at a fix now Ah OK, interesting. I think that's not causing any trouble for *this* bug, but good to know about that footgun.... Created attachment 440659 [details]
Patch
(In reply to Michael Catanzaro from comment #5) > When I downgrade to python-keyring 22.3.0, the last version compatible with > our importlib.metadata, then I get past that error and uncover another > exception, which I will need to investigate tomorrow. Well I'm not sure what I changed, but I'm no longer seeing the same error as yesterday. Today I'm seeing python-secretstorage is broken due to a missing dependency on python-jeepney, which causes an ImportError when keyring imports secretstorage, causing the backend to be disabled. Autoinstalling jeepney fixes it. Created attachment 440661 [details]
Patch
(In reply to Michael Catanzaro from comment #4) > First problem is that python-secretstorage is not being autoinstalled, it > needs to be registered as an implicit dep of python-keyring when running on > Linux. Hm, it works fine for me today. python-keyring does import python-secretstorage, so it shouldn't need to be listed as an implicit dep. Not sure why I convinced myself this was required yesterday. Maybe it was required with older versions of the libraries, but it works fine for me with the newer versions. Created attachment 440663 [details]
Patch
(In reply to Michael Catanzaro from comment #14) > Created attachment 440663 [details] > Patch OK, this is a real patch, not a test patch. Danger of using 'webkit-patch upload' as a test case is that it uploads a patch when your changes work. :P Comment on attachment 440663 [details]
Patch
Macs are happy with this!
Comment on attachment 440663 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440663&action=review > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/__init__.py:75 > +if sys.version_info > (3, 6): Oops, should be >= (In reply to Michael Catanzaro from comment #17) > Oops, should be >= Also forgot to edit the webcorepy version number in setup.up and this __init__.py, although I don't quite understand why we are doing that. Created attachment 440675 [details]
Patch
(In reply to Michael Catanzaro from comment #18) > (In reply to Michael Catanzaro from comment #17) > > Oops, should be >= > > Also forgot to edit the webcorepy version number in setup.up and this > __init__.py, although I don't quite understand why we are doing that. Apple has a pypi instance we're uploading this to, the version numbers indicate a new release. Not a huge deal with changes like this, though. It's more important when APIs are changing. Committed r283843 (242723@main): <https://commits.webkit.org/242723@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440675 [details]. Looks like this broke keyring when queried from the Flatpak SDK (again). Thanks Michael :/ Hah, sorry, false alarm. After reinstalling the auto-installed python things I see jeepney is installed now, all good. Unfortunately all the keyring stuff is quite fragile. :/ I've tried to make it a little more robust by relying on the autoinstaller, but it will surely break again next time we update any of these dependencies. If anyone else has trouble after this, please delete your autoinstalled directories Tools/Scripts/libraries/autoinstalled and Tools/Scripts/webkitpy/autoinstalled. That will trigger the autoinstaller to run again and refresh everything. |