Skip to content

building: BUNDLE: exempt collected .py/.pyc files from relocation#7180

Merged
rokm merged 1 commit intopyinstaller:developfrom
rokm:macos-bundle-no-py-pyc-relocation
Oct 23, 2022
Merged

building: BUNDLE: exempt collected .py/.pyc files from relocation#7180
rokm merged 1 commit intopyinstaller:developfrom
rokm:macos-bundle-no-py-pyc-relocation

Conversation

@rokm
Copy link
Copy Markdown
Member

@rokm rokm commented Oct 22, 2022

In macOS .app bundles, exempt the .py and .pyc files that are collected as DATA files from being relocated from Contents/MacOS to Contents/Resources (and then symlinked back).

In certain scenarios (such as OpenCV loader's code), the path to a .py file needs to resolve to the directory that contains the adjacent binary extensions; i.e., the original directory in Contents/MacOS, beaucse the extensions are not relocated.

This might introduce regressions with bundle signing and notarization that we cannot test, hence the breaking change news entry. But to my knowledge, the main source of problems with that are dots in the directory names, which should not be the problem with directory- and base names of the collected .py/.pyc files.

Fixes #7128.

In macOS .app bundles, exempt the .py and .pyc files that are
collected as DATA files from being relocated from Contents/MacOS
to Contents/Resources (and then symlinked back).

In certain scenarios (such as OpenCV loader's code), the path to
a .py file needs to resolve to the directory that contains the
adjacent binary extensions; i.e., the original directory in
Contents/MacOS, beaucse the extensions are not relocated.

This might introduce regressions with bundle signing and
notarization that we cannot test, hence the breaking change
news entry. But to my knowledge, the main source of problems with
that are dots in the directory names, which should not be the
problem with directory- and base names of the collected .py/.pyc
files.
@bwoodsend
Copy link
Copy Markdown
Member

This might introduce regressions with bundle signing and notarization that we cannot test, hence the breaking change news entry. But to my knowledge, the main source of problems with that are dots in the directory names, which should not be the problem with directory- and base names of the collected .py/.pyc files.

What would it take to test it? A real codesigning certificate and an application containing import cv2? We could probably phone a friend if that's all we need (e.g. nyavramov from #7123 would be the first to spring to mind).

@rokm
Copy link
Copy Markdown
Member Author

rokm commented Oct 22, 2022

I think for codesigning test, the ad-hoc signing that we do by default is sufficient - if there's something that codesign doesn't like, signing the bundle fails (with a warning) - and for cv2 case, it seems to go through. I don't know if there are any extra requirements/restrictions for the bundle to pass notarization, though - that's the part we cannot really test without going through the process ourselves.

The regression part is meant more generally, though, as it might involve any package where we collect source .py files as DATA files (for example, tensorflow) - the breaking change entry is mostly intended for people to quickly pin-point a possible reason for their bundles suddenly failing.

@rokm rokm merged commit 46a5950 into pyinstaller:develop Oct 23, 2022
@rokm rokm deleted the macos-bundle-no-py-pyc-relocation branch October 23, 2022 11:41
if typ == 'DATA' and base_path not in _QT_BASE_PATH:
# Exempt python source files from this relocation, because their real path might need to resolve
# to the directory that also contains the extension module.
relocate_file = type == 'DATA' and base_path not in _QT_BASE_PATH
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rokm @bwoodsend A test is definitely needed as nothing caught the typo here type should be typ which means that no files are relocated ever

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. The effect of this error should be caught by the metadata collection test that will be added in #7187.

Thanks for investigating and fixing the regression!

@phw
Copy link
Copy Markdown

phw commented Oct 31, 2022

This might introduce regressions with bundle signing and notarization that we cannot test, hence the breaking change news entry. But to my knowledge, the main source of problems with that are dots in the directory names, which should not be the problem with directory- and base names of the collected .py/.pyc files.

I can give some background what kind of issues with signing you can expect when there is a .py file, as we had some serious trouble with this in the past on MusicBrainz Picard and it can be hard to figure out the root cause when users report broken signatures.

macOS expects every file in the app bundle to be digitally signed. For the binaries placed in Contents/MacOS this is usually straight forward, as the signature gets directly added to the file itself. But there are many types of files where you cannot add the digital signature to the file itself without potentially breaking the file. Text files, such as .py files belong to this group.

Now the app signing has two strategies how to still add signatures to these files. For files in Contents/Resources the app signing places the signatures in a separate file _CodeSignature/CodeResources. That's because Contents/Resources usually contains all kind of resources an app might need, and it is expected that macOS cannot code sign most of them.

But for files in Contents/MacOS the code signature instead gets placed inside a file extended attribute.

This second part is difficult and can lead to surprising results: After signing everything seems to be fine. If you check the signatures after building everything is ok. You can also package the files up in a .dmg image, distribute it, and it will seemingly work. But file attributes can easily get lost. E.g. if users copy the app to a file system without support for extended attributes the signature break (I think this happens when users have disks formatted with the old HFS file system instead of the newer APFS). Also zipping the app bundle can I think cause the attributes to get lost.

So if you want to have reliable signed app bundles placing file types for which the signature cannot be placed as part of the file itself in Contents/MacOS should be avoided, instead they should be placed in Content/Resources. If that's not possible one has to be aware of the limitation and potential user problems.

@almarklein
Copy link
Copy Markdown

almarklein commented Dec 20, 2022

This change broke Pyzo. We intentionally include .py files via --add-data to make the app hackable. Is there any way to turn this option off in specific situations?

@rokm
Copy link
Copy Markdown
Member Author

rokm commented Dec 20, 2022

This change broke Pyzo. We intentionally include .py files via --add-data to make the app hackable. Is there any way to turn this option off in specific situations?

No, but you can relocate the problematic files yourself once the bundle is generated.

@rokm
Copy link
Copy Markdown
Member Author

rokm commented Dec 20, 2022

Also, unless you prefer to have source directory in the Resources for other reasons (for example, due to signing, as mentioned in comment above), you could modify your custom source importer to use os.path.join(sys._MEIPASS, 'source') regardless of whether it is running in macOS app bundle mode or regular frozen application.

@almarklein
Copy link
Copy Markdown

Thanks for the detailed help 🙏 you even took the time to dive into the obscurities of our freezing process :)

@almarklein
Copy link
Copy Markdown

If I manually move some files around after PyInstaller is doen, it breaks the signatures, rendering the application "broken". Can I tell PyInstaller to redo the signing in such a case?

@rokm
Copy link
Copy Markdown
Member Author

rokm commented Feb 3, 2023

If I manually move some files around after PyInstaller is doen, it breaks the signatures, rendering the application "broken". Can I tell PyInstaller to redo the signing in such a case?

Not, but you can re-sign your bundle yourself after post-processing it.

@rokm
Copy link
Copy Markdown
Member Author

rokm commented Feb 3, 2023

If I manually move some files around after PyInstaller is doen, it breaks the signatures, rendering the application "broken". Can I tell PyInstaller to redo the signing in such a case?

Not, but you can re-sign your bundle yourself after post-processing it.

I.e., use the equivalent of the command PyInstaller is using here (and here):

codesign -s - --force --all-architectures --timestamp --deep --entitlements /path/to/entitlements-file.plist /path/to/bundle.app

for ad-hoc and

codesign -s YOUR-SIGNING-ID --force --all-architectures --timestamp --deep --options=runtime --entitlements /path/to/entitlements-file.plist /path/to/bundle.app

if you use signing cert.

@almarklein
Copy link
Copy Markdown

I decided to have a go at code signing with a proper certificate, on GH Actions. Bit of a rough ride, but it looks like it worked. I documented what I did on this PR: pyzo/pyzo#842

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error "ImportError: ERROR: recursion is detected during loading of "cv2" binary extensions. Check OpenCV installation." with Pyinstaller

5 participants