Skip to content

Comments

Mac: Enable signing with self signed cert#2944

Merged
pljones merged 1 commit intojamulussoftware:mainfrom
ann0see:feature/enableMacSelfsign
Jun 11, 2023
Merged

Mac: Enable signing with self signed cert#2944
pljones merged 1 commit intojamulussoftware:mainfrom
ann0see:feature/enableMacSelfsign

Conversation

@ann0see
Copy link
Member

@ann0see ann0see commented Oct 29, 2022 •
edited
Loading

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
hoffie
hoffie approved these changes Oct 29, 2022
View reviewed changes
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?

@ann0see ann0see force-pushed the feature/enableMacSelfsign branch from 8e850c4 to e411028 Compare October 29, 2022 20:01
@ann0see
Copy link
Member Author

ann0see commented Oct 29, 2022 •
edited
Loading

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 ann0see requested a review from pljones November 6, 2022 20:52
@ann0see ann0see force-pushed the feature/enableMacSelfsign branch from e411028 to 2142c98 Compare November 9, 2022 19:33
@ann0see ann0see closed this Nov 9, 2022
@ann0see ann0see force-pushed the feature/enableMacSelfsign branch from 2142c98 to d2052e9 Compare November 9, 2022 19:36
@ann0see ann0see reopened this Nov 9, 2022
@ann0see ann0see requested a review from hoffie November 9, 2022 19:44
@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
@ann0see ann0see force-pushed the feature/enableMacSelfsign branch from 1446e32 to 3c239bc Compare February 16, 2023 10:30
@ann0see ann0see mentioned this pull request Feb 16, 2023
Merged
5 tasks
@pljones pljones added this to the Release 3.10.0 milestone Apr 19, 2023
@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.

@ann0see ann0see mentioned this pull request May 3, 2023
Closed
5 tasks
@pljones pljones added the tooling Changes to the automated build system label Jun 10, 2023
pljones
pljones approved these changes Jun 10, 2023
View reviewed changes
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

Reviewers

@pljones pljones pljones approved these changes

@hoffie hoffie Awaiting requested review from hoffie

Assignees

@ann0see ann0see

Labels

tooling Changes to the automated build system

Projects

Tracking
Status: Done

Milestone

Release 3.10.0

Development

Successfully merging this pull request may close these issues.

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

4 participants

@ann0see @pljones @emlynmac @hoffie