Update the logic to sign prebuilt Godot Android apks #44645

m4gr3d · 2020-12-24T06:59:43Z

The previously used tool, jarsigner has been deprecated in favor of apksigner which is bundled with the Android SDK.
The logic is refactored accordingly and a few editor settings have been deprecated in the process as they're no longer necessary.

Note: As a side effect, specifying the Android SDK path is now required. The docs will be updated to reflect that change.

Fixes #44587

3.2 backport

m4gr3d · 2020-12-24T07:02:20Z

@godotengine/android This is being opened as a draft as the PR (especially the commands' names and extensions) still needs to be validated on Linux and Windows machines (I only have a mac with me at the moment).

In addition, a couple Android editor settings are being deprecated for which I'd like feedback on.

mbrlabs · 2020-12-24T11:38:21Z

I tested this on Windows 10 and in all my build-tools versions (25-30) apksigner is a Java program (jar file) with a Windows batch script called apksigner.bat. Since this looks for an .exe it can't find the tool.

mbrlabs · 2020-12-24T11:40:25Z

platform/android/export/export.cpp

+	static String get_apksigner_path() {
+		String exe_ext = "";
+		if (OS::get_singleton()->get_name() == "Windows") {
+			exe_ext = ".exe";


That should be changed to: exe_ext = ".bat";

Isn't the executable extension optional on Windows thanks to its PATHEXT environment variable?

Yeah, but what this code does is actually try to find the tool, so you need the whole path incl. file extension.

@Calinou there were a few bugs not too long ago where the lack of the proper extension caused issues on Windows machines (see #43671). Do you know under what conditions PATHEXT takes effect?

@mbrlabs Thanks for checking! I've updated the extension to .bat. Can you take another look.

@m4gr3d Yes, that fixed it. I also exported an APK & did a quick install via the editor -> works.

mbrlabs · 2020-12-24T16:08:47Z

Not sure if this is related since this works now on Windows, but after quick-installing to an Android device i get this error in the Godot console:

editor\progress_dialog.cpp:231 - Condition "!tasks.has(p_task)" is true.
Installing to device (please wait...): Samsung SM-G973F

m4gr3d · 2020-12-25T19:46:45Z

Not sure if this is related since this works now on Windows, but after quick-installing to an Android device i get this error in the Godot console:
editor\progress_dialog.cpp:231 - Condition "!tasks.has(p_task)" is true.
Installing to device (please wait...): Samsung SM-G973F

@mbrlabs It was an unrelated bug, but this PR surfaced it. I've fixed the issue so the error should be gone now.

m4gr3d · 2020-12-25T19:51:14Z

@pouleyKetchoupp @RandomShaper Any chance you can validate this PR (and #44646) on a Linux machine?

akien-mga · 2020-12-27T19:26:58Z

Note: As a side effect, specifying the Android SDK path is now required. The docs will be updated to reflect that change.

That's annoying :( But I guess there's no other way forward.
And at least that removes the requirement on getting a somewhat compatible version of the JDK installed manually to get jarsigner.

akien-mga · 2020-12-29T11:51:56Z

platform/android/export/export.cpp

+		// There are additional versions directories we need to go through.
+		da->list_dir_begin();
+		String sub_dir = da->get_next();
+		while (!sub_dir.empty()) {
+			if (!sub_dir.begins_with(".") && da->current_is_dir()) {
+				// Check if the tool is here.
+				String tool_path = build_tools_dir.plus_file(sub_dir).plus_file(apksigner_command_name);
+				if (FileAccess::exists(tool_path)) {
+					apksigner_path = tool_path;
+					break;
+				}
+			}
+			sub_dir = da->get_next();
+		}
+		da->list_dir_end();


How confident are we that the SDK structure will stay the same? Parts of it have changed every now and then causing breakage in build scripts, and it would be annoying to have users unable to export APKs because apksigner moved someplace else and our hardcoded detection logic can't find it.

That directory structure has been stable for the past 4 years (since revision 24.0.3), so I don't expect it to change soon.

Nonetheless, we can add a setting entry specific to the apksigner command, and only use the hardcoded logic above as a fallback in case the setting is not specified.
As we move more toward AAB and custom build, that setting will be less and less relevant (since we leverage the gradle build process instead for these).

I guess let's go without extra setting for now and see how it evolves.

akien-mga · 2020-12-30T08:14:25Z

Needs a rebase.

akien-mga · 2021-01-01T21:30:37Z

String::empty() was renamed to String::is_empty(), so this needs to be changed here (not in the 3.2 PR).

The previously used tool, `jarsigner` has been deprecated in favor of `apksigner` which is bundled with the Android SDK. The logic is refactored accordingly and a few editor settings have been deprecated in the process as they're no longer necessary. Note: As a side effect, specifying the Android SDK path is now required. The docs will be updated to reflect that change.

akien-mga · 2021-01-01T21:58:14Z

Thanks!

m4gr3d added bug platform:android topic:buildsystem needs testing labels Dec 24, 2020

m4gr3d added this to the 4.0 milestone Dec 24, 2020

m4gr3d requested a review from a team December 24, 2020 06:59

m4gr3d mentioned this pull request Dec 24, 2020

[3.2] Update the logic to sign prebuilt Godot Android apks #44646

Merged

mbrlabs reviewed Dec 24, 2020

View reviewed changes

m4gr3d force-pushed the update_apk_signing_logic branch from a69d0fe to 95ff39f Compare December 24, 2020 15:36

m4gr3d force-pushed the update_apk_signing_logic branch from 95ff39f to ab08e97 Compare December 25, 2020 19:44

RandomShaper approved these changes Dec 25, 2020

View reviewed changes

pouleyKetchoupp self-requested a review December 26, 2020 02:28

m4gr3d marked this pull request as ready for review December 26, 2020 18:00

akien-mga reviewed Dec 29, 2020

View reviewed changes

akien-mga approved these changes Dec 30, 2020

View reviewed changes

m4gr3d force-pushed the update_apk_signing_logic branch from ab08e97 to b91968f Compare January 1, 2021 21:20

m4gr3d force-pushed the update_apk_signing_logic branch from b91968f to 0b12484 Compare January 1, 2021 21:31

m4gr3d force-pushed the update_apk_signing_logic branch from 0b12484 to 49969de Compare January 1, 2021 21:40

akien-mga merged commit 3433780 into godotengine:master Jan 1, 2021

m4gr3d deleted the update_apk_signing_logic branch January 1, 2021 22:23

akien-mga mentioned this pull request Jan 4, 2021

Updates and changes for 3.2.4 godot-sdk-integrations/godot-google-play-billing#21

Draft

m4gr3d mentioned this pull request Jan 5, 2021

Update instructions to setup Exporting for Android godotengine/godot-docs#4519

Merged

madmiraal mentioned this pull request Jan 9, 2021

Split OS::execute into two methods #44514

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update the logic to sign prebuilt Godot Android apks #44645

Update the logic to sign prebuilt Godot Android apks #44645

m4gr3d commented Dec 24, 2020 •

edited

Loading

m4gr3d commented Dec 24, 2020

mbrlabs commented Dec 24, 2020

mbrlabs Dec 24, 2020

Calinou Dec 24, 2020 •

edited

Loading

mbrlabs Dec 24, 2020

m4gr3d Dec 24, 2020

mbrlabs Dec 24, 2020 •

edited

Loading

mbrlabs commented Dec 24, 2020

m4gr3d commented Dec 25, 2020

m4gr3d commented Dec 25, 2020

akien-mga commented Dec 27, 2020

akien-mga Dec 29, 2020

m4gr3d Dec 29, 2020

akien-mga Dec 30, 2020

akien-mga commented Dec 30, 2020

akien-mga commented Jan 1, 2021 •

edited

Loading

akien-mga commented Jan 1, 2021

Update the logic to sign prebuilt Godot Android apks #44645

Update the logic to sign prebuilt Godot Android apks #44645

Conversation

m4gr3d commented Dec 24, 2020 • edited Loading

m4gr3d commented Dec 24, 2020

mbrlabs commented Dec 24, 2020

mbrlabs Dec 24, 2020

Choose a reason for hiding this comment

Calinou Dec 24, 2020 • edited Loading

Choose a reason for hiding this comment

mbrlabs Dec 24, 2020

Choose a reason for hiding this comment

m4gr3d Dec 24, 2020

Choose a reason for hiding this comment

mbrlabs Dec 24, 2020 • edited Loading

Choose a reason for hiding this comment

mbrlabs commented Dec 24, 2020

m4gr3d commented Dec 25, 2020

m4gr3d commented Dec 25, 2020

akien-mga commented Dec 27, 2020

akien-mga Dec 29, 2020

Choose a reason for hiding this comment

m4gr3d Dec 29, 2020

Choose a reason for hiding this comment

akien-mga Dec 30, 2020

Choose a reason for hiding this comment

akien-mga commented Dec 30, 2020

akien-mga commented Jan 1, 2021 • edited Loading

akien-mga commented Jan 1, 2021

m4gr3d commented Dec 24, 2020 •

edited

Loading

Calinou Dec 24, 2020 •

edited

Loading

mbrlabs Dec 24, 2020 •

edited

Loading

akien-mga commented Jan 1, 2021 •

edited

Loading