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

Add ci-windows-shared-local.yml #2309

Merged
merged 9 commits into from
Jul 30, 2024
Merged

Add ci-windows-shared-local.yml #2309

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c518c14
Add ci-windows-shared-local.yml
wantehchang Jul 22, 2024
6e8c535
Set BUILD_SHARED_LIBS to ON
wantehchang Jul 22, 2024
02cb68c
On Windows copy avif.dll to tests binary directory
wantehchang Jul 23, 2024
1ae0307
Enable experimental features
wantehchang Jul 23, 2024
12a030a
Move add_custom_command() after add_subdirectory()
wantehchang Jul 24, 2024
4343faf
Disable experimental features, add comment
wantehchang Jul 26, 2024
a757ce4
Re-enable experimental features
wantehchang Jul 29, 2024
a941372
Disable AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP
wantehchang Jul 30, 2024
4ba66e4
Run on merge only
wantehchang Jul 30, 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
65 changes: 65 additions & 0 deletions .github/workflows/ci-windows-shared-local.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Do a shared library build with local dependencies.

name: CI Windows Shared Local
on:
push:
pull_request:
paths:
- '.github/workflows/ci-windows-shared-local.yml'
- '**CMakeLists.txt'
- 'cmake/**'
- 'ext/**'

permissions:
contents: read

# Cancel the workflow if a new one is triggered from the same PR, branch, or tag, except on main.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}

jobs:
build-windows-shared-local:
runs-on: windows-latest
strategy:
fail-fast: false
matrix:
compiler: [msvc, clang-cl]

steps:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- uses: ./.github/actions/setup-windows
with:
codec-aom: 'LOCAL'
codec-dav1d: 'LOCAL'
extra-cache-key: ${{ matrix.compiler }}
- name: Leave compiler as default
Copy link
Collaborator

Choose a reason for hiding this comment

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

Setting those variables should probably be put in .github/actions/setup-windows

if: matrix.compiler == 'msvc'
run: |
echo "AVIF_CMAKE_C_COMPILER=" >> $env:GITHUB_ENV
echo "AVIF_CMAKE_CXX_COMPILER=" >> $env:GITHUB_ENV
- name: Set clang-cl as compiler
if: matrix.compiler == 'clang-cl'
run: |
echo "AVIF_CMAKE_C_COMPILER=-DCMAKE_C_COMPILER=clang-cl" >> $env:GITHUB_ENV
echo "AVIF_CMAKE_CXX_COMPILER=-DCMAKE_CXX_COMPILER=clang-cl" >> $env:GITHUB_ENV

- name: Prepare libavif (cmake)
run: >
cmake -G Ninja -S . -B build
wantehchang marked this conversation as resolved.
Show resolved Hide resolved
-DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=ON
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should Debug be tested too? I remember difficulties when linking differently built dependencies and wonder if that should be exercised. Probably not worth the effort.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can looking into adding Debug later. Right now I am focusing on getting a clean (no compiler or linker warnings) Release build.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yannis: While working on related build issues on Windows, I remembered the linking issue you were referring to.

In Debug build, we compile source files with /MDd on Windows. The lowercase d means we are using a debug version of the run-time library. All of our dependencies need to be compiled with /MDd to avoid a linker warning.

We may be able to set the CMAKE_MSVC_RUNTIME_LIBRARY variable so that we use the /MD compiler option in all build configurations.

-DAVIF_CODEC_AOM=LOCAL -DAVIF_CODEC_DAV1D=LOCAL
-DAVIF_JPEG=LOCAL -DAVIF_LIBSHARPYUV=LOCAL
-DAVIF_LIBYUV=LOCAL -DAVIF_ZLIBPNG=LOCAL
-DAVIF_BUILD_EXAMPLES=ON -DAVIF_BUILD_APPS=ON
-DAVIF_BUILD_TESTS=ON -DAVIF_ENABLE_GTEST=ON -DAVIF_GTEST=LOCAL
-DAVIF_ENABLE_EXPERIMENTAL_YCGCO_R=ON
-DAVIF_ENABLE_EXPERIMENTAL_METAV1=ON
-DAVIF_ENABLE_EXPERIMENTAL_SAMPLE_TRANSFORM=ON
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a thought: we could have a CI with a matrix testing all combinations of these experimental flags

Copy link
Collaborator Author

@wantehchang wantehchang Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That's a lot of combinations (16) to test. I think we just need to test three combinations:

  • No experimental features are enabled.
  • Only the gain map experimental feature is enabled. (Chrome uses this combination.)
  • All experimental features are enabled.

-DAVIF_ENABLE_WERROR=ON $env:AVIF_CMAKE_C_COMPILER $env:AVIF_CMAKE_CXX_COMPILER
- name: Build libavif (ninja)
working-directory: ./build
run: ninja
- name: Run AVIF Tests
working-directory: ./build
run: ctest -j $Env:NUMBER_OF_PROCESSORS --output-on-failure
Loading