-
Notifications
You must be signed in to change notification settings - Fork 22
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
Introduce MSVC CI support #80
Changes from 6 commits
2bf94fa
37fe7ce
625f140
1bd9055
8c4c8a4
47cc829
45e1005
01bf599
ac89671
2230b3e
7f774ab
e8f6417
2286fa6
84a7177
1a3c911
fe6fc45
bdb06fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -31,12 +31,15 @@ jobs: | |||||||||
strategy: | ||||||||||
fail-fast: false | ||||||||||
matrix: | ||||||||||
platform: [ubuntu-latest] | ||||||||||
compiler: | ||||||||||
- cpp: g++ | ||||||||||
platform: | ||||||||||
- description: "ubuntu gcc" | ||||||||||
cpp: g++ | ||||||||||
c: gcc | ||||||||||
neatudarius marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
- cpp: clang++ | ||||||||||
os: ubuntu-latest | ||||||||||
- description: "ubuntu clang" | ||||||||||
cpp: clang++ | ||||||||||
c: clang | ||||||||||
os: ubuntu-latest | ||||||||||
cpp_version: [17, 20, 23, 26] | ||||||||||
cmake_args: | ||||||||||
- description: "Default" | ||||||||||
|
@@ -46,59 +49,92 @@ jobs: | |||||||||
- description: "ASan" | ||||||||||
args: "-DCMAKE_CXX_FLAGS='-fsanitize=address -fsanitize=undefined'" | ||||||||||
include: | ||||||||||
- platform: ubuntu-latest | ||||||||||
compiler: | ||||||||||
- platform: | ||||||||||
description: "ubuntu gcc" | ||||||||||
cpp: g++ | ||||||||||
c: gcc | ||||||||||
os: ubuntu-latest | ||||||||||
cpp_version: 17 | ||||||||||
cmake_args: | ||||||||||
description: "Werror" | ||||||||||
args: "-DCMAKE_CXX_FLAGS='-Werror=all -Werror=extra'" | ||||||||||
- platform: ubuntu-latest | ||||||||||
compiler: | ||||||||||
- platform: | ||||||||||
description: "ubuntu gcc" | ||||||||||
cpp: g++ | ||||||||||
c: gcc | ||||||||||
os: ubuntu-latest | ||||||||||
cpp_version: 17 | ||||||||||
cmake_args: | ||||||||||
description: "Dynamic" | ||||||||||
args: "-DBUILD_SHARED_LIBS=on" | ||||||||||
# MSVC has limited support for sanitizers, so we use include here for MSVC as an exception. | ||||||||||
# This should be simplified after beman-project/exemplar#76 is merged. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
It isn't clear what is meant by "use include here". Also, it is unclear if #76 in its current form will be merged. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah sorry I should check my comment |
||||||||||
- platform: | ||||||||||
description: "windows MSVC" | ||||||||||
cpp: cl | ||||||||||
c: cl | ||||||||||
os: windows-latest | ||||||||||
cpp_version: 17 | ||||||||||
cmake_args: | ||||||||||
description: "Default" | ||||||||||
args: "" | ||||||||||
- platform: | ||||||||||
description: "windows MSVC" | ||||||||||
cpp: cl | ||||||||||
c: cl | ||||||||||
os: windows-latest | ||||||||||
cpp_version: 17 | ||||||||||
cmake_args: | ||||||||||
description: "ASan" | ||||||||||
# Debug infomation needed to avoid cl: C5072 | ||||||||||
# https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-c5072?view=msvc-170 | ||||||||||
args: "-DCMAKE_CXX_FLAGS='/fsanitize=address /Zi'" | ||||||||||
|
||||||||||
|
||||||||||
name: "Unit: ${{ matrix.compiler.c }} ${{ matrix.cpp_version }} ${{ matrix.cmake_args.description }}" | ||||||||||
runs-on: ${{ matrix.platform }} | ||||||||||
name: "Unit: ${{ matrix.platform.description }} ${{ matrix.cpp_version }} ${{ matrix.cmake_args.description }}" | ||||||||||
runs-on: ${{ matrix.platform.os }} | ||||||||||
steps: | ||||||||||
- uses: actions/checkout@v4 | ||||||||||
- name: Install Ninja | ||||||||||
uses: lukka/get-cmake@latest | ||||||||||
with: | ||||||||||
cmakeVersion: "~3.25.0" | ||||||||||
ninjaVersion: "^1.11.1" | ||||||||||
- name: Setup MSVC | ||||||||||
if: startsWith(matrix.platform.os, 'windows') | ||||||||||
uses: TheMrMilchmann/setup-msvc-dev@v3 | ||||||||||
with: | ||||||||||
arch: x64 | ||||||||||
- name: Print installed softwares | ||||||||||
run: | | ||||||||||
clang++ --version | ||||||||||
g++ --version | ||||||||||
echo "Compiler:" | ||||||||||
${{ matrix.platform.cpp }} --version | ||||||||||
${{ matrix.platform.c }} --version | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that
Suggested change
This comment was marked as outdated.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||||||||||
|
||||||||||
echo "Build system:" | ||||||||||
cmake --version | ||||||||||
ninja --version | ||||||||||
- name: Configure CMake | ||||||||||
run: | | ||||||||||
cmake -B build -S . -DCMAKE_CXX_STANDARD=${{ matrix.cpp_version }} ${{ matrix.cmake_args.args }} | ||||||||||
env: | ||||||||||
CC: ${{ matrix.compiler.c }} | ||||||||||
CXX: ${{ matrix.compiler.cpp }} | ||||||||||
CC: ${{ matrix.platform.c }} | ||||||||||
CXX: ${{ matrix.platform.cpp }} | ||||||||||
CMAKE_GENERATOR: "Ninja Multi-Config" | ||||||||||
- name: Build Release | ||||||||||
run: | | ||||||||||
cmake --build build --config Release --verbose | ||||||||||
cmake --build build --config Release --target all_verify_interface_header_sets | ||||||||||
cmake --install build --config Release --prefix /opt/beman.exemplar | ||||||||||
find /opt/beman.exemplar -type f | ||||||||||
tree /opt/beman.exemplar | ||||||||||
wusatosi marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe something like this?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's a great idea! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This comment was marked as outdated.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agh, it's a quirk with powershell tree. See: https://superuser.com/questions/1270040/windows-10-powershell-tree-command-how-to-show-files There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use the same trick I suggested in this comment to use the |
||||||||||
- name: Test Release | ||||||||||
run: ctest --test-dir build --build-config Release | ||||||||||
- name: Build Debug | ||||||||||
run: | | ||||||||||
cmake --build build --config Debug --verbose | ||||||||||
cmake --build build --config Debug --target all_verify_interface_header_sets | ||||||||||
cmake --install build --config Debug --prefix /opt/beman.exemplar | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This install isn't going to do anything because it's installing to the same location as the Release install. Unless that's the intent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intention here is to test the install command. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It ends up skipping installation because it's already installed, which is a sort of test. Not critical for header only, but if there's any generated config or binaries, they shouldn't clobber each other. |
||||||||||
find /opt/beman.exemplar -type f | ||||||||||
tree /opt/beman.exemplar | ||||||||||
- name: Test Debug | ||||||||||
run: ctest --test-dir build --build-config Debug | ||||||||||
|
||||||||||
|
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.
if it's in description?
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.
Adopted.