Bug 229172 - [webkitcorepy] Return SourceFileLoader in find_module
Summary: [webkitcorepy] Return SourceFileLoader in find_module
Status: RESOLVED FIXED
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-08-16 17:33 PDT by Jonathan Bedard
Modified: 2021-08-17 15:16 PDT (History)
2 users (show)

See Also:


Attachments
Patch (3.94 KB, patch)
2021-08-16 17:35 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (3.93 KB, patch)
2021-08-17 13:07 PDT, Jonathan Bedard
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2021-08-16 17:33:18 PDT
On some of our bots, Python 3 scripts are failing because the auto installer is failing to load modules it has installed. This is because we aren't implementing everything required for a custom import system, namely, we should return a module loader.  Python 2 doesn't require this, and implementing a module loader in Python 2 is non trivial, and we intend to deprecate Python 2 in the near future.
Comment 1 Radar WebKit Bug Importer 2021-08-16 17:33:38 PDT
<rdar://problem/82006256>
Comment 2 Jonathan Bedard 2021-08-16 17:35:47 PDT
Created attachment 435650 [details]
Patch
Comment 3 Stephanie Lewis 2021-08-17 12:13:21 PDT
I don't think we can claim we can deprecate python2 until we stop testing on macOS 10 (at least for stuff engineers might use, I don't care about infra)
Comment 4 Jonathan Bedard 2021-08-17 12:22:28 PDT
(In reply to Stephanie Lewis from comment #3)
> I don't think we can claim we can deprecate python2 until we stop testing on
> macOS 10 (at least for stuff engineers might use, I don't care about infra)

That's what's weird about this particular case. Python 2 doesn't seem to need it (which is why we aren't having this problem on the bots now).  Actually, even Python 3 doesn't seem to always need it in practice, although I'm not entirely sure why, because Python 3's documentation indicates that we always need this.  I did look into a Python 2 implementation, but we need SourceFileLoader, which isn't trivial to upload. Since it doesn't seem like we need this in practice, this code works just fine right now with Python 2, I think we can get away with a Python 3 only implementation
Comment 5 Stephanie Lewis 2021-08-17 12:33:47 PDT
Comment on attachment 435650 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=435650&action=review

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:603
> +            # Python 2 works find with the default module finder, once we've installed the module in question

typo fine

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:609
> +            for trail in ('', '.py', '.pyc', '.py3', '.pyo'):

would ext be a better name than trail?

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:614
> +            if not os.path.isdir(part):

do you mean path here?
Comment 6 Jonathan Bedard 2021-08-17 12:37:44 PDT
(In reply to Stephanie Lewis from comment #5)
> Comment on attachment 435650 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=435650&action=review
> 
> > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:603
> > +            # Python 2 works find with the default module finder, once we've installed the module in question
> 
> typo fine
> 
> > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:609
> > +            for trail in ('', '.py', '.pyc', '.py3', '.pyo'):
> 
> would ext be a better name than trail?

Yes

> 
> > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:614
> > +            if not os.path.isdir(part):
> 
> do you mean path here?

Yes
Comment 7 Jonathan Bedard 2021-08-17 13:07:53 PDT
Created attachment 435704 [details]
Patch
Comment 8 EWS 2021-08-17 14:58:02 PDT
Found 1 new test failure: imported/w3c/web-platform-tests/navigation-timing/nav2_test_attributes_values.html
Comment 9 Jonathan Bedard 2021-08-17 15:16:22 PDT
Committed r281167 (240614@main): <https://commits.webkit.org/240614@main>