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.

Sign up for GitHub

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

Adds AppleClang + msvc preset #82

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+180 −10
Open

Adds AppleClang + msvc preset #82

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
03e11d1
update ci
wusatosi Nov 20, 2024
748d49b
add xcode-debug
wusatosi Nov 20, 2024
b062657
add xcode-release
wusatosi Nov 20, 2024
2bec3b3
add msvc
wusatosi Nov 20, 2024
069a47e
use setup msvc
wusatosi Nov 20, 2024
3a6abdb
Update CMakePresets.json
wusatosi Nov 25, 2024
9436310
bring back debug_base
wusatosi Dec 5, 2024
41f7186
adds /EHsc
wusatosi Dec 7, 2024
96a9e35
Apply suggestions from code review
wusatosi Dec 7, 2024
ad365b4
update _release-base
wusatosi Dec 11, 2024
b01929e
xcode->appleclang
wusatosi Dec 11, 2024
81f818b
fix capitialization
wusatosi Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions .github/workflows/ci_tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,37 @@ on:

jobs:
preset-test:
runs-on: ubuntu-latest
strategy:
matrix:
preset: ["gcc-debug", "gcc-release"]
name: "Preset: ${{ matrix.preset }}"
presets:
- preset: "gcc-debug"
platform: "ubuntu-latest"
- preset: "gcc-release"
platform: "ubuntu-latest"
- preset: "xcode-debug"
platform: "macos-latest"
- preset: "xcode-release"
platform: "macos-latest"
- preset: "msvc-debug"
platform: "windows-latest"
- preset: "msvc-release"
platform: "windows-latest"
name: "Preset: ${{ matrix.presets.preset }} on ${{ matrix.presets.platform }}"
runs-on: ${{ matrix.presets.platform }}
steps:
- uses: actions/checkout@v4
- name: Setup build environment
uses: lukka/get-cmake@latest
with:
cmakeVersion: "~3.25.0"
ninjaVersion: "^1.11.1"
- name: Setup MSVC
if: startsWith(matrix.presets.platform, 'windows')
uses: TheMrMilchmann/setup-msvc-dev@v3
with:
arch: x64
- name: Run preset
run: cmake --workflow --preset ${{ matrix.preset }}
run: cmake --workflow --preset ${{ matrix.presets.preset }}

gtest-test:
strategy:
Expand Down
162 changes: 150 additions & 12 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@
"CMAKE_CXX_STANDARD": "17"
}
},
{
"name": "_debug-base",
"hidden": true,
"cacheVariables": {
"CMAKE_BUILD_TYPE": "Debug",
"CMAKE_CXX_FLAGS": "-fsanitize=address -fsanitize=pointer-compare -fsanitize=pointer-subtract -fsanitize=leak -fsanitize=undefined"
}
},
{
wusatosi marked this conversation as resolved.
Show resolved Hide resolved
"name": "_release-base",
"hidden": true,
Expand All @@ -29,24 +21,66 @@
{
"name": "gcc-debug",
"displayName": "GCC Debug Build",
"inherits": ["_root-config"],
"cacheVariables": {
"CMAKE_CXX_COMPILER": "g++",
"CMAKE_BUILD_TYPE": "Debug",
"CMAKE_CXX_FLAGS": "-fsanitize=address -fsanitize=pointer-compare -fsanitize=pointer-subtract -fsanitize=leak -fsanitize=undefined"
}
},
{
"name": "gcc-release",
"displayName": "GCC Release Build",
"inherits": [
"_root-config",
"_debug-base"
"_release-base"
],
"cacheVariables": {
"CMAKE_CXX_COMPILER": "g++"
"CMAKE_CXX_COMPILER": "g++",
"CMAKE_BUILD_TYPE": "Debug",
"CMAKE_CXX_FLAGS": "-fsanitize=address -fsanitize=pointer-compare -fsanitize=pointer-subtract -fsanitize=leak -fsanitize=undefined"
wusatosi marked this conversation as resolved.
Show resolved Hide resolved
}
},
{
"name": "gcc-release",
"displayName": "GCC Release Build",
"name": "xcode-debug",
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

The name is sourced per your suggestion.
See: https://discourse.bemanproject.org/t/what-do-we-want-our-presets-to-be/246

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@wusatosi wusatosi Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I am simply implementing your initial proposal.

image

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not too too keen on gnu-debug... GCC is self descriptive enough and don't need to consult a chart...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'm with you there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member Author

Choose a reason for hiding this comment

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

hum, should I update msvc-debug to MSVC-debug, or should I use appleclang-debug?

"displayName": "xcode Debug Build",
"inherits": ["_root-config"],
"cacheVariables": {
"CMAKE_CXX_COMPILER": "g++",
wusatosi marked this conversation as resolved.
Show resolved Hide resolved
"CMAKE_BUILD_TYPE": "Debug",
"CMAKE_CXX_FLAGS": "-fsanitize=address -fsanitize=pointer-compare -fsanitize=pointer-subtract -fsanitize=undefined"
}
},
{
"name": "xcode-release",
"displayName": "xcode Release Build",
"inherits": [
"_root-config",
"_release-base"
],
"cacheVariables": {
"CMAKE_CXX_COMPILER": "g++"
wusatosi marked this conversation as resolved.
Show resolved Hide resolved
}
},
{
"name": "msvc-debug",
"displayName": "MSVC Debug Build",
"inherits": ["_root-config"],
"cacheVariables": {
"CMAKE_CXX_COMPILER": "cl",
"CMAKE_BUILD_TYPE": "Debug",
"CMAKE_CXX_FLAGS": "/fsanitize=address /Zi"
wusatosi marked this conversation as resolved.
Show resolved Hide resolved
}
},
{
"name": "msvc-release",
"displayName": "MSVC Release Build",
"inherits": ["_root-config"],
"cacheVariables": {
"CMAKE_CXX_COMPILER": "cl",
"CMAKE_BUILD_TYPE": "RelWithDebInfo",
"CMAKE_CXX_FLAGS": "/O2"
wusatosi marked this conversation as resolved.
Show resolved Hide resolved
}
}
],
"buildPresets": [
Expand All @@ -57,6 +91,22 @@
{
"name": "gcc-release",
"configurePreset": "gcc-release"
},
{
"name": "xcode-debug",
"configurePreset": "xcode-debug"
},
{
"name": "xcode-release",
"configurePreset": "xcode-release"
},
{
"name": "msvc-debug",
"configurePreset": "msvc-debug"
},
{
"name": "msvc-release",
"configurePreset": "msvc-release"
}
],
"testPresets": [
Expand All @@ -80,6 +130,26 @@
"name": "gcc-release",
"inherits": "_test_base",
"configurePreset": "gcc-release"
},
{
"name": "xcode-debug",
"inherits": "_test_base",
"configurePreset": "xcode-debug"
},
{
"name": "xcode-release",
"inherits": "_test_base",
"configurePreset": "xcode-release"
},
{
"name": "msvc-debug",
"inherits": "_test_base",
"configurePreset": "msvc-debug"
},
{
"name": "msvc-release",
"inherits": "_test_base",
"configurePreset": "msvc-release"
}
],
"workflowPresets": [
Expand Down Expand Up @@ -116,6 +186,74 @@
"name": "gcc-release"
}
]
},
{
"name": "xcode-debug",
"steps": [
{
"type": "configure",
"name": "xcode-debug"
},
{
"type": "build",
"name": "xcode-debug"
},
{
"type": "test",
"name": "xcode-debug"
}
]
},
{
"name": "xcode-release",
"steps": [
{
"type": "configure",
"name": "xcode-release"
},
{
"type": "build",
"name": "xcode-release"
},
{
"type": "test",
"name": "xcode-release"
}
]
},
{
"name": "msvc-debug",
"steps": [
{
"type": "configure",
"name": "msvc-debug"
},
{
"type": "build",
"name": "msvc-debug"
},
{
"type": "test",
"name": "msvc-debug"
}
]
},
{
"name": "msvc-release",
"steps": [
{
"type": "configure",
"name": "msvc-release"
},
{
"type": "build",
"name": "msvc-release"
},
{
"type": "test",
"name": "msvc-release"
}
]
}
]
}