-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
Mono/macOS: Move GodotSharp data to Resources to allow codesigning #43768
Conversation
Was just seeing problems if you enable "Hardened Runtime" without an accompanying entitlements file letting Mono work. Probably not something to add to the export process itself, since people might want custom entitlements, but might be worth adding to the documentation that Mono Mac exports need at least these minimal entitlements for codesigning with the hardened runtime to work. (Since hardened runtime is required for notarization, it will be more relevant in the future.) |
There are few Only Original code seems to be correct (ant it's for editor release):
Template part is here: https://github.com/godotengine/godot-build-scripts/blob/master/build-release.sh#L351 And should be changed to do the same as editor (and export code change should be ok). -cp -rp out/macosx/templates-mono/data.mono.osx.universal.* osx_template.app/Contents/MacOS/
+mkdir -p osx_template.app/Contents/{Frameworks,Resources}/data.mono.osx.universal.release/Mono
+mkdir -p osx_template.app/Contents/{Frameworks,Resources}/data.mono.osx.universal.release_debug/Mono
+cp -rp out/macosx/templates-mono/data.mono.osx.universal.release/Mono/lib osx_template.app/Contents/Frameworks/data.mono.osx.universal.release/Mono
+cp -rp out/macosx/templates-mono/data.mono.osx.universal.release/Mono/etc osx_template.app/Contents/Resources/data.mono.osx.universal.release/Mono
+cp -rp out/macosx/templates-mono/data.mono.osx.universal.release_debug/Mono/lib osx_template.app/Contents/Frameworks/data.mono.osx.universal.release_debug/Mono
+cp -rp out/macosx/templates-mono/data.mono.osx.universal.release_debug/Mono/etc osx_template.app/Contents/Resources/data.mono.osx.universal.release_debug/Mono |
There's an option to add entitlements file, but we probably do need to add default one for the Mono exports. |
I know that's the preferred way, but codesign won't accept it. The current editor is also not able to be codesigned -- we get this error:
The problem is that the library files are nested in directories that include periods, which makes codesign try to treat them like a bundle... and then it doesn't recognize the type of bundle, so it craps out. So we either need to rename those directories (and probably patch Mono to deal with a new naming structure) or we need to put them someplace else in the |
Where should that file live? Or should it be generated/inserted during the export if the user hasn't defined one? Easiest solution would be to just have the export fail with a very specific error message, but I'm not sure how we feel about that. |
In the export there's only three Something like:
|
Ah, ok. I see what you're saying. Yes, that makes sense. We can leave the |
Exactly, |
I have a codesignable build of the editor working with the desired layout, and changes to the library loading code, which allays my fears from caveat number 3 in the initial PR description. There's a small issue, though, in that Mono really expects On top of being aesthetically displeasing (I'm going to experiment and see if things really need to be that nested), it has a bit of a code smell to it. But it does work. @bruvzg, how do you feel about this? Is it something that should be addressed by the mono-build scripts, or is doing some (relatively easy) processing on the (Holding off on updating the export templates until we feel solid about the editor build, but they're a relatively easy setup once it's good.) |
Starting on the export stuff, and I think I've got it working. One thing I'm not able to test is How does this get used, though? So far as I can tell all the assemblies referenced in the project get put into the If someone has an example project that uses extra assemblies like this, I'm happy to check that it still works. Also: I'm seeing the |
I've rebased this to master -- any additional thoughts on how best to handle the modifications to the config file? |
This was make like this to make it easy to debug Godot during development. Having to create an app bundle complicates things. |
From what I understand now we have the data folder for macOS split between godot/modules/mono/build_scripts/mono_configure.py Lines 403 to 430 in d834789
IMO we should make the mono module build take care of it. This makes it less problematic for godot-build-scripts. Keeping this logic in the SCons build is also good as not everyone else uses our release build scripts. |
I was finding that I couldn't get the I agree it would be nice to keep the Mono files all together, but to get a signable directory layout that is set up the way @bruvzg (and Apple guidelines) prefer, it looks like it will have to be split. Would you want to see this behind a flag set by SCons so that it only happens during the shipping builds? |
It's possible to run a standalone executable by setting |
Yeah, for some reason I couldn't it to load the |
Ah, I see. I haven't touched macOS with master after the Vulkan merge, only with 3.2. If latest master no longer supports running without creating an app bundle then I guess this change is fine.
I don't want to keep these files all together in a single data directory, I'm fine with the change. I want it to be SCons who outputs these files already split into the two directories ( |
Hello! Sorry I went MIA -- my Godot project was backburnered for a bit and I had some real-life things crop up as well. I have some bandwidth in the next week or so to jump in on this, though, so will review the changes ASAP. What's the current timetable on 3.2.4? |
Hard to say as it depends on which bugs are found and are critical enough to delay the release. But the best case would be within the next two weeks. |
Great, good to know. I'll get my branch synced to the master in the next few days and then can hopefully apply the changes by the end of the week/end. Thanks for poking me! |
Quick update: I'm living in a region with very flaky internet and have had trouble pulling the Godot repo, and since Git doesn't do well with resuming broken transfers, it's been a pain. Currently have a workflow that involves SSHing to a more stable connection, tarballing the repo there and then using other transfer methods that can resume when they stall... but you can imagine that it impedes the process a bit. 😉 Still working here, but I can't promise it in time for 3.2.4. Sorry about that. Will update when I can. 😢 |
I went ahead and rebased this PR + amended @neikeq's changes from #43768 (comment) directly into the commit (I could have kept them as separate commits but it seems they're both necessary parts of the same change so it's best in a single commit IMO). I haven't actually tested the changes yet, but I'll do a |
@neikeq With your changes applied I get the following structure after compiling an editor build:
I suspect that the files under |
I fixed the issue with |
Reverting my change for diff --git a/modules/mono/build_scripts/api_solution_build.py b/modules/mono/build_scripts/api_solution_build.py
index 9abac22df6..b2417ddc8e 100644
--- a/modules/mono/build_scripts/api_solution_build.py
+++ b/modules/mono/build_scripts/api_solution_build.py
@@ -62,7 +62,11 @@ def build(env_mono):
for build_config in ["Debug", "Release"]:
output_dir = Dir("#bin").abspath
- editor_api_dir = os.path.join(output_dir, "GodotSharp", "Api", build_config)
+ editor_api_dir = (
+ os.path.join(output_dir, "GodotSharp", "Api", build_config)
+ if env_mono["platform"] != "osx"
+ else os.path.join(output_dir, "GodotSharp.resources", "Api", build_config)
+ )
targets = [os.path.join(editor_api_dir, filename) for filename in target_filenames]
|
This allows signing the editor .app (will be done in next commit) and should let users sign their macOS exports. Co-authored-by: Shane Liesegang <shane@techie.net>
See godotengine/godot-build-scripts#24, the editor .app which I can generate with this PR can now be signed and notarized but it's broken, it can't find the Mono libs. Help needed :) |
…igning Co-authored-by: Ignacio Etcheverry <ignalfonsore@gmail.com>
In the end we decided to put everything in |
Thanks everyone for the joint work on this issue! I'll update container build scripts soon and make some binaries to confirm that all works as expected. |
@@ -114,7 +114,7 @@ class EditorExportPlatformOSX : public EditorExportPlatform { | |||
virtual void get_platform_features(List<String> *r_features) override { | |||
r_features->push_back("pc"); | |||
r_features->push_back("s3tc"); | |||
r_features->push_back("OSX"); | |||
r_features->push_back("macOS"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be skipped on cherry-picking to 3.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder!
Cherry-picked for 3.2.4. |
This allows signing the editor .app (will be done in next commit) and should let users sign their macOS exports. Co-authored-by: Shane Liesegang <shane@techie.net>
This allows signing the editor .app (will be done in next commit) and should let users sign their macOS exports. Co-authored-by: Shane Liesegang <shane@techie.net>
This fixes #43732 -- that exported Mono projects could not be codesigned on the Mac due to their directory layout. This change creates a directory layout that is capable of being signed, and combined with a change to the release build scripts will also create a version of the editor and export templates that work similarly.
A few caveats:
In the original issue, @bruvzg indicated thatPer discussion below, theetc
should go intoFrameworks
but this causes the codesign to fail due to the policies about nested code. I've put it intoResources
instead, where the codesign cares less about the directory structure.lib
directory has been split up betweenResources
andFrameworks
.data_{GAMENAME}
, since the Mono distribution is more split up through the.app
. I assume it was originally called that to have parity with the Linux and Windows exports, but since Mac applications are all bundled together and those details are invisible to the user, I didn't think that needed to be preserved. If it's significant for some other reason, please let me know.I'm slightly concerned that none of the loading code had to change to accommodate the different directories. I can't tell if Mono is just very good at finding its files, or if it's somehow picking up my system installation at runtime, or what. It works, but it's not 100% clear how, and I'm not a fan of that.The loading code has now been changed, so it's clear there isn't magic going on..app
packages that are unusable until they havelibMoltenVK.dylib
added to theFrameworks
directory. I was following the existing build practices there, as it seems like the maintainers haven't yet settled on the best means of including it (Vulkan: Documentation and/or buildsystem changes needed for MoltenVK #36213). I've poked on that issue thread to re-raise the question, and if it's something that can be decided soon-ish, this PR could easily be amended to accommodate whatever method makes sense.Long-term: