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

Jump to bottom

CI improvements #368

Closed
wants to merge 25 commits into from
Closed

CI improvements #368

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e04110c
Streamline format checking
bifurcation Sep 9, 2023
ba572e2
Use a specific version of clang-format-action
bifurcation Sep 9, 2023
24af92c
Induce failures
bifurcation Sep 9, 2023
0d8f3f9
Add path to regex
bifurcation Sep 9, 2023
f120c77
Add path to regex (take 2)
bifurcation Sep 9, 2023
c0498b6
Add path to regex (take 3)
bifurcation Sep 9, 2023
ba8f6b6
Fix induced errors
bifurcation Sep 9, 2023
e68d127
Use lukka/run-vcpkg
bifurcation Sep 9, 2023
45dac6f
Fix workflow file syntax
bifurcation Sep 9, 2023
d3b8f1f
Correct directory
bifurcation Sep 9, 2023
1adbc00
Don't wait on clang-format
bifurcation Sep 9, 2023
b3a449b
Streamline toolchain file definitions
bifurcation Sep 9, 2023
340f8db
Re-enable remaining linux check builds
bifurcation Sep 9, 2023
e4d01e8
Apply run-vcpk to 'older macOS' build
bifurcation Sep 9, 2023
e7ab7f4
Enable for draft PR
bifurcation Sep 9, 2023
d46046e
Remove unnecessary dependencies
bifurcation Sep 9, 2023
4b461d6
Update OpenSSL3 vcpkg baseline
bifurcation Sep 9, 2023
5fec99c
Update to latest vcpkg
bifurcation Sep 9, 2023
e5c3541
Enable linter builds
bifurcation Sep 9, 2023
363113c
Enable linter builds for draft PRs
bifurcation Sep 9, 2023
7511cf0
Remove unnecessary restore-cache step
bifurcation Sep 9, 2023
ac96b07
Disable slow dependency fetching
bifurcation Sep 9, 2023
f5fa4a5
Enable all builds, clean up more vars
bifurcation Sep 9, 2023
7dd2f81
Don't clean up build artifacts
bifurcation Sep 9, 2023
035b51b
Use Ninja for builds
bifurcation Sep 11, 2023
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
112 changes: 41 additions & 71 deletions .github/workflows/main_ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@ on:

env:
CTEST_OUTPUT_ON_FAILURE: 1
CMAKE_BUILD_DIR: ${{ github.workspace }}/build
CMAKE_BUILD_OPENSSL3_DIR: ${{ github.workspace }}/build_openssl3
CMAKE_TEST_OPENSSL3_DIR: ${{ github.workspace }}/build_openssl3/test
CMAKE_TEST_DIR: ${{ github.workspace }}/build/test
VCPKG_BINARY_SOURCES: files,${{ github.workspace }}/build/cache,readwrite
CMAKE_TOOLCHAIN_FILE: ${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake
VCPKG_COMMIT_ID: 962e5e39f8a25f42522f51fffc574e05a3efd26b

jobs:
formatting-check:
Expand All @@ -23,11 +20,8 @@ jobs:
strategy:
matrix:
path:
- 'include'
- 'src'
- 'test'
- 'cmd'
- 'lib'
- '.'
- 'cmd/interop'
bifurcation marked this conversation as resolved.
Show resolved Hide resolved
steps:
- uses: actions/checkout@v3

Expand All @@ -36,7 +30,7 @@ jobs:
with:
clang-format-version: '16'
check-path: ${{ matrix.path }}
fallback-style: 'Mozilla'
include-regex: ${{ matrix.path }}\/(src|include|test|lib)\/.*\.(cpp|h)

quick-linux-interop-check:
needs: formatting-check
Expand All @@ -45,30 +39,27 @@ jobs:
steps:
- uses: actions/checkout@v3

- name: Dependencies (Ubuntu)
if: ${{ matrix.os == 'ubuntu-latest' }}
run: |
sudo apt-get install -y linux-headers-$(uname -r)
- uses: lukka/get-cmake@latest

- name: Restore cache
uses: actions/cache@v3
- name: Setup vcpkg
uses: lukka/run-vcpkg@v10
with:
path: ${{ github.workspace }}/build/cache
key: VCPKG-BinaryCache-${{ runner.os }}
vcpkgGitCommitId: ${{ env.VCPKG_COMMIT_ID }}
vcpkgDirectory: vcpkg

- name: Build (OpenSSL 1.1)
run: |
cmake -B "${{ env.CMAKE_BUILD_DIR }}" -DTESTING=ON -DCMAKE_TOOLCHAIN_FILE="$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake"
cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target all --parallel 2
cmake -B build -DTESTING=ON
cmake --build build --target all
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add the --parallel 2 back. Even though we may only have once vcpu, we want to keep the compiler as busy as possible. I have always seen recommendations of cpu.count + 1.


- name: Unit Test (OpenSSL 1.1)
run: |
cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target test
cmake --build build --target test

- name: Build (Interop Harness)
run: |
cd cmd/interop
cmake -B build -DCMAKE_TOOLCHAIN_FILE="$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake"
cmake -B build
cmake --build build
Copy link
Contributor

Choose a reason for hiding this comment

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

add --paralllel 2


- name: Test self-interop
Expand All @@ -83,15 +74,15 @@ jobs:
run: |
cd cmd/interop
./grpc-self-test.sh

- name: Build (OpenSSL 3)
run: |
cmake -B "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" -DTESTING=ON -DVCPKG_MANIFEST_DIR="alternatives/openssl_3" -DCMAKE_TOOLCHAIN_FILE="$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake"
cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}"
cmake -B build -DTESTING=ON -DVCPKG_MANIFEST_DIR="alternatives/openssl_3"
cmake --build build

- name: Unit Test (OpenSSL 3)
run: |
cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" --target test
cmake --build build --target test
Copy link
Contributor

Choose a reason for hiding this comment

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

add --parallel 2

I won't bore you with adding this repeatedly.


platform-sanitizer-tests:
if: github.event.pull_request.draft == false
Expand All @@ -104,86 +95,65 @@ jobs:
os: [windows-latest, ubuntu-latest, macos-latest]
include:
- os: windows-latest
vcpkg-cmake-file: "$env:VCPKG_INSTALLATION_ROOT\\scripts\\buildsystems\\vcpkg.cmake"
ossl3-vcpkg-dir: "alternatives\\openssl_3"
ctest-target: RUN_TESTS
- os: ubuntu-latest
vcpkg-cmake-file: "$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake"
ossl3-vcpkg-dir: "alternatives/openssl_3"
ctest-target: test
- os: macos-latest
vcpkg-cmake-file: "$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake"
ossl3-vcpkg-dir: "alternatives/openssl_3"
ctest-target: test

steps:
- uses: actions/checkout@v3

- name: Dependencies (macOs)
if: ${{ matrix.os == 'macos-latest' }}
run: |
brew install llvm pkg-config
ln -s "/usr/local/opt/llvm/bin/clang-format" "/usr/local/bin/clang-format"
ln -s "/usr/local/opt/llvm/bin/clang-tidy" "/usr/local/bin/clang-tidy"
- uses: lukka/get-cmake@latest
Copy link
Contributor

Choose a reason for hiding this comment

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

for a build environment, I would try and lock in a version.

- uses: lukka/get-cmake@latest
+ uses: lukka/[email protected]


- name: Dependencies (Ubuntu)
if: ${{ matrix.os == 'ubuntu-latest' }}
run: |
sudo apt-get install -y linux-headers-$(uname -r)

- name: Restore cache
uses: actions/cache@v3
- name: Setup vcpkg
uses: lukka/run-vcpkg@v10
with:
path: ${{ github.workspace }}/build/cache
key: VCPKG-BinaryCache-${{ runner.os }}
vcpkgGitCommitId: ${{ env.VCPKG_COMMIT_ID }}
vcpkgDirectory: vcpkg

- name: Build (OpenSSL1.1)
- name: Build (OpenSSL 1.1)
run: |
cmake -B "${{ env.CMAKE_BUILD_DIR }}" -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DCMAKE_TOOLCHAIN_FILE="${{ matrix.vcpkg-cmake-file}}"
cmake --build "${{ env.CMAKE_BUILD_DIR }}" --parallel 2
cmake -B build -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON
cmake --build build

- name: Unit Test (OpenSSL1.1)
- name: Unit Test (OpenSSL 1.1)
run: |
cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target "${{ matrix.ctest-target}}"
cmake --build build --target "${{ matrix.ctest-target}}"

- name: Build (OpenSSL3)
- name: Build (OpenSSL 3)
run: |
cmake -B "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DVCPKG_MANIFEST_DIR="${{ matrix.ossl3-vcpkg-dir }}" -DCMAKE_TOOLCHAIN_FILE="${{ matrix.vcpkg-cmake-file}}"
cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}"
cmake -B build -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DVCPKG_MANIFEST_DIR="${{ matrix.ossl3-vcpkg-dir }}"
cmake --build build

- name: Unit Test (OpenSSL3)
run: |
cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" --target "${{ matrix.ctest-target}}"
cmake --build build --target "${{ matrix.ctest-target}}"

old-macos-compatibility:
if: github.event.pull_request.draft == false
needs: quick-linux-interop-check
name: Build for older MacOS
name: Build for older macOS
runs-on: macos-latest

env:
CMAKE_BUILD_DIR: ${{ github.workspace }}/build
TOOLCHAIN_FILE: $VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake
VCPKG_BINARY_SOURCES: files,${{ github.workspace }}/build/cache,readwrite
MACOSX_DEPLOYMENT_TARGET: 10.11
MACOSX_DEPLOYMENT_TARGET: 10.11

steps:
- uses: actions/checkout@v3

- name: dependencies
run: |
brew install llvm pkg-config
ln -s "/usr/local/opt/llvm/bin/clang-format" "/usr/local/bin/clang-format"
ln -s "/usr/local/opt/llvm/bin/clang-tidy" "/usr/local/bin/clang-tidy"
- uses: lukka/get-cmake@latest

- name: Restore cache
uses: actions/cache@v3
- name: Setup vcpkg
uses: lukka/run-vcpkg@v10
with:
path: ${{ github.workspace }}/build/cache
key: VCPKG-BinaryCache-${{ runner.os }}
vcpkgGitCommitId: ${{ env.VCPKG_COMMIT_ID }}
vcpkgDirectory: vcpkg

- name: Build
run: |
cmake -B "${{ env.CMAKE_BUILD_DIR }}" -DCMAKE_TOOLCHAIN_FILE="${{ env.TOOLCHAIN_FILE }}"
cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target mlspp --parallel 2

cmake -B build
cmake --build build --target mlspp
2 changes: 1 addition & 1 deletion alternatives/openssl_3/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"doctest",
"nlohmann-json"
],
"builtin-baseline": "5908d702d61cea1429b223a0b7a10ab86bad4c78",
"builtin-baseline": "962e5e39f8a25f42522f51fffc574e05a3efd26b",
Copy link
Contributor

Choose a reason for hiding this comment

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

If this passes CI, that is great!

"overrides": [
{
"name": "openssl",
Expand Down
2 changes: 1 addition & 1 deletion cmd/interop/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"gflags",
"nlohmann-json"
],
"builtin-baseline": "3b3bd424827a1f7f4813216f6b32b6c61e386b2e",
"builtin-baseline": "962e5e39f8a25f42522f51fffc574e05a3efd26b",
"overrides": [
{
"name": "openssl",
Expand Down
2 changes: 1 addition & 1 deletion vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"doctest",
"nlohmann-json"
],
"builtin-baseline": "3b3bd424827a1f7f4813216f6b32b6c61e386b2e",
"builtin-baseline": "962e5e39f8a25f42522f51fffc574e05a3efd26b",
"overrides": [
{
"name": "openssl",
Expand Down
Loading