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

Automate build of iOS signed ipa file for App Store submission #2625

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

danryu
Copy link
Contributor

@danryu danryu commented May 9, 2022

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:

  1. In Apple Developer Account, create the following resources in in https://developer.apple.com/account/resources/certificates/list
  • Certificates:

    • iOS Distribution
  • Identifier:

    • app ID (bundleID)
  • Profile:

    • iOS Distribution (AppStore) - ProvisioningProfile
  1. Add the certs and provisioning profile to Github Secrets as per https://docs.github.com/en/actions/deployment/deploying-xcode-applications/installing-an-apple-certificate-on-macos-runners-for-xcode-development

Status of this Pull Request

What is missing until this pull request can be merged?

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@ann0see ann0see added the feature request Feature request label May 9, 2022
@ann0see ann0see added this to Triage in Tracking (old) via automation May 9, 2022
@ann0see ann0see marked this pull request as draft May 9, 2022 19:39
Copy link
Member

@ann0see ann0see left a 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.

security find-identity -v

# Tell Github Workflow that we need notarization & stapling:
echo "::set-output name=macos_signed::true"
Copy link
Member

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe ios_signed

Copy link
Contributor Author

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"
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@emlynmac emlynmac left a 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.

security find-identity -v

# Tell Github Workflow that we need notarization & stapling:
echo "::set-output name=macos_signed::true"
Copy link
Contributor

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}"
Copy link
Contributor

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.

Copy link
Contributor Author

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.

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
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@ann0see
Copy link
Member

ann0see commented May 15, 2022

@danryu The CI failed. We currently don't have a valid cert on this repo.

@danryu
Copy link
Contributor Author

danryu commented May 16, 2022

@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.

@ann0see
Copy link
Member

ann0see commented May 16, 2022

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.

@danryu
Copy link
Contributor Author

danryu commented May 16, 2022

Ok, the Github release stuff switches on needs.create_release.outputs.publish_to_release == 'true' - so maybe I should add a separate extra step in autobuild.yml for iOS App Store upload - after the Notarization and Staple steps perhaps?
Then I can split out the validate and upload stages from the main build run, which seems sensible.

@ann0see
Copy link
Member

ann0see commented May 16, 2022

Yes. I think so.

@danryu
Copy link
Contributor Author

danryu commented May 19, 2022

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 Images.xcassets - I have this working in branch CI as a subdir of ios. However the asset catalog is a collection of ~26 files, normally generated by XCode or another utility. I'm thinking of integrating a tool like https://github.com/alfianlosari/AppIconGenCLI, which would then mean we just have to include a single 1024x1024 png file to generate all the others, and saving us from putting all 26+ files in source control.

@ann0see
Copy link
Member

ann0see commented Jun 8, 2022

I think both approaches have advantages and disadvantages (another CI job vs more data in the repo). Any other comments on this @ngocdh @emlynmac

@ann0see
Copy link
Member

ann0see commented Jul 24, 2022

@danryu any updates here?

@ann0see ann0see moved this from Triage to Waiting Externally in Tracking (old) Jul 24, 2022
@danryu
Copy link
Contributor Author

danryu commented Jul 25, 2022

Hi @ann0see yes, I have Store submission working as part of the release procedure (also for macOS for the other PR).
I just need to confirm what the desired behavior is before updating the PR. Do we still want to keep the unsigned IPA if the signed one is generated?

@ann0see
Copy link
Member

ann0see commented Jul 25, 2022

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.

@danryu
Copy link
Contributor Author

danryu commented Jul 26, 2022

I'm actually not 100% sure on that point, so I will keep the unsigned IPA for now, to not break the existing pattern.

@pljones pljones moved this from Waiting Externally to Triage in Tracking (old) Oct 5, 2022
@danryu
Copy link
Contributor Author

danryu commented Oct 11, 2022

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.
This does NOT happen when the same build and submission is carried out locally on macOS/XCode.
I will update this PR when I have worked out the reason for the CI failure.

@ann0see
Copy link
Member

ann0see commented Oct 11, 2022

Strange. Do you use Qt5?

@danryu
Copy link
Contributor Author

danryu commented Oct 12, 2022

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/ios-build-action@v1.5.0 - which I think might be the simplest way of doing iOS builds going forward.

@ann0see
Copy link
Member

ann0see commented Oct 12, 2022

Yes, the iOS build crashes with Qt6 for me too (at least on GH actions). It's a known issue with Qt6

#2711

@danryu
Copy link
Contributor Author

danryu commented Oct 12, 2022

Yes, the iOS build crashes with Qt6 for me too (at least on GH actions). It's a known issue with Qt6

#2711

Hmmm, I think I may have fixed that particular issue by including this in .pro file for iOS:
QMAKE_LFLAGS += -Wl,-e,_qt_main_wrapper

Though I don't have the reference for the fix to hand.
With this in place, I've been building and running the iOS app for several months, with Qt 6.3.2 and now 6.4.0.
However, within the last 4-6 weeks (roughly), TestFlight has been validating and accepting the CI iOS builds, only to have them crash with SIG11 on startup.
I spent considerable time building from historical commits, exactly replicating previous configurations - but couldn't shake the error.
To me it seems like an opaque change in how TestFlight is handling the builds in each case - very frustrating.
The workaround currently for me is to do the local XCode build as mentioned.

Please check and try the Koord app in the iOS App Store for proof that Qt6 works fine with iOS :)

@ann0see
Copy link
Member

ann0see commented Oct 12, 2022

Koord app

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

@danryu
Copy link
Contributor Author

danryu commented Oct 14, 2022

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!
https://github.com/koord-live/koord-app
We are a few months behind upstream master, and that will be merged in shortly.

There are a few points of interest in the app - but I might open a quick Discussion for that!

@danryu danryu changed the title Draft: automate build of iOS signed ipa file for App Store submission Automate build of iOS signed ipa file for App Store submission Oct 17, 2022
@ann0see ann0see changed the base branch from master to main December 26, 2022 19:08
@ann0see
Copy link
Member

ann0see commented May 21, 2023

Adding this stackoverflow post as reference for the new flags: https://stackoverflow.com/questions/45508043/qt-ios-linker-error-entry-point-main-undefined

@ann0see
Copy link
Member

ann0see commented Jul 1, 2023

Needs a rebase. Not closing yet, but might in future...

@pljones pljones removed this from Triage in Tracking (old) Jul 28, 2023
@pljones pljones added tooling Changes to the automated build system and removed feature request Feature request labels Aug 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Changes to the automated build system
Projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

None yet

4 participants