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

VSProj Configure type on build command - to resolve #1582 #1600

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

enetheru
Copy link
Contributor

@enetheru enetheru commented Sep 21, 2024

Visual Studio projects are multi-config projects like Ninja-MultiConfig which means you can't set the configuration at configure time as there are multiple, it always chooses the first one by default when not specified in the build command.

Instead of this:
cmake -DCMAKE_BUILD_TYPE=Release -G"Visual Studio 17 2022" .
cmake --build . --verbose

It should be this
cmake -G"Visual Studio 17 2022" .
cmake --build . --verbose --config Release

Because the existing cmake solution does not use generator expressions for its per config configuration, we actually have to specify it in both commands :(

cmake -DCMAKE_BUILD_TYPE=Release -G"Visual Studio 17 2022" .
cmake --build . --verbose --config Release

Fixes #1582

@enetheru enetheru requested a review from a team as a code owner September 21, 2024 10:20
@enetheru
Copy link
Contributor Author

I know why its failing.

# Default build type is Debug in the SConstruct
if("${CMAKE_BUILD_TYPE}" STREQUAL "")
	set(CMAKE_BUILD_TYPE Debug)
endif()

I'll fix the commit so it adds only the --config to the build command so its correct in both places.

@enetheru
Copy link
Contributor Author

I'm in the middle of compiling right now or i would squash these two commits.

Visual Studio projects are multi-config projects like Ninja-MultiConfig which means you can't set the configuration at configure time as there are multiple, it always chooses the first one by default when not specified in the build command.

Instead of this:
cmake -DCMAKE_BUILD_TYPE=Release -G"Visual Studio 17 2022" .
cmake --build . --verbose

It should be this
cmake -G"Visual Studio 17 2022" .
cmake --build . --verbose --config Release

Update ci.yml

Because the current build system doesnt use generator expressions for multi config builds, both the CMAKE_BUILD_TYPE and the build --config options need to be set
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks!

@dsnopek dsnopek merged commit 96565e1 into godotengine:master Sep 26, 2024
12 checks passed
@dsnopek dsnopek added bug This has been identified as a bug cmake topic:buildsystem Related to the buildsystem or CI setup labels Sep 26, 2024
@dsnopek dsnopek added this to the 4.x milestone Sep 26, 2024
@dsnopek
Copy link
Collaborator

dsnopek commented Oct 28, 2024

Cherry-picked for 4.2 in PR #1631

@dsnopek
Copy link
Collaborator

dsnopek commented Oct 28, 2024

Cherry-picked for 4.3 in PR #1632

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug cmake topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible bug with CMake and visual studio release type
3 participants