-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
VS2022 Win10 build fix #5806
VS2022 Win10 build fix #5806
Conversation
vovodroid
commented
Jun 22, 2024
- use DEPS variable for both deps and slicer build
- use installed WinSDK version
- echo real cmake command line
- use DEPS variable for both deps and slicer build - use installed WinSDK version - echo real cmake command line
echo cmake .. -G "Visual Studio 17 2022" -A x64 -DBBL_RELEASE_TO_PUBLIC=1 -DCMAKE_PREFIX_PATH="%DEPS%/usr/local" -DCMAKE_INSTALL_PREFIX="./OrcaSlicer" -DCMAKE_BUILD_TYPE=%build_type% | ||
cmake .. -G "Visual Studio 17 2022" -A x64 -DBBL_RELEASE_TO_PUBLIC=1 -DCMAKE_PREFIX_PATH="%DEPS%/usr/local" -DCMAKE_INSTALL_PREFIX="./OrcaSlicer" -DCMAKE_BUILD_TYPE=%build_type% -DWIN10SDK_PATH="C:/Program Files (x86)/Windows Kits/10/Include/10.0.22000.0" | ||
echo on | ||
cmake .. -G "Visual Studio 17 2022" -A x64 -DBBL_RELEASE_TO_PUBLIC=1 -DCMAKE_PREFIX_PATH="%DEPS%/usr/local" -DCMAKE_INSTALL_PREFIX="./OrcaSlicer" -DCMAKE_BUILD_TYPE=%build_type% -DWIN10SDK_PATH="%WindowsSdkDir%Include\%WindowsSDKVersion%\" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the Windows SDK path/version being gotten from? If you are pulling it from the ENV variables set by CMake for the deps build, remember that the slicer can be build independently of the deps and vice versa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's from x64 Native tool window environment, so it's set before BAT file is run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only problem is if you don't run from that environment. Maybe check if the variables are initialized and if not set the current values as default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually build instruction clearly says Run build_release.bat in x64 Native Tools Command Prompt for VS 2019
(applied to 2022 as well I guess).
I understand the convenience having the command echo itself rather than having the command written out in the script twice, but I personally like the look of not having the user path before the command. I do have to admit that is just me being very nitpicky though 😄 |
It was included into echo before this PR, so I just eliminated copy-paste errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice change
Thank you
This breaks build on Github Actions: #6724 |