Skip to content
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

macOS: robustify macOS assembly pipeline #6188

Merged
merged 3 commits into from
Sep 4, 2021

Conversation

rokm
Copy link
Member

@rokm rokm commented Sep 4, 2021

Two changes that make our assembly pipeline on macOS a bit more robust to bogus behavior of the codesign utility on 10.13 High Sierra (which is EOL at this point, but apparently still popular enough), and fix #6167.

I think we can live without signature removal when modifying the headers of collected binaries regardless of macOS version, as forced re-sign should take care of invalidated signatures. If this was not the case, we'd probably be seeing issue reports on earlier versions of PyInstaller where users had to do re-signing themselves. So by skipping signature removal we avoid one call to codesign and one file resizing (which can be problematic on 10.13).

The revised segment and section size computation is more direct, and allows us to compensate for oddities with 10.13 codesign when removing signature from bootloader executable. It is also more consistent with slice size adjustment that we do for fat binaries at the end of that function.

When adjusting the size of the SYMTAB section and its parent
__LINKEDIT segment, directly compute the new size as the difference
between end-of-file and the corresponding offset. Previously, we
reconstructed the old file size from the __LINKEDIT segment's start
offset and size, and then computed the size delta based on the
new file size.

The new method is more direct and follows the basic assumption
that in the final file with appended CArchive, the SYMTAB section
and the __LINKEDIT segment must cover everything up to the end of
the file.

It also gives us some resilience against the codesign utility on
Mac OS 10.13 producing bogus __LINKEDIT sizes when removing  the
signature from the executable.
Disable signature removal before modifying headers of the collected
binaries. Doing so makes the process faster, and avoids potential
bugs in the codesign utility (e.g., on Mac OS 10.13).
@rokm rokm added the needs backport This pull request needs cherry picking onto the v4 branch. label Sep 4, 2021
Copy link
Member

@bwoodsend bwoodsend left a comment

Choose a reason for hiding this comment

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

I can't offer much of a review for this one but what you're saying sounds sensible to me.

@rokm rokm merged commit a901199 into pyinstaller:develop Sep 4, 2021
rokm added a commit that referenced this pull request Sep 4, 2021
* osxutils: robustify the fix_exe_for_code_signing

When adjusting the size of the SYMTAB section and its parent
__LINKEDIT segment, directly compute the new size as the difference
between end-of-file and the corresponding offset. Previously, we
reconstructed the old file size from the __LINKEDIT segment's start
offset and size, and then computed the size delta based on the
new file size.

The new method is more direct and follows the basic assumption
that in the final file with appended CArchive, the SYMTAB section
and the __LINKEDIT segment must cover everything up to the end of
the file.

It also gives us some resilience against the codesign utility on
Mac OS 10.13 producing bogus __LINKEDIT sizes when removing  the
signature from the executable.

* building: macOS: disable signature removal when modifying binaries

Disable signature removal before modifying headers of the collected
binaries. Doing so makes the process faster, and avoids potential
bugs in the codesign utility (e.g., on Mac OS 10.13).
@rokm rokm removed the needs backport This pull request needs cherry picking onto the v4 branch. label Sep 4, 2021
@rokm rokm deleted the robustify-macos-assembly-pipeline branch September 4, 2021 18:01
@rokm rokm added the backported Backported to v4 branch label Sep 26, 2021
@rokm rokm mentioned this pull request Oct 29, 2021
6 tasks
@bwoodsend bwoodsend mentioned this pull request Feb 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backported Backported to v4 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PyInstaller 4.5.1 "codesign failure!" on macos
2 participants