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

Added missing cmakeToolchain path setting to windows_buildenv.bat #4709

Merged

Conversation

JoergAtGithub
Copy link
Member

Without this setting the CMake cache is uneccessary often invalidated, which requires complete recompile. With this setting switching between different branches and build configurations doesn't require a full rebuild in most cases.

… of the CMake cache, in case of repeated switching between build types.
@ronso0
Copy link
Member

ronso0 commented Mar 29, 2022

just a side note: you can keep the commit message subject short (use PR title, ~50 chars max recommended) and put the detailed description into the message body (wrapping at 70-75, recommendations vary ; )

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

now cmakeToolchain is passed as -DCMAKE_TOOLCHAIN_FILE to cmake.
Howerver it was already set here:

set(CMAKE_TOOLCHAIN_FILE "${VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" CACHE STRING "")
if VCPKG_ROOT is set which should be the case.

Please explain how this PR makes a difference and how it prevents rebuilding.

@JoergAtGithub
Copy link
Member Author

Indeed, this looks like the same to me. Somehow my addition helped - currently I can't explain this.

@daschuer
Copy link
Member

daschuer commented Apr 4, 2022

Can you double check it, before merge?

@JoergAtGithub JoergAtGithub marked this pull request as draft April 4, 2022 18:54
@JoergAtGithub JoergAtGithub marked this pull request as ready for review June 12, 2022 11:16
@JoergAtGithub
Copy link
Member Author

I checked this again, while testing the different built targets with the new builenv. It doesn't work without this setting in CMakeSettings.json. The CMake cache is always invalidated, when I change the target.
I can't explain, why the setting in CMakelists.txt doesn't work here too, but the result is reproduceable.

@daschuer
Copy link
Member

OK, It is still weird. But passing the CMAKE_TOOLCHAIN_FILE via CMakeSetting.json is the standard, so let's go for it.

@daschuer daschuer merged commit 326ea88 into mixxxdj:main Jun 12, 2022
@JoergAtGithub
Copy link
Member Author

Thanks!

@JoergAtGithub JoergAtGithub deleted the windows_buildenv_cmakeToolchain_setting branch June 12, 2022 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants