Adds appleclang + msvc preset#82
Conversation
wusatosi
requested review from
bretbrownjr,
camio,
dietmarkuehl,
neatudarius and
steve-downey
as code owners
camio
left a comment
camio
left a comment
There was a problem hiding this comment.
Some minor tweaks suggested. I tested this on a MacOS and Windows system directly and it worked well.
| { | ||
| "name": "gcc-release", | ||
| "displayName": "GCC Release Build", | ||
| "name": "xcode-debug", |
There was a problem hiding this comment.
XCode is an IDE, not a compiler. Unless your intent is to use the Xcode generator, this should be called something like apple_clang-debug since it targets Apple's clang fork
If there are no differences between supporting apple clang and stock clang then maybe "clang-debug" is a good name.
There was a problem hiding this comment.
The name is sourced per your suggestion.
See: https://discourse.bemanproject.org/t/what-do-we-want-our-presets-to-be/246
There was a problem hiding this comment.
There is a reason to separate apple clang out in respect to clang-clang, it is to prevent scenaior like: #65 .
The main pain-point is sanitizers.
There was a problem hiding this comment.
I am simply implementing your initial proposal.
I take that your initial proposal is to support xcode based toolchain. Apple-clang is just part of it (we will likly add whatever the etc part is going to be in the future).
The intention here is not to soly support apple clang but extra configurations that might arise from xcode based toolchains.
Let me know if I am interpreting your initial proposal incorrectly.
Your proposal is linked above as well.
There was a problem hiding this comment.
Ah, I see the confusion now. My proposal was suggesting that the Generator be "XCode" for the MacOS platform and that the Generator be "Visual Studio 2022" for the Windows platform.
What you've done here is use "Ninja" as the generator for these platforms. I think that's fine for now.
There was a problem hiding this comment.
AppleClang-debug?
Yep, that sounds good.
For compiler names we could potentially follow the convention here. That implies we would change gcc-release to GNU-release which may be surprising for those not familiar with that list, but at least we'd have something to reference.
There was a problem hiding this comment.
I am not too too keen on gnu-debug... GCC is self descriptive enough and don't need to consult a chart...
There was a problem hiding this comment.
hum, should I update msvc-debug to MSVC-debug, or should I use appleclang-debug?
camio
left a comment
camio
left a comment
There was a problem hiding this comment.
CMAKE_CXX_FLAGS should be removed from _release-base per prior discussion.
Wait presets are for end users. Any upstream that consumes our library as a dependency wouldn't get any If the idea for preset is that it is a substitute for the generic setup script of: cmake -S . -B build -DCMAKE_CXX_STANDARD=20
cmake --build build
ctest --test-dir buildLeaving c++ version undefined will let to varying development environment. It would like saying the build command for the project is: cmake -S . -B build
cmake --build build
ctest --test-dir buildThis may lead to tests that are disabled for some while enabled for others. |
Co-authored-by: David Sankel <[email protected]>
Oops, I intended to say they should be moved from |
Okay this make sense 👌 |
Done |
camio
left a comment
camio
left a comment
There was a problem hiding this comment.
One minor suggestion. Otherwise looks good to me.
Co-authored-by: David Sankel <[email protected]>
This reverts commit 81f818b.
4 participants
Added preset in accordance to discourse topics:
Appropriate CI testings are added.
closes #65