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

[macos] codesign binary addons #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kambala-decapitator
Copy link

Addition to xbmc/xbmc#17254.

'PACKAGE_ZIP=ON',
"PACKAGE_DIR=`pwd`/../../../../cmake/addons/build/zips",
]
if (platform == 'osx-x86_64' && version == 'Leia')
Copy link
Member

Choose a reason for hiding this comment

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

Why Leia only? Will newer Kodi versions not get signed anymore?
Does setting CODE_SIGN_IDENTITY for the other platforms break the build?

Choose a reason for hiding this comment

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

like I wrote in the related PR, Leia releases are already signed, while nightly builds aren't yet. The version condition will be removed in the next PR when Kodi builds become signed.

CODE_SIGN_IDENTITY doesn't break other platforms, but why add an unused variable? Should I remove the platform condition still?

Copy link
Member

Choose a reason for hiding this comment

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

Why make the code more complex then required?
I would actually prefer if there is no change required in the buildPlugin function and handle it in Kodi code only.

Copy link
Author

@kambala-decapitator kambala-decapitator Jan 26, 2020

Choose a reason for hiding this comment

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

I would actually prefer if there is no change required in the buildPlugin function and handle it in Kodi code only.

could you give a hint how to pass the signing identity as a parameter in such case? Basically signing should be done only by Jenkins, development builds don't care about code signing. But still it should be possible to pass the signing identity manually (e.g. for testing on local machine). That's why I thought it'd be natural to modify the actual build command.

edit: should I add the signing identity to tools/buildsteps/defaultenv?

Copy link
Member

Choose a reason for hiding this comment

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

e.g. only pass -DSIGN:BOOL=ON and handle everything else in cmake/addons or cmake/scripts/common/AddonHelpers.cmake

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.

2 participants