-
Notifications
You must be signed in to change notification settings - Fork 398
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
Codesigning packages for macOS #10645
Changes from all commits
f1cddd7
034a251
2739ab0
456c4b4
6295052
ab4b654
50db3ce
8019ac5
6e9ed68
a4a461f
9976b6c
2c863ea
16f9d9a
097d5d4
dd66f7b
2f7908a
af34647
e6222b4
213f143
62e2908
45aa076
ea0bc67
f6152d2
1843aab
95be097
05e301d
e28088f
6b59ff0
83f1bef
23150c2
241d167
9fd3048
0edad6f
004ec10
6a2dc11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,13 +4,15 @@ on: | |
push: | ||
tags: | ||
- '*' | ||
workflow_dispatch: | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When prototyping, this was extremely helpful, so I'm keeping it for next time I need to launch 50 release builds to test something, the speedup is almost 2x. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
jobs: | ||
build_installer_artifact: | ||
|
@@ -39,6 +41,61 @@ jobs: | |
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
# - name: Setup QtIFW 4.x | ||
# uses: jmarrec/setup-qtifw@v1 | ||
# with: | ||
# qtifw-version: '4.6.1' | ||
Comment on lines
+44
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment this out. |
||
|
||
- name: "Configure for codesigning" | ||
run: | | ||
set -x | ||
cd $RUNNER_TEMP | ||
mkdir codesigning && cd codesigning | ||
# ----- Create certificate files from secrets base64 ----- | ||
echo "${{ secrets.MACOS_DEVELOPER_ID_APPLICATION_CERTIFICATE_P12_BASE64 }}" | base64 --decode > certificate_application.p12 | ||
echo "${{ secrets.MACOS_DEVELOPER_ID_INSTALLER_CERTIFICATE_P12_BASE64 }}" | base64 --decode > certificate_installer.p12 | ||
|
||
# ----- Configure Keychain ----- | ||
KEYCHAIN_PATH=$RUNNER_TEMP/app-signing.keychain-db | ||
security create-keychain -p "${{ secrets.MACOS_KEYCHAIN_PASSWORD }}" $KEYCHAIN_PATH | ||
# Unlock it for 6 hours | ||
security set-keychain-settings -lut 21600 $KEYCHAIN_PATH | ||
security unlock-keychain -p "${{ secrets.MACOS_KEYCHAIN_PASSWORD }}" $KEYCHAIN_PATH | ||
|
||
# ----- Import certificates on Keychain ----- | ||
security import certificate_application.p12 -P '${{ secrets.MACOS_DEVELOPER_ID_APPLICATION_CERTIFICATE_P12_PASSWORD }}' -A -t cert -f pkcs12 -k $KEYCHAIN_PATH | ||
security import certificate_installer.p12 -P '${{ secrets.MACOS_DEVELOPER_ID_INSTALLER_CERTIFICATE_P12_PASSWORD }}' -A -t cert -f pkcs12 -k $KEYCHAIN_PATH | ||
security list-keychain -d user -s $KEYCHAIN_PATH | ||
security find-identity -vvvv $KEYCHAIN_PATH | ||
|
||
# Add needed intermediary certificates | ||
aria2c https://www.apple.com/certificateauthority/AppleWWDRCAG2.cer | ||
aria2c https://www.apple.com/certificateauthority/DeveloperIDG2CA.cer | ||
security import AppleWWDRCAG2.cer -k $KEYCHAIN_PATH | ||
security import DeveloperIDG2CA.cer -k $KEYCHAIN_PATH | ||
security find-identity -vvvv $KEYCHAIN_PATH | ||
security find-identity -v -p codesigning | ||
|
||
# Store AppConnect credentials | ||
echo "${{ secrets.NOTARIZATION_API_KEY }}" > AppConnect_Developer_API_Key.p8 | ||
xcrun notarytool store-credentials EnergyPlus \ | ||
--key AppConnect_Developer_API_Key.p8 \ | ||
--key-id ${{ secrets.NOTARIZATION_API_TEAM_ID }} \ | ||
--issuer ${{ secrets.NOTARIZATION_API_ISSUER_ID }} \ | ||
--keychain $KEYCHAIN_PATH | ||
|
||
cd .. && rm -Rf codesigning | ||
Comment on lines
+49
to
+87
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setup the codesigning certifications and notarytool profiles from github secrets into a keychain, so we can sign. |
||
|
||
# Download my patched QtIFW | ||
mkdir QtIFW && cd QtIFW | ||
aria2c https://github.com/jmarrec/QtIFW-fixup/releases/download/v5.0.0-dev-with-fixup/QtIFW-5.0.0-${{ matrix.arch }}.zip | ||
xattr -r -d com.apple.quarantine ./QtIFW-5.0.0-${{ matrix.arch }}.zip | ||
unzip QtIFW-5.0.0-${{ matrix.arch }}.zip | ||
rm -Rf ./*.zip | ||
chmod +x * | ||
./installerbase --version | ||
echo "$(pwd)" >> $GITHUB_PATH | ||
Comment on lines
+89
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have patched the QtIFW source, setup a GHA repo to build Qt and QtIFW and host the binaries for macos. The TL;DR is that binarycreator signs the .app but without the I'm trying to upstream the changes at https://codereview.qt-project.org/c/installer-framework/installer-framework/+/568786 but there's not much traction |
||
|
||
- name: Remove python ${{ env.Python_REQUIRED_VERSION }} from the toolcache | ||
run: | | ||
ls $RUNNER_TOOL_CACHE/Python || true | ||
|
@@ -52,11 +109,6 @@ jobs: | |
python-version: ${{ env.Python_REQUIRED_VERSION }} | ||
# check-latest: true # Force pick up the python I built instead of the (potential) toolcache one. I could also do `rm -Rf $RUNNER_TOOL_CACHE/Python/3.12.2` before this action | ||
|
||
- name: Setup QtIFW 4.x | ||
uses: jmarrec/setup-qtifw@v1 | ||
with: | ||
qtifw-version: '4.6.1' | ||
|
||
- name: Install Python dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
|
@@ -66,16 +118,22 @@ jobs: | |
shell: bash | ||
run: | | ||
set -x | ||
echo "Using brew to install mactex and adding it to PATH" | ||
brew update | ||
brew install --cask mactex-no-gui | ||
echo "/Library/TeX/texbin" >> $GITHUB_PATH | ||
if [[ "$BUILD_DOCS" != "false" ]]; then | ||
echo "Using brew to install mactex and adding it to PATH" | ||
brew install --cask mactex-no-gui | ||
echo "/Library/TeX/texbin" >> $GITHUB_PATH | ||
echo "DOCUMENTATION_BUILD=BuildWithAll" >> $GITHUB_ENV | ||
else | ||
echo "DOCUMENTATION_BUILD=DoNotBuild" >> $GITHUB_ENV | ||
fi | ||
# The MACOSX_DEPLOYMENT_TARGET environment variable sets the default value for the CMAKE_OSX_DEPLOYMENT_TARGET variable. | ||
# We use cmake commands to build some subprojects, so setting it globally | ||
echo MACOSX_DEPLOYMENT_TARGET=${{ matrix.macos_dev_target }} >> $GITHUB_ENV | ||
echo "Installing gcc@13 for gfortran support of -static-libquadmath" | ||
brew list gcc@13 || brew install gcc@13 | ||
which gfortran-13 || echo "FC=$(brew --prefix gcc@13)/bin/gfortran-13" >> $GITHUB_ENV | ||
brew install ninja | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm using ninja now. |
||
|
||
- name: Create Build Directory | ||
run: cmake -E make_directory ./build/ | ||
|
@@ -86,43 +144,99 @@ jobs: | |
working-directory: ./build | ||
shell: bash | ||
run: | | ||
cmake -DCMAKE_BUILD_TYPE:STRING=$BUILD_TYPE \ | ||
cmake -G Ninja -DCMAKE_BUILD_TYPE:STRING=$BUILD_TYPE \ | ||
-DCMAKE_OSX_DEPLOYMENT_TARGET:STRING=${{ matrix.macos_dev_target }} \ | ||
-DLINK_WITH_PYTHON:BOOL=ON -DPython_REQUIRED_VERSION:STRING=${{ steps.setup-python.outputs.python-version }} \ | ||
-DPython_ROOT_DIR:PATH=$RUNNER_TOOL_CACHE/Python/${{ steps.setup-python.outputs.python-version }}/${{ matrix.python-arch }}/ \ | ||
-DBUILD_FORTRAN:BOOL=ON -DBUILD_PACKAGE:BOOL=ON \ | ||
-DDOCUMENTATION_BUILD:STRING="BuildWithAll" -DTEX_INTERACTION:STRING="batchmode" \ | ||
-DDOCUMENTATION_BUILD:STRING=$DOCUMENTATION_BUILD -DTEX_INTERACTION:STRING="batchmode" \ | ||
-DENABLE_OPENMP:BOOL=OFF -DUSE_OpenMP:BOOL=OFF \ | ||
-DCPACK_CODESIGNING_DEVELOPPER_ID_APPLICATION:STRING="Developer ID Application: National Renewable Energy Laboratory (K7JYVQJL7R)" \ | ||
-DCPACK_CODESIGNING_NOTARY_PROFILE_NAME:STRING=EnergyPlus \ | ||
Comment on lines
+154
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pass the new configuration variables to cmake so signing happens |
||
../ | ||
|
||
- name: Build Package | ||
working-directory: ./build | ||
shell: bash | ||
run: cmake --build . --target package -j 3 | ||
|
||
- name: otool the exes and libs | ||
shell: bash | ||
working-directory: ./build | ||
run: | | ||
set -x | ||
cd _CPack_Packages/Darwin/TGZ/EnergyPlus*/ | ||
otool -L ExpandObjects || true | ||
otool -L ConvertInputFormat || true | ||
otool -L energyplus || true | ||
otool -L libenergyplusapi.dylib || true | ||
otool -L PreProcess/IDFVersionUpdater/Transition-V23-1-0-to-V23-2-0 || true | ||
otool -L PostProcess/ReadVarsESO || true | ||
otool -L PostProcess/HVAC-Diagram || true | ||
ninja package | ||
|
||
- name: Upload Tarball as artifact for testing | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: energyplus-${{ matrix.os }} | ||
name: energyplus-${{ matrix.os }}.tar.gz | ||
path: build/EnergyPlus-*-${{ matrix.arch }}.tar.gz | ||
if-no-files-found: error | ||
retention-days: 7 | ||
overwrite: false | ||
|
||
- 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 | ||
Comment on lines
+173
to
+180
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 | ||
Comment on lines
+182
to
+216
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 | ||
Comment on lines
+218
to
+238
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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: Upload Tarball to release | ||
uses: svenstaro/upload-release-action@v2 | ||
with: | ||
|
@@ -153,39 +267,96 @@ jobs: | |
include: | ||
- macos_dev_target: 12.1 | ||
os: macos-12 | ||
arch: x86_64 | ||
python-arch: x64 | ||
test_key: mac12 | ||
- macos_dev_target: 13.0 | ||
os: macos-14 | ||
arch: arm64 | ||
python-arch: arm64 | ||
test_key: mac13-arm64 | ||
|
||
steps: | ||
- uses: actions/checkout@v4 # Still need E+ checked out to get testing scripts | ||
with: | ||
path: checkout | ||
|
||
- name: Set up Python ${{ env.Python_REQUIRED_VERSION }} | ||
uses: actions/setup-python@v5 | ||
id: setup-python | ||
with: | ||
python-version: ${{ env.Python_REQUIRED_VERSION }} | ||
architecture: ${{ matrix.python-arch }} | ||
|
||
- name: Gather Test Package from Artifacts | ||
uses: actions/download-artifact@v4 | ||
with: | ||
name: energyplus-${{ matrix.os }} | ||
path: package | ||
|
||
- name: Check Contents | ||
shell: bash | ||
run: ls | ||
|
||
- name: Check Package contents | ||
shell: bash | ||
working-directory: package | ||
run: ls | ||
|
||
- 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just whitespace change, use https://github.com/NREL/EnergyPlus/pull/10645/files?w=1 to not see them. |
||
with: | ||
path: checkout | ||
|
||
- name: Set up Python ${{ env.Python_REQUIRED_VERSION }} | ||
uses: actions/setup-python@v5 | ||
id: setup-python | ||
with: | ||
python-version: ${{ env.Python_REQUIRED_VERSION }} | ||
architecture: ${{ matrix.python-arch }} | ||
|
||
- name: Gather Test Package from Artifacts | ||
uses: actions/download-artifact@v4 | ||
with: | ||
name: energyplus-${{ matrix.os }}.tar.gz | ||
path: package | ||
|
||
- name: Check Contents | ||
shell: bash | ||
run: ls | ||
|
||
- name: Check Package contents | ||
shell: bash | ||
working-directory: package | ||
run: ls | ||
|
||
- name: Run Package Tests | ||
run: python checkout/scripts/package_tests/runner.py --verbose ${{ matrix.test_key }} package/ | ||
|
||
- 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 | ||
Comment on lines
+309
to
+362
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is new. I download the .dmg artifact on the test machine and I:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is helpful to trigger it for testing, without actually creating a release / tag