-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
gh-114099 - Add iOS framework loading machinery. #116454
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
Changes from 7 commits
e907723
1359bc8
5e0659e
d5fda7e
284e225
9f42faa
8308611
fa3ffbb
dbf818d
e66aee1
04d1c79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ | |
|
||
# Bootstrap-related code ###################################################### | ||
_CASE_INSENSITIVE_PLATFORMS_STR_KEY = 'win', | ||
_CASE_INSENSITIVE_PLATFORMS_BYTES_KEY = 'cygwin', 'darwin' | ||
_CASE_INSENSITIVE_PLATFORMS_BYTES_KEY = 'cygwin', 'darwin', 'ios', 'tvos', 'watchos' | ||
_CASE_INSENSITIVE_PLATFORMS = (_CASE_INSENSITIVE_PLATFORMS_BYTES_KEY | ||
+ _CASE_INSENSITIVE_PLATFORMS_STR_KEY) | ||
|
||
|
@@ -1711,6 +1711,57 @@ def __repr__(self): | |
return f'FileFinder({self.path!r})' | ||
|
||
|
||
class AppleFrameworkLoader(ExtensionFileLoader): | ||
"""A loader for modules that have been packaged as frameworks for | ||
compatibility with Apple's iOS App Store policies. | ||
""" | ||
def create_module(self, spec): | ||
mod = super().create_module(spec) | ||
if spec.loader_state.origfile is not None: | ||
mod.__file__ = spec.loader_state.origfile | ||
return mod | ||
|
||
|
||
class AppleFrameworkFinder: | ||
"""A finder for modules that have been packaged as Apple Frameworks | ||
for compatibility with Apple's App Store policies. | ||
|
||
See AppleFrameworkLoader for details. | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this a meta-path finder and not a path-entry finder? In other words, why don't you subclass FWIW, I think I know the answer already. 😄 Correct me if I'm wrong, but that would require That said, would it be feasible to write a custom One reason I bring it up is because, any time we step outside the normal flow of things, we run the risk of disrupting users. For example, in this case a user might expect extension modules to be found via a path-entry finder rather than a meta-path finder. They might mess with things under that assumption and then get grumpy when things break (most likely mysteriously). Of course the risk here is supremely small, but users have a habit of surprising us. 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, what should one expect Also, if I've understood correctly then extension modules can only be found under the app's frameworks folder and never on any other IIUC, .py files may still be imported using the normal path-entry finder ( Relatedly, what happens if an app maintainer (or, somehow, a user) registers additional importers or messes with
FYI, the import machinery already makes this (almost) a non-issue with path hooks and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're correct that the reason for the metapath finder is the structure of the Frameworks folder. I hadn't considered using a custom FileFinder; I agree we should do anything we can to avoid breaking user expectations (however eccentric or unexpected those might be), so I'll take a look and see if I can make this work. As for ExtensionFileLoader - that's an interesting case. iOS apps are entirely capable of loading binary modules from the PYTHONPATH; it's entirely an issue of the resulting app being acceptable to the App Store. If you've got a project that doesn't have the "move dylibs to the Frameworks folder" build step, the app will still run - it will just be rejected when you try to submit it to the App store (and it will be rejected by an automated check at time of submission, not hours/days/weeks later as a result of the review process). I've also had a couple of bugs (including one I'm still chasing with cryptography) where it's useful to be able to confirm if the problem you're seeing is because the .dylib has been moved, or because the .dylib isn't working. I therefore opted to retain the ExtensionFileLoader, just in case it was useful. The SourceFileLoader and SourcelessFileLoader both work exactly the same, however. Loading If a user installs an additional importer... I guess that depends on what the importer is doing. The entire contents of the app bundle is fair game for reading; so a novel mechanism for finding .py files shouldn't be an issue. The only place I could force a problem is if the importer is expecting to find an executable binary in a specific location outside the Frameworks folder - but the app won't be accepted into the App Store in the first place if they try to do this. As for circumventing the App Store rules - it might be a lack of imagination on my part, but I'm having difficulty imaging what you'd be able to do here. Normal filesystem write protections should prevent even a malicious user from doing anything damaging, and you won't be able to use anything binary that isn't in the app bundle and signed on that basis. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. Thanks for the detailed explanation! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Let me know if you have any questions on this. |
||
def __init__(self, frameworks_path): | ||
self.frameworks_path = frameworks_path | ||
|
||
def find_spec(self, fullname, paths, target=None): | ||
name = fullname.rpartition(".")[-1] | ||
|
||
for extension in EXTENSION_SUFFIXES: | ||
ericsnowcurrently marked this conversation as resolved.
Show resolved
Hide resolved
|
||
dylib_file = _path_join( | ||
self.frameworks_path, | ||
f"{fullname}.framework", | ||
f"{name}{extension}", | ||
) | ||
_bootstrap._verbose_message("Looking for Apple Framework dylib {}", dylib_file) | ||
|
||
dylib_exists = _path_isfile(dylib_file) | ||
if dylib_exists: | ||
origfile = None | ||
if paths: | ||
for parent_path in paths: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly, when importing a top-level module, In fact, I don't think it could be rewritten with the information available. If an app has both the standard library and some other source of binary modules on Maybe the framework name needs some additional dotted segments at the start, so it effectively represents the "original" path relative to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new loader implementation I've just pushed should resolve this. It leaves a |
||
if _path_isdir(parent_path): | ||
origfile = _path_join( | ||
parent_path, | ||
_path_split(self.path)[-1], | ||
) | ||
break | ||
loader = AppleFrameworkLoader(fullname, dylib_file) | ||
spec = _bootstrap.spec_from_loader(fullname, loader) | ||
spec.loader_state = type(sys.implementation)( | ||
origfile=origfile, | ||
) | ||
return spec | ||
|
||
return None | ||
|
||
# Import setup ############################################################### | ||
|
||
def _fix_up_module(ns, name, pathname, cpathname=None): | ||
|
@@ -1760,3 +1811,7 @@ def _install(_bootstrap_module): | |
supported_loaders = _get_supported_file_loaders() | ||
sys.path_hooks.extend([FileFinder.path_hook(*supported_loaders)]) | ||
sys.meta_path.append(PathFinder) | ||
if sys.platform in {"ios", "tvos", "watchos"}: | ||
frameworks_folder = _path_join(_path_split(sys.executable)[0], "Frameworks") | ||
ericsnowcurrently marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_bootstrap._verbose_message("Adding Apple Framework dylib finder at {}", frameworks_folder) | ||
sys.meta_path.append(AppleFrameworkFinder(frameworks_folder)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reworked this sentence, so the suggestion no longer applies.