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

Introduce MSVC CI support #80

Merged
merged 17 commits into from
Dec 10, 2024
Merged

Introduce MSVC CI support #80

Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
2bf94fa
init windows CI
wusatosi Nov 14, 2024
37fe7ce
update config
wusatosi Nov 14, 2024
625f140
temp find -> tree
wusatosi Nov 14, 2024
1bd9055
ugly workaround
wusatosi Nov 19, 2024
8c4c8a4
add debug info for msvc asan
wusatosi Nov 19, 2024
47cc829
cleanup
wusatosi Nov 19, 2024
45e1005
Update .github/workflows/ci_tests.yml
wusatosi Dec 4, 2024
01bf599
Update .github/workflows/ci_tests.yml
wusatosi Dec 4, 2024
ac89671
Update .github/workflows/ci_tests.yml
wusatosi Dec 4, 2024
2230b3e
Revert no --version on cl
wusatosi Dec 4, 2024
7f774ab
use temporary directory for install
wusatosi Dec 4, 2024
e8f6417
Use ls -R instead of tree for compatibility
wusatosi Dec 4, 2024
2286fa6
fix print software version for cl
wusatosi Dec 5, 2024
84a7177
update configuration testing
wusatosi Dec 5, 2024
1a3c911
revert back to use /opt/beman.exemplar
wusatosi Dec 5, 2024
fe6fc45
add comment
wusatosi Dec 5, 2024
bdb06fc
capitalize description
wusatosi Dec 5, 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
82 changes: 64 additions & 18 deletions .github/workflows/ci_tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -46,59 +49,100 @@ 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"
- 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
shell: bash
run: |
clang++ --version
g++ --version
echo "Compiler:"

# cl does not have a --version option
if [ "${{ matrix.platform.cpp }}" != "cl" ]; then
${{ matrix.platform.cpp }} --version
${{ matrix.platform.c }} --version
else
${{ matrix.platform.cpp }}
${{ matrix.platform.c }}
fi

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: |
# Portable commands only
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
ls -R /opt/beman.exemplar
Copy link
Member

@neatudarius neatudarius 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.

why find is not good?

do we just prefer a recursive display? If so, why not using tree?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we need to execute this on windows runner for MSVC, where tree doesn't work with the /opt/.... Directory.

Should I include a comment here documenting this?

Copy link
Member

Choose a reason for hiding this comment

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

OK, got it, right. I would add a command like name: Build Release # Portable commands only. thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Adopted.

- name: Test Release
run: ctest --test-dir build --build-config Release
- name: Build Debug
run: |
# Portable commands only
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
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Intention here is to test the install command.

Copy link
Member

Choose a reason for hiding this comment

The 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
ls -R /opt/beman.exemplar
- name: Test Debug
run: ctest --test-dir build --build-config Debug

Expand Down Expand Up @@ -130,16 +174,18 @@ jobs:
CMAKE_GENERATOR: "Ninja Multi-Config"
- name: Build Release
run: |
# Portable commands only
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
ls -R /opt/beman.exemplar
- name: Build Debug
run: |
# Portable commands only
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
find /opt/beman.exemplar -type f
ls -R /opt/beman.exemplar

create-issue-when-fault:
runs-on: ubuntu-latest
Expand Down
Loading