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: Only set entitlements for main binaries, fixing C# signing issues #95498

Merged

Conversation

LeonardoDemartino
Copy link
Contributor

This fixes #91469

@LeonardoDemartino LeonardoDemartino requested a review from a team as a code owner August 13, 2024 19:13
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

That's fine for .dylibs but probably should be applied to .frameworks as well (same issue is likely relevant for GDExtensions).

Something like:

diff --git a/platform/macos/export/export_plugin.cpp b/platform/macos/export/export_plugin.cpp
index 2ff02d2e74..de145d431f 100644
--- a/platform/macos/export/export_plugin.cpp
+++ b/platform/macos/export/export_plugin.cpp
@@ -1092,7 +1092,7 @@ void EditorExportPlatformMacOS::_code_sign(const Ref<EditorExportPreset> &p_pres
 			List<String> args;
 			args.push_back("sign");
 
-			if (p_path.get_extension() != "dmg") {
+			if (!p_ent_path.is_empty()) {
 				args.push_back("--entitlements-xml-path");
 				args.push_back(p_ent_path);
 			}
@@ -1153,7 +1153,7 @@ void EditorExportPlatformMacOS::_code_sign(const Ref<EditorExportPreset> &p_pres
 				args.push_back("runtime");
 			}
 
-			if (p_path.get_extension() != "dmg") {
+			if (!p_ent_path.is_empty()) {
 				args.push_back("--entitlements");
 				args.push_back(p_ent_path);
 			}
@@ -1237,7 +1237,7 @@ void EditorExportPlatformMacOS::_code_sign_directory(const Ref<EditorExportPrese
 		}
 
 		if (extensions_to_sign.has(current_file.get_extension())) {
-			String ent_path = p_ent_path;
+			String ent_path;
 			bool set_bundle_id = false;
 			if (sandbox && FileAccess::exists(current_file_path)) {
 				int ftype = MachO::get_filetype(current_file_path);
@@ -1357,7 +1357,7 @@ Error EditorExportPlatformMacOS::_copy_and_sign_files(Ref<DirAccess> &dir_access
 			_code_sign_directory(p_preset, p_in_app_path, p_ent_path, p_helper_ent_path, p_should_error_on_non_code_sign);
 		} else {
 			if (extensions_to_sign.has(p_in_app_path.get_extension())) {
-				String ent_path = p_ent_path;
+				String ent_path;
 				bool set_bundle_id = false;
 				if (p_sandbox && FileAccess::exists(p_in_app_path)) {
 					int ftype = MachO::get_filetype(p_in_app_path);
@@ -2210,7 +2210,7 @@ Error EditorExportPlatformMacOS::export_project(const Ref<EditorExportPreset> &p
 				if (ep.step(TTR("Code signing DMG"), 3)) {
 					return ERR_SKIP;
 				}
-				_code_sign(p_preset, p_path, ent_path, false, false);
+				_code_sign(p_preset, p_path, String(), false, false);
 			}
 		} else if (export_format == "pkg") {
 			// Create a Installer.

This will also ensure the same is done with built-in ad-hoc signer (which is irrelevant for the issue, but is good for consistency).

@Calinou Calinou added bug platform:macos topic:dotnet topic:export cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Aug 13, 2024
@Calinou Calinou added this to the 4.4 milestone Aug 13, 2024
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Seems to be working fine for out of App Store exports.

Please squash the commits into one, see pull request workflow.

@akien-mga akien-mga changed the title Only main binaries require entitlements. This fixes signing issues on macOS macOS: Only set entitlements for main binaries, fixing C# signing issues Aug 16, 2024
@akien-mga akien-mga merged commit de44c20 into godotengine:master Aug 16, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@TCROC
Copy link
Contributor

TCROC commented Aug 30, 2024

This fixed it for me! Just cherry picked it into my fork! We were running into this issue with all of our gdextensions as well as additional macos frameworks such as godotsteam https://github.com/GodotSteam/GodotSteam

@akien-mga
Copy link
Member

Cherry-picked for 4.3.1.

@akien-mga akien-mga removed the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Sep 16, 2024
@LeonardoDemartino LeonardoDemartino deleted the fix_macos_dylibsigning branch October 9, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[macOS] Godot 4.3 Dev Snapshot 6 (Mono) - macOS App from TestFlight not working
5 participants