Builds pass macOS notarization #2025

johan-ronnkvist · 2023-08-31T15:18:20Z

This PR is intended to resolve this issue as its primary purpose, in the course of doing this we are also addressing some other smaller issues that are linked further down as well as extending the functionality of cx_Freeze's signing options.

Our intent with this PR is to focus on ensuring the bundle structure is valid with the minimal impact on the internals of the freezer itself -> we are aware of further optimizations that can be made which are listed below - but we'd prefer not to increase the scope beyond whats necessary.

In order to ensure that application bundles pass macOS notarization the essential changes we had to make included the following:

Changes:

Moved license file to Resources (The folder MacOS may only contain binaries)
Clean build directory between builds.
Perform inside-out codesign of all libs/binary files.
Added new parameters to control behavior of codesign
- codesign-verify
- codesign-timestamp
- codesign-strict
- codesign-options
- sptcl-assess
We now use --force when codesigning in order to replace existing signatures.

Upon making these changes and building + deploying a custom version of cx_freeze internally so our CI/CD could make use of it - Our internal pipeline that builds a .app bundle with cx_Freeze now runs and passes notarization by Apple.

We're submitting this as a draft for now for initial feedback as there are a couple of points worthy of further discussion before finalizing these changes.

Questions

The current implementation of the adhoc signing is signing as changes happen. In theory, we think it could be an optimization to replace the current adhoc signing flow with our signing method after creation of the .app bundle. Using an adhoc signature if no codesign identity is provided by the user. How would you like us to proceed?
Would you like for us to update the existing samples? or create a new sample for this flow?

Thoughts on future improvements/subsequent PR's

The license file is today moved as a post-step. Is this something that could be modified inside the freezer that places the license file initially?
Documentation for contributors could be improved regarding use of environment variables and requirements for CI builds. As well as steps for building locally.
Even with this PR we are not fully adhering to Apples expectations for .app bundle structure because frameworks are not stored in /Contents/Frameworks/ (but it's still working 😃)
Consideration should be given to removing the codesign-resourcerules parameter as it's no longer supported on newer version of macOS.
It might be worth adding validation/asserts when the MacOS folder only contains binaries

Related Issues

In our testing, this PR also resolved this issue
While testing we discovered that the Qt SimpleBrowser example does not run on OSX due to missing the QtWebEngineProcess executable

The work in this PR has been a co-dev effort of myself and @TechnicalPirate. Feedback would be greatly appreciated @marcelotduarte 🙏. Thank you for your efforts in helping us make this contribution.

codecov · 2023-08-31T15:20:46Z

Codecov Report

Patch coverage: 40.96% and project coverage change: +0.05% 🎉

Comparison is base (a7548d9) 51.60% compared to head (e3ffaff) 51.66%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2025      +/-   ##
==========================================
+ Coverage   51.60%   51.66%   +0.05%     
==========================================
  Files          67       67              
  Lines        5716     5780      +64     
==========================================
+ Hits         2950     2986      +36     
- Misses       2766     2794      +28

Files Changed	Coverage Δ
cx_Freeze/darwintools.py	`65.80% <40.00%> (+2.11%)`	⬆️
cx_Freeze/command/bdist_mac.py	`51.65% <41.02%> (-2.54%)`	⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

marcelotduarte · 2023-08-31T18:37:04Z

Very good! I'll review the code, your questions and then answer your questions.
I will also do a test later.
I'm going to do a rebase and then I need the errors pointed out by the pre-commit to be corrected. I can do it myself if that's the case.

* Create symlink from MacOS/lib > Resources/Lib to avoid needing to set *relative* rpaths which will fail verification

Adding timestamp option + verification options Added missing option Another missing option Change var name

for more information, see https://pre-commit.ci

marcelotduarte · 2023-09-01T02:37:24Z

* The current implementation of the adhoc signing is signing as changes happen. In theory, we think it could be an optimization to replace the current adhoc signing flow with our signing method after creation of the .app bundle. Using an adhoc signature if no codesign identity is provided by the user. How would you like us to proceed?

I like optimizations, but I'm a fan of the theory that it has to be working well to optimize later.

* Would you like for us to update the existing samples? or create a new sample for this flow?

If it's pyqt samples for example, I'd rather just update, putting bdist_mac options, it's even better, because I use them for testing and I can compare the results.

* The license file is today moved as a post-step. Is this something that could be modified inside the freezer that places the license file initially?

Let's leave it the way you did it for now because even if it changes in the freezer, there isn't a folder equivalent to Resources in the current structure.

* Documentation for contributors could be improved regarding use of environment variables and requirements for CI builds. As well as steps for building locally.

Sure.

* Even with this PR we are not fully adhering to Apples expectations for .app bundle structure because frameworks are not stored in `/Contents/Frameworks/` (but it's still working 😃)

Ok.

* Consideration should be given to removing the `codesign-resourcerules` parameter as it's no longer supported on newer version of macOS.

Where is this parameter?

* It might be worth adding validation/asserts when the `MacOS` folder does not contain any binaries

* While testing we discovered that the Qt SimpleBrowser example does not run on OSX due to missing the `QtWebEngineProcess` executable

#933

The work in this PR has been a co-dev effort of myself and @TechnicalPirate. Feedback would be greatly appreciated @marcelotduarte 🙏. Thank you for your efforts in helping us make this contribution.

Thanks!

cx_Freeze/command/bdist_mac.py

for more information, see https://pre-commit.ci

TechnicalPirate · 2023-09-01T10:23:46Z

Hey thanks for the feedback and initial review.

It seems most of the questions we had can be done as subsequent PR's / improvements.

The only change we're including now based on your feedback is updating the samples as well as resolving the inline comments :)

Where is this parameter?

Re: codesign-resourcerules, it's here - i can't find the official announcement but googling around has plenty of results talking about its deprecation 👍 ( i imagine this will also be logged as its own issue )

Ready for final review :)

samples/pyside2/setup.py

cx_Freeze/command/bdist_mac.py

marcelotduarte · 2023-09-01T16:28:33Z

Re: codesign-resourcerules, it's here - i can't find the official announcement but googling around has plenty of results talking about its deprecation 👍 ( i imagine this will also be logged as its own issue )

ok, in another issue/pr

Should be the code about prepare_qt_app changed to support newer qt versions or removed (since we no longer support qt4)?

johan-ronnkvist · 2023-09-04T07:06:54Z

Should be the code about prepare_qt_app changed to support newer qt versions or removed (since we no longer support qt4)?

From what I can see, this code effectively does nothing today. We didn't make any changes here in order to minimize changes.

The purpose of the qt_menu.nib is to ensure native macOS menu bar integration. But it's not something we've come across in testing.

In my opinion, the code should be removed.

… feature/simple_restructure

marcelotduarte · 2023-09-04T18:16:24Z

The purpose of the qt_menu.nib is to ensure native macOS menu bar integration. But it's not something we've come across in testing.

In my opinion, the code should be removed.

I also thought about removing it, but I understand from your answer that it is also possible to adapt to qt5/6, right?

marcelotduarte · 2023-09-04T18:18:39Z

Feel free to make new contributions.
@johan-ronnkvist @TechnicalPirate Thank you!

TechnicalPirate · 2023-09-04T20:03:33Z

Feel free to make new contributions. @johan-ronnkvist @TechnicalPirate Thank you!

You're very welcome - We will use the next dev release to verify everything is still good our side and start making some other contributions as we phase out our wrapper :)

johan-ronnkvist · 2023-09-05T07:54:04Z

The purpose of the qt_menu.nib is to ensure native macOS menu bar integration. But it's not something we've come across in testing.
In my opinion, the code should be removed.

I also thought about removing it, but I understand from your answer that it is also possible to adapt to qt5/6, right?

Yes, it's technically possible to update these paths to match newer versions of Qt. But I'd suggest to find a valid repro case before doing so. Again, with at least some effort, I wasn't able to find a trace of the mentioned file with newer versions of Qt. I've spent a couple of years working with an application that was cross-platform and never come across any issues with menu bar management in macOS. I can't guarantee it's not relevant, but have not seen usage or the need for it previously.

marcelotduarte · 2023-09-07T01:45:59Z

Yes, it's technically possible to update these paths to match newer versions of Qt. But I'd suggest to find a valid repro case before doing so.

As there are no issues and given your experience, I agree that it should be removed, even to simplify the code.

marcelotduarte · 2023-09-07T01:46:40Z

We will use the next dev release to verify everything is still good our side

You can test the patch in the latest development build:
pip install --upgrade --pre --extra-index-url https://marcelotduarte.github.io/packages/ cx_Freeze
For conda-forge the command is:
conda install -y --no-channel-priority -S -c https://marcelotduarte.github.io/packages/conda cx_Freeze

marcelotduarte · 2023-09-07T01:49:20Z

* Documentation for contributors could be improved regarding use of environment variables and requirements for CI builds. As well as steps for building locally.

Sure.

PR #2033
https://cx-freeze--2033.org.readthedocs.build/en/2033/development/index.html#building-binary-wheels

Co-authored-by: Joe Deuchar <deucharj@gmail.com> Co-authored-by: Johan Rönnkvist <johan.ronnkvist@king.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Marcelo Duarte <marcelotduarte@users.noreply.github.com>

mathiascode · 2023-09-08T23:25:23Z

It seems like Contents/MacOS/lib and Contents/Resources/lib are duplicated in the final .dmg file instead of symlinked. Is this intended?

marcelotduarte · 2023-09-09T03:01:15Z

@mathiascode
You can test the patch in the latest development build:
pip install --upgrade --pre --extra-index-url https://marcelotduarte.github.io/packages/ cx_Freeze
For conda-forge the command is:
conda install -y --no-channel-priority -S -c https://marcelotduarte.github.io/packages/conda cx_Freeze

mathiascode · 2023-09-09T23:07:50Z

@marcelotduarte I don't have a macOS installation right now to run the .dmg, but Contents/MacOS/lib is a symlink now. Thanks!

marcelotduarte · 2023-11-07T19:10:27Z

While testing we discovered that the Qt SimpleBrowser example does not run on OSX due to missing the QtWebEngineProcess executable

#933

@johan-ronnkvist Please see #933 (comment)

TechnicalPirate and others added 20 commits August 31, 2023 15:37

Move output location of license file to resources

e2cdf0a

Force signing

240d298

Delete Existing .app dir before build

abb4d66

Change output to /Resources

549ed95

* Create symlink from MacOS/lib > Resources/Lib to avoid needing to set *relative* rpaths which will fail verification

Changed to kat-assemble codesign for verification reasons

69cbf9c

Adding timestamp option + verification options Added missing option Another missing option Change var name

Trying an idea

8dcf437

Removed repeated arg (-vvv is --verbose)

cf9e533

WIP reimplementation of code sign

4f7c694

Changed to capture un-extensioned files

5ccee47

doh! recursion is important

deeb3a0

Hookup options

ad5b51e

Test recursive codesign

e23bc76

Added string cast

36cf1f0

Whitespace fix

f204621

Split out verify_signature

20e44c0

cleanup

0dfec60

Added information about new options for codesign.

d16ffd7

[pre-commit.ci] auto fixes from pre-commit.com hooks

b4de221

for more information, see https://pre-commit.ci

fix html build

9a225f2

make ruff and pylint happy

3304c8f

marcelotduarte reviewed Sep 1, 2023

View reviewed changes

cx_Freeze/command/bdist_mac.py Outdated Show resolved Hide resolved

Johan Rönnkvist and others added 4 commits September 1, 2023 12:06

Rename isMachOFile for public access

a764fd0

use isMachOFile to determine binaries to sign

2d158ad

Updated sample with codesign options

7cf6c66

[pre-commit.ci] auto fixes from pre-commit.com hooks

c5c842c

for more information, see https://pre-commit.ci

johan-ronnkvist marked this pull request as ready for review September 1, 2023 10:30

marcelotduarte requested changes Sep 1, 2023

View reviewed changes

samples/pyside2/setup.py Show resolved Hide resolved

cx_Freeze/command/bdist_mac.py Show resolved Hide resolved

Johan Rönnkvist added 2 commits September 4, 2023 09:11

Review feedback

ad7442d

Merge remote-tracking branch 'origin/feature/simple_restructure' into…

e3ffaff

… feature/simple_restructure

marcelotduarte approved these changes Sep 4, 2023

View reviewed changes

marcelotduarte merged commit eb23c38 into marcelotduarte:main Sep 4, 2023

marcelotduarte mentioned this pull request Sep 5, 2023

Code-sign breaks on macOS with python 3.11 #1861

Closed

marcelotduarte mentioned this pull request Sep 9, 2023

bdist_mac: regression - fix duplicate lib in bdist_dmg #2037

Merged

marcelotduarte mentioned this pull request Nov 7, 2023

PyQt5, PyQtWebEngine, bdist_mac: Cannot find QtWebEngineProcess #933

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Builds pass macOS notarization #2025

Builds pass macOS notarization #2025

johan-ronnkvist commented Aug 31, 2023 •

edited

Loading

codecov bot commented Aug 31, 2023 •

edited

Loading

marcelotduarte commented Aug 31, 2023

marcelotduarte commented Sep 1, 2023

TechnicalPirate commented Sep 1, 2023

marcelotduarte commented Sep 1, 2023

johan-ronnkvist commented Sep 4, 2023 •

edited

Loading

marcelotduarte commented Sep 4, 2023

marcelotduarte commented Sep 4, 2023

TechnicalPirate commented Sep 4, 2023

johan-ronnkvist commented Sep 5, 2023

marcelotduarte commented Sep 7, 2023

marcelotduarte commented Sep 7, 2023

marcelotduarte commented Sep 7, 2023

mathiascode commented Sep 8, 2023

marcelotduarte commented Sep 9, 2023

mathiascode commented Sep 9, 2023 •

edited

Loading

marcelotduarte commented Nov 7, 2023

Builds pass macOS notarization #2025

Builds pass macOS notarization #2025

Conversation

johan-ronnkvist commented Aug 31, 2023 • edited Loading

Changes:

Questions

Thoughts on future improvements/subsequent PR's

Related Issues

codecov bot commented Aug 31, 2023 • edited Loading

Codecov Report

marcelotduarte commented Aug 31, 2023

marcelotduarte commented Sep 1, 2023

TechnicalPirate commented Sep 1, 2023

marcelotduarte commented Sep 1, 2023

johan-ronnkvist commented Sep 4, 2023 • edited Loading

marcelotduarte commented Sep 4, 2023

marcelotduarte commented Sep 4, 2023

TechnicalPirate commented Sep 4, 2023

johan-ronnkvist commented Sep 5, 2023

marcelotduarte commented Sep 7, 2023

marcelotduarte commented Sep 7, 2023

marcelotduarte commented Sep 7, 2023

mathiascode commented Sep 8, 2023

marcelotduarte commented Sep 9, 2023

mathiascode commented Sep 9, 2023 • edited Loading

marcelotduarte commented Nov 7, 2023

johan-ronnkvist commented Aug 31, 2023 •

edited

Loading

codecov bot commented Aug 31, 2023 •

edited

Loading

johan-ronnkvist commented Sep 4, 2023 •

edited

Loading

mathiascode commented Sep 9, 2023 •

edited

Loading