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

Fix #1064 and LINKFLAGS typo for "-static" #1082

Closed
wants to merge 2 commits into from

Conversation

feiyunw
Copy link
Contributor

@feiyunw feiyunw commented Apr 2, 2023

This PR closes #1064, and #976 if it happens to MinGW only.
I suggest applying PR #1059 's patch before test this one.

Statically linked DLL will not depend on external MinGW DLLs (like libgcc_s_seh-1.dll, libstdc++-6.dll and libwinpthread-1.dll), thus it can be loaded by itself.

BTW, "--static" should be "-static".

FYI:
gcc: -static
ld: -static

@feiyunw feiyunw requested a review from a team as a code owner April 2, 2023 16:11
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.

Probably it should be an option (enabled by default), similar to the Godot build configs:

BoolVariable("use_static_cpp", "Link MinGW/MSVC C++ runtime libraries statically", True)

And also forced for MSVC build:

env.AppendUnique(CCFLAGS=["/MT"])

I'm not sure why we are setting -O3 for debug build, but since it's done for cross-build, it should be the same for native/msys build (or removed from both).

@bruvzg
Copy link
Member

bruvzg commented Apr 2, 2023

I suggest applying PR #1059 's patch before test this one.

A wrong link? This issue seems to be unrelated to static linking.

@feiyunw
Copy link
Contributor Author

feiyunw commented Apr 2, 2023

I suggest applying PR #1059 's patch before test this one.

A wrong link? This issue seems to be unrelated to static linking.

Unrelated to this issue, but tools\my_spawn.py has a (deadlock?) bug spoiling MinGW build.

("-O3" overridden and handled by Scons options "debug_symbols" and "optimize" in tools/targets.py)
@feiyunw
Copy link
Contributor Author

feiyunw commented Apr 8, 2023

I'm not sure why we are setting -O3 for debug build, but since it's done for cross-build, it should be the same for native/msys build (or removed from both).

-O3 is removed.
It is overridden and handled in tools/targets.py

@akien-mga akien-mga added bug This has been identified as a bug enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup labels Apr 11, 2023
@akien-mga akien-mga requested review from Faless and akien-mga April 11, 2023 09:01
@dsnopek
Copy link
Collaborator

dsnopek commented Jun 16, 2023

Discussed at the GDExtension meeting: overall, we think this makes sense, however, we think it should be an option that is enabled by default as suggested by @bruvzg in an earlier comment.

@dsnopek
Copy link
Collaborator

dsnopek commented Aug 6, 2023

I just tested this one and it fixed the problem for me! I think the only thing that still needs to be done here is adding the option that's enabled by default. If the OP doesn't have time to do this, I may pick it up some time this week.

@akien-mga
Copy link
Member

Superseded by #1203. Thanks for the contribution!

@akien-mga akien-mga closed this Aug 9, 2023
@feiyunw feiyunw deleted the issue-1064 branch December 13, 2023 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived bug This has been identified as a bug enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
5 participants