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.

Sign up for GitHub

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

Jump to bottom

Align and fix some Windows MSVC build setting #71089

Merged
merged 21 commits into from
Jan 24, 2024
Merged

Align and fix some Windows MSVC build setting #71089

merged 21 commits into from
Jan 24, 2024

Conversation

alef
Copy link
Contributor

@alef alef commented Jan 19, 2024 •
edited
Loading

Summary

Build "Align and fix some Windows MSVC build setting"

Purpose of change

Mostly align CMake linker settings, see below.

Describe the solution

  • Drop that stupid hack of mine to force Ninja Build on Windows Terminal
  • Keep msvc-full-features Solution file flags aligned into CMake's
  • Merge Windows manifests for utf-8 and UAC settings
  • Avoid CMake's MVSC build failing for duplicated resource ID
  • Edit: rework MSYS2/MINGW64 static linking in the process

Describe alternatives you've considered

Testing

Built with Solution file and CMake presets. The icon is there, the .exe contains the manifest. It runs. NOT tested the msys2/mingw build.

Additional context

I am not sure the UAC section and UTF-8 thing can coexist together this way.
I hope @Qrox and @akrieger review this.

@github-actions github-actions bot added Code: Build Issues regarding different builds and build environments [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jan 19, 2024
@alef alef marked this pull request as ready for review January 19, 2024 22:17
Qrox
Qrox reviewed Jan 20, 2024
View reviewed changes
src/resource.rc Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. Code: Tests Measurement, self-control, statistics, balancing. [Markdown] Markdown issues and PRs Code: Tooling Tooling that is not part of the main game but is part of the repo. labels Jan 20, 2024
github-actions[bot]
github-actions bot reviewed Jan 20, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Auto-requesting reviews from non-collaborators: @jbytheway

Qrox
Qrox reviewed Jan 21, 2024
View reviewed changes
src/resource.rc Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 21, 2024
akrieger
akrieger reviewed Jan 21, 2024
View reviewed changes
Copy link
Member

@akrieger akrieger left a comment

Choose a reason for hiding this comment

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

What do we need the raised minimum cmake version for?

build-scripts/show-manifest Outdated Show resolved Hide resolved
build-scripts/show-manifest Outdated Show resolved Hide resolved
data/application_manifest.xml Show resolved Hide resolved
@alef
Copy link
Contributor Author

alef commented Jan 21, 2024

What do we need the raised minimum cmake version for?

Because of this message:

CMake Deprecation Warning at src/third-party/CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.5 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.

@alef
Copy link
Contributor Author

alef commented Jan 21, 2024 •
edited
Loading

92b1caa not true. Still left out are:

  • libavif-16.dll
  • libharfbuzz-0.dll
  • libhwy.dll
  • libogg-0.dll

Qrox
Qrox reviewed Jan 22, 2024
View reviewed changes
CMakeLists.txt Show resolved Hide resolved
alef added 12 commits January 23, 2024 18:37
@alef
Drop VsDevCmd.cmake hack because we use MSBuild
1e4a7c5 
This stop overwriting CMakeUserPresets.json too.
@alef
Fix duplicated resource ID when build w/ CMake
1150975 
Link.exe tries add both our application_manifest.xml and it's
UAC thing resulting in conflicts. Probably it's CMake injecting
`/manifest:ebmed`, because the Solution file doesn't.
@alef
Adding manually the UAC section to the manifest
1f3fbed
@alef
Check for multi-config generator before set default build type
44f29c3 
Build type should be specified at `cmake --build --config Debug`
or generator level, like `.. -- -p:Configuration=Debug` (MSBuild)
@alef
Add MSVC warning wd4661 as in msvc-full-features/
cc3dec3
@alef
Obey cmake-lint
f64a75c
@alef
Detect running under MSYS and MINGW
24e3a7a
@alef
Add a CMake preset for MSYS2 + MinGW64 build
752520c
@alef
Upgrade CMake minimum required version everywhere
d9e45db
@alef
No need to create directories with the same git clone name
0ffb52b
@alef
Statically link all MSYS2/MINGW64 libraries
267ab94
@alef
Fix and update MSYS2 compiling document
700a12c
@alef
Copy link
Contributor Author

alef commented Jan 23, 2024

rebased

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 23, 2024
@Maleclypse Maleclypse merged commit c374e1a into CleverRaven:master Jan 24, 2024
23 of 28 checks passed
@alef alef deleted the link_exe_things branch January 24, 2024 16:22
@ZhilkinSerg ZhilkinSerg mentioned this pull request Mar 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@Qrox Qrox Qrox left review comments

@akrieger akrieger akrieger left review comments

Assignees
No one assigned
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Build Issues regarding different builds and build environments Code: Tests Measurement, self-control, statistics, balancing. Code: Tooling Tooling that is not part of the main game but is part of the repo. <Documentation> Design documents, internal info, guides and help. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alef @akrieger @Qrox @Maleclypse