-
Notifications
You must be signed in to change notification settings - Fork 223
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
Automate build of iOS signed ipa file for App Store submission #2625
base: main
Are you sure you want to change the base?
Conversation
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.
Interesting! To me it seems as if this still needs manual work. In an ideal workflow a release push would upload the app to the app store automatically and just wait on someone to press a "publish" button there.
.github/autobuild/ios.sh
Outdated
security find-identity -v | ||
|
||
# Tell Github Workflow that we need notarization & stapling: | ||
echo "::set-output name=macos_signed::true" |
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.
should be renamed to apple_signed if we merge this.
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.
or maybe ios_signed
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.
@ann0see it is actually possible to upload the binary to the App Store, using "altool" as outlined here: https://help.apple.com/app-store-connect/#/devb1c185036
However, as the App Store submission process involves a number of manual setup steps anyway (ie creating the actual submission on the App Store Connect website, adding screenshots, descriptions, compliance details and all sorts) and you likely only want to upload specific versions to the App Store (rather than having it run automatically), I thought the generation of the signed, verifiable IPA is a big enough win for this automation.
Quick note: the only manual step remaining once you have the signed IPA is to drag and drop the IPA file into Transporter (on macOS), and click the Upload button.
If I have time today though I will experiment with adding the upload directly using xcrun altool.
} | ||
|
||
pass_artifact_to_job() { | ||
local artifact="jamulus_${JAMULUS_BUILD_VERSION}_iOSUnsigned${ARTIFACT_SUFFIX:-}.ipa" | ||
local artifact="jamulus_${JAMULUS_BUILD_VERSION}_iOS_unsigned${ARTIFACT_SUFFIX:-}.ipa" |
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.
Not sure how much a signed IPA on the web without the app store helps?
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.
I don't think there's any point to that, no - the signed IPA is purely for App Store submission. So it's really for @emlynmac or whoever is managing the Jamulus App Store account to have a ready-built file for submission.
I assume the unsigned IPA is useful for general testing purposes so obviously I've kept that in.
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.
Looks like this PR needs a bit more work to make it happy with the case of not having certificates around.
.github/autobuild/ios.sh
Outdated
security find-identity -v | ||
|
||
# Tell Github Workflow that we need notarization & stapling: | ||
echo "::set-output name=macos_signed::true" |
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.
or maybe ios_signed
echo "Moving build artifact to deploy/${artifact}" | ||
mv ./deploy/Jamulus.ipa "./deploy/${artifact}" | ||
mv ./deploy/Jamulus_unsigned.ipa "./deploy/${artifact}" |
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.
Probably not worth keeping this around, only signed version would be useful past making sure the build works.
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.
Ok, I left a note above, I wasn't sure if people were maybe using the unsigned IPA for local testing or somesuch.
No problem to remove it if you prefer.
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.
Should not be removed IMO.
building_on_os: macos-10.15 | ||
base_command: QT_VERSION=5.15.2 ./.github/autobuild/ios.sh | ||
building_on_os: macos-11 | ||
base_command: QT_VERSION=5.15.2 SIGN_IF_POSSIBLE=1 ./.github/autobuild/ios.sh |
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.
Could you use the presence of a certificate in the environment to determine whether to sign, so not needing another parameter?
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.
I would rather follow the lead as being done for macOS, which uses that flag. Or we can remove from both macOS and iOS but to have in just one feels weird.
@danryu The CI failed. We currently don't have a valid cert on this repo. |
Ok I've pushed a fix now to check for the cert non-zero value before attempting to move the signed IPA for download. This should resolve this CI failure where there is no valid cert in place. Please note also that if signing is requested and dependencies are satisfied, the script will now attempt to validate and upload to App Store using the existing NOTARIZATION_USER and NOTARIZATION_PASSWORD. We may need to fine-tune this behaviour if eg re-uploading the same version (eg 3.9.0) to the App Store repeatedly (I think it will fail rather than updating the existing similar version). Let me know what you think should happen in this case. |
Hmm. I think the upload should only happen if we have a release build (there's a GitHub variable for that, I think). Look for the variable which enables/disabled uploading to GitHub releases. |
Ok, the Github release stuff switches on |
Yes. I think so. |
I'm working on adding the extra step now. Also: I've noticed that the iOS App Store upload validation (to appear in TestFlight in App Connect) requires an images assets catalog in a directory entitled |
@danryu any updates here? |
Hi @ann0see yes, I have Store submission working as part of the release procedure (also for macOS for the other PR). |
Can the signed ipa be re-signed for debugging? If yes, no need to have the unsigned ipa. Else just provide the unsigned one and upload the signed one to Apple. |
I'm actually not 100% sure on that point, so I will keep the unsigned IPA for now, to not break the existing pattern. |
I am re-assessing this PR in the light of errors that I get when running apps in TestFlight, that have been built and submitted by this automation. The app builds fine and is validated and accepted by TestFlight - but when the app actually starts it quits immediately with a SIG11 error. |
Strange. Do you use Qt5? |
No, Qt 6.3.2 and actually 6.4.0 for iOS (for some touch-handling fixes). It is indeed very strange - after all, if TestFlight validates the package, one would expect it to run! I suspect it may be related to something like the handling of Bitcode in the build, or something equally obscure. Incidentally, the result is the same when I use a dedicated Github Action for iOS building - yukiarrr/[email protected] - which I think might be the simplest way of doing iOS builds going forward. |
Yes, the iOS build crashes with Qt6 for me too (at least on GH actions). It's a known issue with Qt6 |
Hmmm, I think I may have fixed that particular issue by including this in .pro file for iOS: Though I don't have the reference for the fix to hand. Please check and try the Koord app in the iOS App Store for proof that Qt6 works fine with iOS :) |
Great. That's a modified Jamulus client I suppose? If you plan to maintain your app and keep it compatible and up to date, we could link it as Jamulus compatible 3rd party App here in the KB: https://jamulus.io/kb/2022/03/04/Alternative-Install-Methods.html |
That would be great! We definitely intend on maintaining it :) and also keeping upstream compatibility! There are a few points of interest in the app - but I might open a quick Discussion for that! |
Adding this stackoverflow post as reference for the new flags: https://stackoverflow.com/questions/45508043/qt-ios-linker-error-entry-point-main-undefined |
Needs a rebase. Not closing yet, but might in future... |
@danryu I'd suggest you add a very brief note to compatibility at https://github.com/jamulussoftware/jamuluswebsite/edit/release/_posts/2022-03-04-Alternative-Install-Methods.md without any big links to Koord. It can and should not be an ad. |
Sorry I don't understand this request. Why would I mention Koord, and why is a specific note required? |
No. There is interest in this PR. This was only outstanding from above (#2625 (comment)). |
# add notarization/validation/upload password to keychain | ||
xcrun altool --store-password-in-keychain-item --keychain build.keychain APPCONNAUTH -u $NOTARIZATION_USER -p $NOTARIZATION_PASSWORD | ||
# set lock timeout on keychain to 6 hours | ||
security set-keychain-settings -lut 21600 |
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.
As in the macOS case - maybe this is no longer needed.
# Signing was requested, now check all prerequisites: | ||
[[ -n "${IOSDIST_CERTIFICATE:-}" ]] || return 1 | ||
[[ -n "${IOSDIST_CERTIFICATE_ID:-}" ]] || return 1 | ||
[[ -n "${IOSDIST_CERTIFICATE_PWD:-}" ]] || return 1 | ||
[[ -n "${NOTARIZATION_PASSWORD:-}" ]] || return 1 | ||
[[ -n "${IOS_PROV_PROFILE_B64:-}" ]] || return 1 | ||
[[ -n "${KEYCHAIN_PASSWORD:-}" ]] || return 1 | ||
|
||
echo "Signing was requested and all dependencies are satisfied" | ||
|
||
# use this as filename for Provisioning Profile | ||
IOS_PP_PATH="embedded.mobileprovision" | ||
|
||
## Put the cert to a file | ||
# IOSDIST_CERTIFICATE - iOS Distribution | ||
echo "${IOSDIST_CERTIFICATE}" | base64 --decode > iosdist_certificate.p12 | ||
|
||
## Echo Provisioning Profile to file | ||
echo -n "${IOS_PROV_PROFILE_B64}" | base64 --decode > $IOS_PP_PATH |
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.
As in the macOS case, probably worth putting distribution related stuff in an if.
This PR adds automation to create a signed ipa (installer) file for direct submission to the iOS App Store.
CHANGELOG: adds iOS signed pkg build automation
Context: automates building of signed pkg file for iOS App Store
Does this change need documentation? What needs to be documented and how?
Required:
Certificates:
Identifier:
Profile:
Status of this Pull Request
What is missing until this pull request can be merged?
Checklist