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

chore(msvc): add msvc cmake support #667

Merged
merged 3 commits into from
Dec 19, 2022

Conversation

nitz
Copy link
Contributor

@nitz nitz commented Dec 17, 2022

Just two small changes and some CMakeLists.txt additions is all it took to build through Visual Studio 2022.

C&C welcome!

Proposed changes

  • Initializes a "potentially uninitialized" pointer that msvc whines about due to /sdl.
  • Replaces a VLA with a QVarLengthArray since msvc doesn't support all of C99.
  • Adds a check for msvc to CMakeLists.txt, and sets compiler flags that closely match the existing gcc/clang counterparts.
  • Adds a few lines to BUILDING.md to elaborate on how to use Visual Studio's built-in cmake support to compile the project.

A screenshot quite unlikely needed for this, but I took one anyways 😂

image

Fixes the msvc warning as error due to /sdl.Fixes the msvc warning as error due to /sdl.Fixes the msvc warning as error due to /sdl.Fixes the msvc warning as error due to /sdl.

Fixes the msvc warning as error due to `/sdl`.
@nitz nitz changed the title chore(msvc): fix potentially uninitialized warning chore(msvc): add msvc cmake support Dec 17, 2022
@pktiuk
Copy link
Member

pktiuk commented Dec 18, 2022

Everything looks good so far. proper MSVC support may be very helpful for developers using Windows.
I have 2 comments:

  • I think git history would look better without this style commit
  • It would be good to add Compilation via msvc to CI to avoid breaking this support by accident

BTW
When you will think your code will be ready to merge press Ready for review.

msvc doesn't conform to c99, so it misses out on variable length arrays. Qt has the answer.

style: missed clang format on the includes
Also add notes to BUILDING.md about how to build under Visual Studio.
@nitz nitz force-pushed the chore-add-msvc-cmake-support branch from 4628f04 to 05ba132 Compare December 19, 2022 12:42
@nitz
Copy link
Contributor Author

nitz commented Dec 19, 2022

I'll go ahead and ready this one up while I look into the cleanest way to add msvc ci. I've set it up several times for .NET projects but not yet done one for C++, or one that uses Qt. (Though the jurplel/install-qt-action@v2 you use looks like it should make that pretty easy!)

@nitz nitz marked this pull request as ready for review December 19, 2022 12:51
@pktiuk pktiuk merged commit 53a0e08 into AntiMicroX:master Dec 19, 2022
@pktiuk
Copy link
Member

pktiuk commented Dec 19, 2022

Great.
I merged these changes now.
I am waiting for another PR with GitHub Action :)

@nitz
Copy link
Contributor Author

nitz commented Dec 19, 2022

Fantastic! This looks like an excellent thing to work on now too, because my brain is not otherwise having monday morning hahah

@nitz nitz deleted the chore-add-msvc-cmake-support branch December 19, 2022 13:42
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.

2 participants