Skip to content

APPLE: Make codesigning improvements#3710

Closed
dgovil wants to merge 6 commits intoPixarAnimationStudios:devfrom
dgovil:codesigning-improvements
Closed

APPLE: Make codesigning improvements#3710
dgovil wants to merge 6 commits intoPixarAnimationStudios:devfrom
dgovil:codesigning-improvements

Conversation

@dgovil
Copy link
Copy Markdown
Collaborator

@dgovil dgovil commented Jul 7, 2025

Description of Change(s)

This PR overhauls the current code signing setup with the following changes:

  1. Developers can now pass in code signing identifiers directly to the build_usd.py script.
  2. Codesigning identifiers are now passed to CMake for Xcode targets if they want to sign anything.
  3. The code signing identifier is calculated less often
  4. Codesigning now supports signing frameworks
  5. Codesigning supports checking previous signing status and only re-signing as needed. Note: Since USD's cmake always installs all targets again, they're always re-signed. Future work might help optimize this, but at the least we're not re-signing every dependency.
  6. Codesigning is validated earlier to prevent running through a whole build just to find code signing fails

This makes code signing much more reliable and faster overall.

This work is needed to enable properly building OpenUSD as a framework

Checklist

@dgovil dgovil requested a review from davidgyu July 7, 2025 18:47
@dgovil dgovil added needs review Issue needing input/review by the repo maintainer (Pixar) build Build-related issue/PR labels Jul 7, 2025
@jesschimein
Copy link
Copy Markdown
Collaborator

Filed as internal issue #USD-11198

(This is an automated message. See here for more information.)

Copy link
Copy Markdown
Contributor

@sunyab sunyab left a comment

Choose a reason for hiding this comment

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

Hi @dgovil , left some notes from a first pass review. Thanks!

Comment thread build_scripts/apple_utils.py Outdated
Comment thread build_scripts/apple_utils.py
Comment thread build_scripts/apple_utils.py Outdated
Comment thread build_scripts/build_usd.py Outdated
Comment thread build_scripts/apple_utils.py Outdated
Comment thread build_scripts/apple_utils.py Outdated
Comment thread build_scripts/apple_utils.py Outdated
@dgovil
Copy link
Copy Markdown
Collaborator Author

dgovil commented Nov 14, 2025

Thanks, @sunyab !
I'll take a pass next week at resolving these

@dgovil dgovil force-pushed the codesigning-improvements branch from d1be066 to 0983b32 Compare November 26, 2025 20:05
@dgovil
Copy link
Copy Markdown
Collaborator Author

dgovil commented Nov 26, 2025

Thanks, @sunyab
I pushed an update that also rebases on top of dev.

Appreciate the notes. Hope you have a good thanksgiving break.

@sunyab
Copy link
Copy Markdown
Contributor

sunyab commented Dec 12, 2025

Hey @dgovil, I ran into an error when testing this change earlier. I was testing the process we follow internally when doing QA for USD releases:

  • Create and activate a Python venv
  • Install the necessary Python modules into the venv (PySide6 and PyOpenGL)
  • Build USD into the venv via build_usd.py

This was the result:

sunya@usd-mac-08 ~ --> /usr/local/bin/python3 -m venv codesign_test    
sunya@usd-mac-08 ~ --> source codesign_test/bin/activate
(codesign_test) sunya@usd-mac-08 ~ --> python -m pip install PyOpenGL PySide6==6.3.1
(codesign_test) sunya@usd-mac-08 ~ --> python OpenUSD/build_scripts/build_usd.py --build-target universal --tests codesign_test 

<snip>

STATUS: Installing USD...
/Users/sunya/codesign_test/lib/python3.9/site-packages/PySide6/Qt/lib/QtWebEngineCore.framework: code object is not signed at all
In subcomponent: /Users/sunya/codesign_test/lib/python3.9/site-packages/PySide6/Qt/lib/QtWebEngineCore.framework/Helpers/QtWebEngineProcess.app
Traceback (most recent call last):
  File "/Users/sunya/dev-cmake/build_scripts/build_usd.py", line 2982, in <module>
    apple_utils.Codesign(context.usdInstDir,
  File "/Users/sunya/dev-cmake/build_scripts/apple_utils.py", line 312, in Codesign
    result = CodesignPath(path, identifier, team_identifier=team_identifier, force=force, is_framework=True)
  File "/Users/sunya/dev-cmake/build_scripts/apple_utils.py", line 270, in CodesignPath
    subprocess.check_call(
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 373, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['codesign', '--force', '--sign', '-', '--generate-entitlement-der', '--verbose', '/Users/sunya/codesign_test/lib/python3.9/site-packages/PySide6/Qt/lib/QtWebEngineCore.framework']' returned non-zero exit status 1.

Since we're now code-signing frameworks, it looks like we're now picking up some Qt frameworks that are hidden deep in the PySide6 Python module.

I know we were previously recursing over the entire install directory and signing dylibs, but since we're now looking at frameworks too I wonder if we ought to limit the search that the Codesign method does. What do you think?

@dgovil
Copy link
Copy Markdown
Collaborator Author

dgovil commented Dec 12, 2025

Oh good catch. Yeah we should probably only sign the OpenUSD.framework.

I can add a check , but if you need to unblock your teams, just let me know if you add it first.

@dgovil
Copy link
Copy Markdown
Collaborator Author

dgovil commented Dec 12, 2025

Okay, I added what I think should do the trick.

Comment thread build_scripts/apple_utils.py Outdated
Comment on lines +293 to +313
for framework in frameworks:
framework_name = os.path.splitext(framework)[0]
if framework_name.lower() not in ["openusd", "opensubdiv", "materialx"]:
continue
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.

Instead of hardcoding a list of names, what do you think about just limiting the search to all .dylib, .so, and .framework files/directories in specific paths directory instead of walking the entire directory structure?

So something like:

codesignPaths = [
    os.path.join(install_dir, 'lib'),
    os.path.join(install_dir, 'plugin'),
    os.path.join(install_dir, 'share/usd')
]

for dir in codesignPaths:
    shared_libs = glob.glob(f"{os.path.join(dir, '*.dylib')")
    shared_libs += glob.glob(f"{os.path.join(dir, '*.so')")
    for lib in shared_libs:
        # Codesign..

    frameworks = glob.glob(f"{os.path.join(dir, '*.framework')")
    for framework in frameworks:
        # Codesign...

I like that this is more targeted -- the current implementation winds up signing files in unnecessary places like the build directory.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh yeah that makes sense. I guess I hadn't considered that since I always install into a clean directory, but I'm assuming others may install into a communal directory?

Copy link
Copy Markdown
Collaborator Author

@dgovil dgovil Dec 12, 2025

Choose a reason for hiding this comment

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

Oh I see. It's because the usdInstPath is not the final install directory but the full container directory.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@sunyab Okay, I pushed an update that integrates your solution to filter. I still use my walk within the filtered set because there are some dylibs that are within nested folders that need to be signed.
But it should hopefully resolve what you're seeing

@dgovil dgovil force-pushed the codesigning-improvements branch from 195ae0c to 4e57c2d Compare December 12, 2025 19:07
@dgovil dgovil force-pushed the codesigning-improvements branch from 4e57c2d to e84f930 Compare December 12, 2025 19:10
@pixar-oss pixar-oss closed this in 48cb715 Dec 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Build-related issue/PR needs review Issue needing input/review by the repo maintainer (Pixar)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants