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

Mac: Enable signing with self signed cert #2944

Merged
merged 1 commit into from
Jun 11, 2023

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Oct 29, 2022

Short description of changes
Enables the CI to use a code signing certificate signed by a non Apple CA to sign development builds.

grafik
grafik

CHANGELOG: Build: Enabled signing of macOS binary if a self signed certificate is given.

Context: Fixes an issue?

Fixes: #2924

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

Probably not.

Status of this Pull Request

Ready for review (and test on Apple Silicon, a repo without the respective secrets set and a repo with the real apple secrets set (@emlynmac 's repo))

What is missing until this pull request can be merged?

Still needs some (external testing). Artifacts are building on my repo: https://github.com/ann0see/jamulus/actions/runs/3352547263/jobs/5554760442

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want (on my repo)
  • 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

AUTOBUILD: Please build all targets

@ann0see ann0see requested a review from hoffie October 29, 2022 18:29
@ann0see ann0see force-pushed the feature/enableMacSelfsign branch 4 times, most recently from 8ba1b8f to 5d7a39a Compare October 29, 2022 18:47
Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Minor nit wrt [usage. Other than that, looks good. Thanks!

As you say, it should receive proper testing, especially of release builds. I think you still have access to @emlynmac's repo as well and could build a fake 0.something version for testing?

.github/autobuild/mac.sh Outdated Show resolved Hide resolved
.github/autobuild/mac.sh Outdated Show resolved Hide resolved
@ann0see
Copy link
Member Author

ann0see commented Oct 29, 2022

I've pushed to emlyns repo, but my macOS VM is currently unavailable, so I can't test. But it builds and seems to be signed.

@ann0see
Copy link
Member Author

ann0see commented Nov 1, 2022

Ok. Tested yesterday, and it seems to be signed.

@ann0see
Copy link
Member Author

ann0see commented Nov 9, 2022

@hoffie I probably did some mistake during the rebase (again...). However, I think it's ok now. Please review again, nevertheless.

@ann0see
Copy link
Member Author

ann0see commented Dec 3, 2022

@pljones @hoffie could you please review this?

@pljones
Copy link
Collaborator

pljones commented Dec 4, 2022

I've no idea what any of the changes mean. It's not affecting anything other than MacOS and none of the builds broke, so I'm happy with it.

@emlynmac
Copy link
Contributor

emlynmac commented Dec 5, 2022

Can we make sure this actually does what is expected?
All of the artifacts have expired, so not possible to test.
I'd be surprised if a self signed binary works on a macOS platform TBH.

@ann0see
Copy link
Member Author

ann0see commented Dec 6, 2022

I'd be surprised if a self signed binary works on a macOS platform TBH.

They do still show a warning but the point is that cert signing problems would show up in production already.

@emlynmac
Copy link
Contributor

emlynmac commented Dec 6, 2022

I'd be surprised if a self signed binary works on a macOS platform TBH.

They do still show a warning but the point is that cert signing problems would show up in production already.

If this is purely to validate the process, then the signed artifact should not be published.

@ann0see
Copy link
Member Author

ann0see commented Dec 7, 2022

I’d there any downside of a self signed test version?

@emlynmac
Copy link
Contributor

emlynmac commented Dec 7, 2022

I’d there any downside of a self signed test version?

If it's published and people try to use it, then issues might be raised in confusion.

@ann0see
Copy link
Member Author

ann0see commented Dec 7, 2022

Yes, but the same would be true for unsigned builds. This would just affect PR builds and maybe the legacy build. Release builds would be built on your repo.

@ann0see ann0see changed the base branch from master to main December 26, 2022 19:02
Co-authored-by: Christian Hoffmann <christian@hoffie.info>
@pljones
Copy link
Collaborator

pljones commented Apr 19, 2023

Hi @ann0see, is there any progress here?

@ann0see
Copy link
Member Author

ann0see commented Apr 19, 2023

There's still a Test and OK by Emlyn outstanding.

@pljones pljones added the tooling Changes to the automated build system label Jun 10, 2023
Copy link
Collaborator

@pljones pljones left a comment

Choose a reason for hiding this comment

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

Looks syntactically correct.

@pljones pljones merged commit e4daf27 into jamulussoftware:main Jun 11, 2023
@ann0see ann0see deleted the feature/enableMacSelfsign branch June 11, 2023 18:02
@ann0see
Copy link
Member Author

ann0see commented Jun 11, 2023

Ok. Now we need to add a certificate.

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: Done
Development

Successfully merging this pull request may close these issues.

Mac: Align non-signed Mac build logic with signed builds
4 participants