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

Add compatibility-only suffix to env, denoting what environment the binary is compatible with. #1638

Closed
wants to merge 1 commit into from

Conversation

Ivorforce
Copy link
Contributor

@Ivorforce Ivorforce commented Nov 8, 2024

I use this scheme in my own GDExtension - I want my object to be named not by how it's optimized (e.g. template_debug vs template_release, or if it's a dev build), but just by what it's compatible with.

This PR should change no behavior for existing builds, but it does add the option for custom SConstructs to use env["compatibility_suffix"] instead of env["suffix] for naming binaries.

Here's what the test .gdextension entries would look like adopting this scheme instead (just for illustrative purposes):

macos = "res://bin/libgdexample.macos.framework"
windows.x86_32 = "res://bin/libgdexample.windows.x86_32.dll"
windows.x86_64 = "res://bin/libgdexample.windows.x86_64.dll"
windows.arm64 = "res://bin/libgdexample.windows.arm64.dll"
linux.x86_32 = "res://bin/libgdexample.linux.x86_32.so"
linux.x86_64 = "res://bin/libgdexample.linux.x86_64.so"
linux.arm32 = "res://bin/libgdexample.linux.arm32.so"
linux.arm64 = "res://bin/libgdexample.linux.arm64.so"
linux.rv64 = "res://bin/libgdexample.linux.rv64.so"
android.x86_64 = "res://bin/libgdexample.android.x86_64.so"
android.arm64 = "res://bin/libgdexample.android.arm64.so"
ios = "res://bin/libgdexample.ios.xcframework"
web.wasm32 = "res://bin/libgdexample.web.wasm32.wasm"

(note that macos and iOS happen to have no change right now, due to not using env["suffix"] anyway, but it would use the same name even if it used env["compatibility_suffix"])

@Ivorforce Ivorforce requested a review from a team as a code owner November 8, 2024 11:25
@Ivorforce Ivorforce changed the title Add compatibility-only suffix, denoting what environment the binary is compatible with. Add compatibility-only suffix to env, denoting what environment the binary is compatible with. Nov 8, 2024
@Faless
Copy link
Contributor

Faless commented Nov 8, 2024

The target is part of the compatibility (i.e. template_release is not compatible with engine template_debug builds, and vice versa, although the editor and template_debug are compatible with each other).

Likewise, threads/nothreads builds are incompatible with nothreads/threads web build (the main case where the nothreads build makes sense).

Similarly, simulator builds, are not compatible with actual devices (and non-simulator builds, are not compatible with the simulator).

@Ivorforce
Copy link
Contributor Author

Ivorforce commented Nov 8, 2024

The target is part of the compatibility (i.e. template_release is not compatible with engine template_debug builds, and vice versa, although the editor and template_debug are compatible with each other).

Ok, good to know. I got tricked by the editor being compatible with template_debug builds and assumed it's just for setting linking flags etc.

Similarly, simulator builds, are not compatible with actual devices (and non-simulator builds, are not compatible with the simulator).

I looked through the source code, and setting the flag only seems to pass x86_64 to the target arches additionally. Correct me if I'm wrong.

But all in all, this changes things, obviously, simply making suffix what I need for my own builds. The godot-cpp-template .gdextension does not currently honor all these parameters, so we should adjust them. This issue seems even more related now: godotengine/godot-cpp-template#62

I'm closing this PR to tackle it from the other side instead. Thanks for clearing things up!

@Ivorforce Ivorforce closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants