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

Add changable bundle id and docs for macOS signing #3348

Closed
wants to merge 2 commits into from

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Aug 19, 2024

Short description of changes

Adds variable for a changable Bundle ID for notarization for macOS. One can add the bundle id here: https://github.com/jamulussoftware/jamulus/settings/variables/actions/new

Also adds some more (basic) documentation which needs improvement as soon as signing works, and some clean up.

CHANGELOG: Autobuild: Use variable for Bundle ID on macOS. This allows to change the bundle ID for macOS notarization on GitHub

Context: Fixes an issue?

No. Related to: #3255 (comment)

Does this change need documentation? What needs to be documented and how?

Maybe the signing - once it works.

Status of this Pull Request

Ready for review

What is missing until this pull request can be merged?
Review (language wise)

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want (should be merged if it doesn't break. I will try signing on my own repo)
  • 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

This comment was marked as outdated.

@ann0see ann0see requested review from softins and pljones August 19, 2024 09:27
@ann0see ann0see force-pushed the mac/AddBundleVarAndDoc branch from ea4bad4 to 26bd38e Compare August 19, 2024 09:31
@ann0see ann0see force-pushed the mac/AddBundleVarAndDoc branch from 26bd38e to f336bf3 Compare August 19, 2024 09:34
@ann0see ann0see force-pushed the mac/AddBundleVarAndDoc branch from f336bf3 to be8f12c Compare August 19, 2024 09:36
@ann0see ann0see force-pushed the mac/AddBundleVarAndDoc branch from 81b3e5c to be8f12c Compare August 19, 2024 09:42
@pljones
Copy link
Collaborator

pljones commented Aug 19, 2024

Will this be in for 3.11.0 release? If so we should put it on the tracking board as in progress for the release. As it affects the release process, I feel we should try to target a release...

@ann0see
Copy link
Member Author

ann0see commented Aug 19, 2024

Yes. Agree

@ann0see ann0see added this to the Release 3.11.0 milestone Aug 19, 2024
@ann0see
Copy link
Member Author

ann0see commented Aug 22, 2024

Maybe this will be replaced by another PR soon. I'm refactoring the build script for macOS now.

Copy link
Member

@softins softins left a comment

Choose a reason for hiding this comment

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

Changes look sensible to me. So happy to approve, even if it might get superseded.

MAC_STORE_APP_CERT: ${{ secrets.MACAPP_CERT}}
MACOS_CERTIFICATE: ${{ secrets.MACOS_CERT }} # Base64 encoded (Developer ID Application (?)) certificate. See https://help.apple.com/xcode/mac/current/#/dev154b28f09
MACOS_CERTIFICATE_PWD: ${{ secrets.MACOS_CERT_PWD }} # Password protecting MACOS_CERTIFICATE
MACOS_CERTIFICATE_ID: ${{ secrets.MACOS_CERT_ID }} # Certificate ID of MACOS_CERTIFICATE. If unknown, import MACOS_CERT into keychain and check the ID there
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be more explicit?

Suggested change
MACOS_CERTIFICATE_ID: ${{ secrets.MACOS_CERT_ID }} # Certificate ID of MACOS_CERTIFICATE. If unknown, import MACOS_CERT into keychain and check the ID there
MACOS_CERTIFICATE_ID: ${{ secrets.MACOS_CERT_ID }} # Certificate ID of MACOS_CERTIFICATE. If unknown, import secrets.MACOS_CERT into keychain and check the ID there

or have I misunderstood the reference?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. This is OK

@ann0see
Copy link
Member Author

ann0see commented Aug 22, 2024

Ok. Superseded by: #3352

@ann0see ann0see closed this Aug 22, 2024
@pljones pljones removed this from the Release 3.11.0 milestone Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants