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

C#: Validate project TFM for Android template exports #102627

Merged

Conversation

raulsntos
Copy link
Member

@raulsntos raulsntos commented Feb 9, 2025

The C# Android export template includes .jar dependencies from .NET 8.0, so other TFMs are not supported.

The error messages assume #98066 will be merged so this won't be a problem for Gradle builds.


I don't like that we keep introducing more .NET-specific stuff on the Android platform. Maybe we can expose some virtual method on EditorExportPlugin to do this validation on the plugin's side, then it could be implemented entirely on ExportPlugin.cs, but this can be postponed to 4.5+.

Bugsquad edit:

print_line(pipe);
WARN_PRINT("dotnet command exited with code " + itos(exitcode) + ". See output above for more details.");
}
r_error += TTR("Unable to determine the C# project's TFM, it may be incompatible. The export template only supports '" + required_tfm + "'. Make sure the project targets '" + required_tfm + "' or consider using gradle builds instead.") + "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
r_error += TTR("Unable to determine the C# project's TFM, it may be incompatible. The export template only supports '" + required_tfm + "'. Make sure the project targets '" + required_tfm + "' or consider using gradle builds instead.") + "\n";
r_error += vformat(TTR("Unable to determine the C# project's TFM, it may be incompatible. The export template only supports '%s'. Make sure the project targets '%s' or consider using gradle builds instead."), required_tfm, required_tfm) + "\n";

Afaik, the translation won't work otherwise I think

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, TTR() strings are extracted as is by a Python parser, so they need to be literals.

} else {
String tfm = pipe.strip_edges();
if (tfm != required_tfm) {
r_error += TTR("C# project targets '" + tfm + "' but the export template only supports '" + required_tfm + "'. Consider using gradle builds instead.") + "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
r_error += TTR("C# project targets '" + tfm + "' but the export template only supports '" + required_tfm + "'. Consider using gradle builds instead.") + "\n";
r_error += vformat(TTR("C# project targets '%s' but the export template only supports '%s'. Consider using gradle builds instead."), tfm, required_tfm) + "\n";

@akien-mga
Copy link
Member

With this change and #98066, should we consider that #101948 gets fixed?

The C# Android export template includes `.jar` dependencies from .NET 8.0, so other TFMs are not supported.
@raulsntos raulsntos force-pushed the dotnet/android-export-validate-tfm branch from f75fee5 to 70ff213 Compare February 10, 2025 16:19
@raulsntos
Copy link
Member Author

With this change and #98066, should we consider that #101948 gets fixed?

Yes. #98066 fixes it for Gradle builds and this PR adds an error that should block anything other than net8.0 for the non-Gradle builds.

@Repiteo Repiteo merged commit 0b9fd7e into godotengine:master Feb 10, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Feb 10, 2025

Thanks!

@raulsntos raulsntos deleted the dotnet/android-export-validate-tfm branch February 10, 2025 19:26
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.

(4.4.beta1) Android app crashes when exported with .NET 9
5 participants