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

Rename macOS and windows binaries to fit the OSs better in SConstruct #55

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Ivorforce
Copy link
Contributor

@Ivorforce Ivorforce commented Oct 3, 2024

The primary change is in SConstruct:

"{}{}{}{}".format(env.subst("$SHLIBPREFIX"), libname, env["suffix"], env.subst("$SHLIBSUFFIX"))

This names binaries as they should be named depending on OS and godot tags. This happens to rename only windows and macOS:

  • windows drops the lib prefix
  • macOS adds a .universal.dylib suffix

Another change regards the macos.framework folder, which is now named EXTENSION-NAME-macos.framework. It is common practice to name the .framework folder after the actual library name. I'm keeping the -macos because, unlike other libraries, we have different binaries for different platforms.

Finally, using Install copies the original file name, allowing us to avoid passing the name again manually and stay consistent.

Originally part of #49

… the library name in the framework folder name. This renames the mac and windows binaries.
demo/bin/example.gdextension Outdated Show resolved Hide resolved
demo/bin/example.gdextension Outdated Show resolved Hide resolved
…ctures for macOS and iOS for App Store purposes.
@Ivorforce Ivorforce requested a review from bruvzg October 4, 2024 16:07
@Ivorforce
Copy link
Contributor Author

Ivorforce commented Oct 4, 2024

It is notable that a rather annoying bug currently exists that makes the .framework approach rather unattractive: godotengine/godot#96403 (edit: though notably in 4.3 only, not 4.4 dev)

I've narrowed it down to this exact cause, but it's difficult for me to pinpoint where exactly in the code the error occurs, so far.

@fire
Copy link
Member

fire commented Nov 8, 2024

As you sure? I remember the requirement on windows to link by abs path and linkflags

@Ivorforce
Copy link
Contributor Author

Ivorforce commented Nov 8, 2024

As you sure? I remember the requirement on windows to link by abs path and linkflags

You mean regarding my .framework problem? It may have to do with the fact that I don't sign the frameworks yet. Or that i've been using a template_release build for the editor, which apparently expects template_release? I'll have to do some more testing before coming to final conclusions.

Either way for this PR i'm using the recommended .framework approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants