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

ScmRevGen: Don't generate Info.plist files directly #13210

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

OatmealDome
Copy link
Member

Some generators (like Unix Makefiles and Xcode) copy an app's Info.plist at configure time. This causes a problem when we need to generate the Info.plist at build time, like how we currently do it with ScmRevGen.

Instead of generating the Info.plist directly in ScmRevGen, provide an Info.plist without any version information to CMake at configure time, have ScmRevGen generate a separate plist file with the version information at build time, and then merge the two together to create the final Info.plist.

Fixes https://bugs.dolphin-emu.org/issues/13669.

(This PR also includes a bonus fix for codesigning in Xcode.)

@TellowKrinkle
Copy link
Contributor

Since you seem to be merging in place, how does PlistBuddy Merge handle the case when the key already exists in the plist?

Some generators (like Unix Makefiles and Xcode) copy an app's Info.plist at configure time.
This causes a problem when we need to generate the Info.plist at build time, like how we
currently do it with ScmRevGen. Instead of generating the Info.plist directly in ScmRevGen,
provide an Info.plist without any version information to CMake at configure time, have
ScmRevGen generate a separate plist file with the version information at build time, and
then merge the two together to create the final Info.plist.
@OatmealDome
Copy link
Member Author

When I was testing it, I think that the base Info.plist was being re-copied on every build so there weren't any duplicate keys.

I've went ahead and added some protection against duplicate keys just in case, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants