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

Some improvements for MinGW and LLVM build on Windows #94747

Merged
merged 5 commits into from
Aug 28, 2024

Conversation

alvinhochun
Copy link
Contributor

@alvinhochun alvinhochun commented Jul 25, 2024

This PR aims to improve the MinGW developer experience, especially for llvm-mingw on Windows:

  • Detect llvm-mingw - this toolchain provides GCC-like wrappers (e.g. x86_64-w64-mingw-gcc.exe) that helps using the toolchain as a drop-in replacement of GCC, but because the Godot build has some special logic for LLVM based on the use_llvm option, it will be better if the build script detects this situation and automatically set use_llvm=True.
  • Enable Clang colour output if supported on terminal (self explanatory)
  • Use thin archives - full archive files take a lot of space (1.3 GB total) compared to thin archives (57 MB total). (This is with llvm-mingw doing an editor dev build, other builds may vary.)
    • Something to note is that the --thin flag is only supported for llvm-ar since LLVM 14 from 2022 (binutils ar has had it for ages). Do we need to support older LLVM versions? Adding a check for this would be annoying though.
  • Print compiler warnings when building on Windows
  • ...and some minor changes.

@alvinhochun alvinhochun requested review from a team as code owners July 25, 2024 18:30
@alvinhochun alvinhochun force-pushed the mingw-llvm-build-flags branch from ed9f580 to 346a8b8 Compare July 25, 2024 19:44
@Calinou
Copy link
Member

Calinou commented Jul 25, 2024

Do we need to support older LLVM versions?

I'd say it's fine to require a recent (still 3 2 years old) LLVM version on Windows, as it's easy to install a recent version there (unlike Linux distributions where this can require third-party repositories).

@alvinhochun
Copy link
Contributor Author

Oh sorry, LLVM 14 was actually early 2022. I mis-wrote.

Regardless, trying to use a distro build of LLVM together with a distro build of mingw-w64 runtime to cross-compile is not quite straightforward, so I honestly would not expect many people (if any at all) to do it with Godot. (Not to mention that GCC is still the usual compiler people use to target MinGW.)

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected (Windows 11 23H2, LLVM 17.0.6 from https://github.com/mstorsjo/llvm-mingw/releases/tag/20231128). The resulting binary runs correctly.

@alvinhochun alvinhochun force-pushed the mingw-llvm-build-flags branch from 346a8b8 to 38e47f5 Compare August 28, 2024 09:24
@akien-mga akien-mga requested a review from bruvzg August 28, 2024 09:39
methods.py Outdated
# got built correctly regardless the invocation strategy.
# Furthermore, since SCons will rebuild the library from scratch when an object file
# changes, no multiple versions of the same object file will be present.
self.Replace(ARFLAGS="q")
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how this is related to PR and why it's removed., nothing seems to fix the mentioned issue, so it's likely still relevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, let me remove this commit from the PR. I will check again some time and handle it in another PR.

@alvinhochun alvinhochun force-pushed the mingw-llvm-build-flags branch from 38e47f5 to c7a7a5a Compare August 28, 2024 10:20
@alvinhochun alvinhochun requested a review from bruvzg August 28, 2024 10:21
@akien-mga akien-mga merged commit 2730d70 into godotengine:master Aug 28, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

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 participants