Codesigning packages for macOS #10645
Conversation
…w due to installing mactex from brew (15min)
…'s only for ppc and i386 (so outdated, I recommend just trashing it) On M1 mac ``` ./bin/EP-Compare/Run-Mac/EP-Compare.app/Contents/MacOS/EP-Compare -bash: /Users/julien/Software/Others/EnergyPlus/bin/EP-Compare/Run-Mac/EP-Compare.app/Contents/MacOS/EP-Compare: Bad CPU type in executable ` ``
… intermittently fails due to timeout eg https://github.com/jmarrec/EnergyPlus/actions/runs/10114280998/job/27973519917#step:11:67
The Fortran (and ConvertInputFormat) are actual sub-`project()` so codesigning does not work from Install.cmake
(locally I build with sphinxcontrib-moderncmakedomain)
jmarrec
added
Defect
| @@ -98,7 +98,7 @@ def generate_epJSON_schema(source_dir_path: str): | |||
| p = multiprocessing.Process(target=generate_epJSON_schema, args=(sys.argv[1], )) | |||
| p.start() | |||
|
|
|||
| timeout_seconds = 30 | |||
| timeout_seconds = 60 | |||
There was a problem hiding this comment.
I kept getting a timeout on macos-12, so I increased the timeout a bit.
I was getting it only intermittently, and never on macos-14 (the arm64 runner), so I suspect the runtime was very near 30 seconds and sometimes it'd be ok and sometimes not.
| if(APPLE) | ||
|
|
||
| include(${CMAKE_CURRENT_LIST_DIR}/CodeSigning.cmake) | ||
|
|
||
| # Defines CPACK_CODESIGNING_DEVELOPPER_ID_APPLICATION and CPACK_CODESIGNING_NOTARY_PROFILE_NAME | ||
| setup_macos_codesigning_variables() | ||
|
|
||
| if(CPACK_CODESIGNING_DEVELOPPER_ID_APPLICATION) |
There was a problem hiding this comment.
If APPLE and the CPACK_CODESIGNING_DEVELOPPER_ID_APPLICATION is defined, we're in business!
| set(FILES_TO_SIGN | ||
| # Targets are signed already via register_install_codesign_target | ||
| #$<TARGET_FILE_NAME:ConvertInputFormat> | ||
| #$<TARGET_FILE_NAME:energyplus> | ||
| #$<TARGET_FILE_NAME:energyplusapi> | ||
|
|
||
| # Bash scripts, not sure if needed or not | ||
| "runenergyplus" | ||
| "runepmacro" | ||
| "runreadvars" | ||
| # Copied-verbatim apps: Already signed because just copied from bin to package | ||
| # "EPMacro" | ||
| # "PreProcess/EP-Launch-Lite.app" | ||
| # "PreProcess/IDFVersionUpdater/IDFVersionUpdater.app" | ||
| # "PostProcess/EP-Compare/EP-Compare.app" | ||
| ) | ||
|
|
||
| # Codesign inner binaries and libraries, in the CPack staging area for the EnergyPlus project, component Unspecified | ||
| # Define some required variables for the script in the scope of the install(SCRIPT) first | ||
| install(CODE "set(CPACK_CODESIGNING_DEVELOPPER_ID_APPLICATION \"${CPACK_CODESIGNING_DEVELOPPER_ID_APPLICATION}\")" COMPONENT Unspecified) | ||
| install(CODE "set(FILES_TO_SIGN \"${FILES_TO_SIGN}\")" COMPONENT Unspecified) | ||
| # call the script | ||
| install(SCRIPT "${CMAKE_CURRENT_LIST_DIR}/install_codesign_script.cmake" COMPONENT Unspecified) |
There was a problem hiding this comment.
This bit will sign the:
- FILES_TO_SIGN
- but that part is probably not needed anymore since only scripts are left... I ended up changing totally the way I sign binaries after realizing that the FORTRAN stuff (and ConvertInputFormat) are in different
cmakeproject()and therefore keep overriding this.
- but that part is probably not needed anymore since only scripts are left... I ended up changing totally the way I sign binaries after realizing that the FORTRAN stuff (and ConvertInputFormat) are in different
- It globs the python .so and the root dylibs too.
| # Register the CPACK_POST_BUILD_SCRIPTS | ||
| set(CPACK_POST_BUILD_SCRIPTS "${CMAKE_CURRENT_LIST_DIR}/CPackSignAndNotarizeDmg.cmake") |
There was a problem hiding this comment.
This post cpack script will sign the resulting .dmg
and notarize + staple (+ verify via spctl --assess) it if CPACK_CODESIGNING_NOTARY_PROFILE_NAME is defined.
| include(${CMAKE_CURRENT_LIST_DIR}/CodeSigning.cmake) | ||
|
|
||
| # Defines CPACK_CODESIGNING_DEVELOPPER_ID_APPLICATION and CPACK_CODESIGNING_NOTARY_PROFILE_NAME | ||
| setup_macos_codesigning_variables() |
There was a problem hiding this comment.
This will setset CPACK_IFW_PACKAGE_SIGNING_IDENTITY to the same value as CPACK_CODESIGNING_DEVELOPPER_ID_APPLICATION, so QtIFW's binarycreator signs the .app installer created.
Note: I had to patch the source code for binarycreator so the signing works, more on this in a bit when I comment the release workflow
| - name: Upload DMG as artifact for testing | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: energyplus-${{ matrix.os }}.dmg | ||
| path: build/EnergyPlus-*-${{ matrix.arch }}.dmg | ||
| if-no-files-found: error | ||
| retention-days: 7 | ||
| overwrite: false |
There was a problem hiding this comment.
Uploading the dmg as an artifact too, for testing you'll see.
| - name: Quick Test Package Signing and otool exes and libs | ||
| shell: bash | ||
| working-directory: ./build | ||
| run: | | ||
| begin_group() { echo -e "::group::\033[93m$1\033[0m"; } | ||
|
|
||
| subheader() { echo -e "\033[95m---- $1\033[0m"; } | ||
|
|
||
| exes=( | ||
| "energyplus" "libenergyplusapi.dylib" | ||
| "ExpandObjects" "ConvertInputFormat" | ||
| "PreProcess/IDFVersionUpdater/Transition-V23-1-0-to-V23-2-0" | ||
| "PostProcess/ReadVarsESO" "PostProcess/HVAC-Diagram" | ||
| ) | ||
|
|
||
| TGZ_DIR=$(find _CPack_Packages/Darwin/TGZ -name "EnergyPlus*" -type d -maxdepth 1) | ||
| echo "TGZ_DIR=$TGZ_DIR" >> $GITHUB_ENV | ||
| echo "Checking TGZ dir at $TGZ_DIR" | ||
|
|
||
| for rel_exe in "${exes[@]}"; do | ||
| exe="$TGZ_DIR/$rel_exe" | ||
| begin_group "Checking $exe" | ||
| subheader "otool" | ||
| otool -L "${exe}" || true | ||
| subheader "codesign" | ||
| siginfo=$(codesign --display -vv "${exe}" 2>&1) | ||
| if [[ $siginfo == *"K7JYVQJL7R"* ]]; then | ||
| echo -e "\033[92mSIGNATURE OK\033[0m" | ||
| echo "$siginfo" | grep Authority | ||
| else | ||
| echo -e "\033[91mSignature not ok for ${exe}\033[0m" | ||
| echo "::error::title=Signature not ok for ${exe}::$siginfo" | ||
| fi | ||
| echo "::endgroup::" | ||
| done |
There was a problem hiding this comment.
Maybe this could be removed altogether, but I kinda like having a short and sweet version, the full test below is verbose. And I've improved the grouping and colors and co so that's very readable here
cf https://github.com/jmarrec/EnergyPlus/actions/runs/10266015716/job/28403444892
| - name: Full Test Package signing and otool for IFW and TGZ | ||
| working-directory: ./build | ||
| shell: bash | ||
| run: | | ||
| begin_group() { echo -e "::group::\033[93m$1\033[0m"; } | ||
|
|
||
| begin_group "Full Check signature of _CPack_Packages for both IFW and TGZ and resolve otool libraries" | ||
| python ../scripts/dev/verify_signature.py --verbose --otool --otool-out-file otool_infos_cpack.json . | ||
| echo "::endgroup::" | ||
|
|
||
| begin_group "Running a simulation with python" | ||
| ./$TGZ_DIR/energyplus --help | ||
| ./$TGZ_DIR/energyplus -w ./$TGZ_DIR/WeatherData/USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw -d out ./$TGZ_DIR/ExampleFiles/PythonPluginCustomSchedule.idf | ||
| echo "::endgroup::" | ||
|
|
||
| - name: Upload otool info as artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: otool_infos_cpack_${{ matrix.os }}_${{ matrix.arch }} | ||
| path: build/otool*json | ||
| if-no-files-found: error |
There was a problem hiding this comment.
I check the _CPack staging areas to ensure it's codesigned there. No point continuing if not.
I also make sure I can run a python simulation, I was getting issues early on when the python lib-dynload sos were not signed as well
and I upload the otool json for inspection if needed
|
|
||
| - name: Run Package Tests | ||
| run: python checkout/scripts/package_tests/runner.py --verbose ${{ matrix.test_key }} package/ | ||
| - uses: actions/checkout@v4 # Still need E+ checked out to get testing scripts |
There was a problem hiding this comment.
This is just whitespace change, use https://github.com/NREL/EnergyPlus/pull/10645/files?w=1 to not see them.
| - name: Gather Dmg Package from Artifacts | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: energyplus-${{ matrix.os }}.dmg | ||
| path: dmg | ||
|
|
||
| - name: Test Dmg Install and Package signing | ||
| working-directory: ./dmg | ||
| shell: bash | ||
| run: | | ||
| begin_group() { echo -e "::group::\033[93m$1\033[0m"; } | ||
|
|
||
| set -x | ||
|
|
||
| dmg=$(ls EnergyPlus-*.dmg) | ||
| begin_group "Checking Signature of .dmg" | ||
| spctl --assess --type open --context context:primary-signature -vvvv $dmg | ||
| echo "::endgroup::" | ||
|
|
||
| begin_group "Mounting Dmg, and checking signature of installer app" | ||
| mkdir temp_mount | ||
| hdiutil attach -mountpoint ./temp_mount/ $dmg | ||
| filename="${dmg%.*}" | ||
| spctl --assess --type open --context context:primary-signature -vvvv ./temp_mount/$filename.app | ||
| echo "::endgroup::" | ||
|
|
||
| begin_group "Installing" | ||
| sudo ./temp_mount/$filename.app/Contents/MacOS/$filename --accept-licenses --default-answer --confirm-command --root $(pwd)/test_install install | ||
| hdiutil detach ./temp_mount/ | ||
| echo "::endgroup::" | ||
|
|
||
| begin_group "Quick Check signature of inner executables and binaries" | ||
| codesign -dvvv ./test_install/energyplus | ||
| codesign -dvvv ./test_install/libenergyplusapi.dylib | ||
| codesign -dvvv ./test_install/libpython*.dylib | ||
| codesign -dvvv ./test_install/ConvertInputFormat | ||
| codesign -dvvv ./test_install/PostProcess/ReadVarsESO | ||
| echo "::endgroup::" | ||
|
|
||
| begin_group "Full Check signature of installed DMG for all executables and resolve otool libraries" | ||
| python ../checkout/scripts/dev/verify_signature.py --otool --otool-out-file otool_info_dmg.json --verbose --install test_install | ||
| echo "::endgroup::" | ||
|
|
||
| begin_group "Running a simulation with python" | ||
| ./test_install/energyplus --help | ||
| ./test_install/energyplus -w ./test_install/WeatherData/USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw -d out ./test_install/ExampleFiles/PythonPluginCustomSchedule.idf | ||
| echo "::endgroup::" | ||
|
|
||
| - name: Upload otool info as artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: otool_info_dmg_${{ matrix.os }}_${{ matrix.arch }} | ||
| path: dmg/otool*json | ||
| if-no-files-found: error |
There was a problem hiding this comment.
This is new.
I download the .dmg artifact on the test machine and I:
- Ensure it's properly signed and notarized
- Mount it
- Ensure the installer .app is properly signed and notarized
- Install the ifw installer
- Then I test the resulting folder for codesigning and otool for binaries and executables
|
Testing it at https://github.com/NREL/EnergyPlus/actions/runs/10268471197 / https://github.com/NREL/EnergyPlus/releases/tag/v24.2.0-codesigning_macos on this repo. FYI: Last successful build on my fork was https://github.com/jmarrec/EnergyPlus/actions/runs/10266015716 / https://github.com/jmarrec/EnergyPlus/releases/tag/v24.2.0-codesigning_macos2 |
|
Thanks @jmarrec for doing this, I just took a quick look at the changes but only understood a little bit of it so I don't think it can be considered a real review. I do think the users will really like this change. If I update a MacOS exe file in the repo, do I need to do anything special? |
|
@JasonGlazer Yes, the exe will need to resigned. |
|
|
||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| BUILD_TYPE: Release | ||
| FC: gfortran-13 | ||
| SDKROOT: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk | ||
| Python_REQUIRED_VERSION: 3.12.2 | ||
| BUILD_DOCS: true # Installing MacTex takes like 15min, so you can speed things up by disabling it |
There was a problem hiding this comment.
Awesome. we should make sure that our package testing scripts confirm docs are built, otherwise this could easily slip through on someone's test commits on a PR.
|
Alright. This is going in. We will see how things go with IO freeze and tweak things as needed, but this is going to be vital as we move toward upcoming Mac OS releases. @jmarrec this was a substantial lift, and I am especially grateful for not only the work itself but also how you have documented the process. I look forward to a day soon when we can eliminate all the prebuilt binaries, and all the Fortran apps, and make this whole process just much easier. |
Pull request overview
Fix [Codesign] Codesign and notarize .dmg installer on macOS #10641
Fix [Codesign] Codesign with hardened runtime the inner executables and libraries for macOS #10642
Fix [Codesign] Configure github actions to sign macOS packages and implement QA/QC #10643
Addresses partially Precompiled binaries are not digitally signed #6364 (comment)
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.