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 test for Windows with GitHub actions #48

Open
wants to merge 9 commits into
base: winport
Choose a base branch
from
Open

Add CI test for Windows with GitHub actions #48

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
57531d6
Add CI test for Windows with GitHub actions
csukuangfj Apr 12, 2024
3ac85e4
minor fixes
csukuangfj Apr 12, 2024
ecbb347
show cmake help
csukuangfj Apr 22, 2024
f6c0551
fix comments
csukuangfj Apr 22, 2024
de964e2
remove ninja
csukuangfj Apr 22, 2024
c9f2bdf
Fixes after review
csukuangfj Apr 23, 2024
5cd98d8
switch to bash
csukuangfj Apr 23, 2024
95fe0bc
install Ninja
csukuangfj Apr 23, 2024
a415140
use -j for parallel build
csukuangfj Apr 23, 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
72 changes: 72 additions & 0 deletions .github/workflows/windows-x64.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
name: windows-x64

on:
push:
branches:
- master
kkm000 marked this conversation as resolved.
Show resolved Hide resolved
- alpha
- preflight

pull_request:
branches:
- master
- alpha
- preflight

workflow_dispatch:

concurrency:
group: windows-x64-${{ github.ref }}
cancel-in-progress: true
kkm000 marked this conversation as resolved.
Show resolved Hide resolved

jobs:
windows_x64:
name: Windows x64
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [windows-latest]
build_type: [Release, Debug]

steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: "3.8"

- name: Install Python dependencies
shell: cmd
run: |
pip install ninja

- name: Show ninja help
shell: bash
Copy link
Owner

Choose a reason for hiding this comment

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

bash exists on Windows only when WSL is installed. It may take 10 seconds to boot the lightweight WSL VM, and file access is fairly slow from inside of the VM to files outside of its VHD. I'd use cmd if I were you. The default on Windows is pwsh. Verify that the VS command line environment is set up in the worker:

shell: cmd
run: |
  cmake --version
  where cmake
  ninja --version
  where ninja
  cl

I'm getting

cmake version 3.25.0
CMake suite maintained and supported by Kitware (kitware.com/cmake).
C:\Program Files\Microsoft Visual Studio\2022\Professional\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe
1.11.1  <== this is ninja
C:\Program Files\Microsoft Visual Studio\2022\Professional\Common7\IDE\CommonExtensions\Microsoft\CMake\Ninja\ninja.exe
Microsoft (R) C/C++ Optimizing Compiler Version 19.39.33523 for x86
usage: cl [ option... ] filename... [ /link linkoption... ]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It may take 10 seconds to boot the lightweight WSL VM

It is quite fast in GitHub actions and you won't feel any latency between cmd and bash.

Copy link
Owner

Choose a reason for hiding this comment

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

I still strongly suggest switching to the native shell. I know what is going on in the guts of the OS when you use WSL Bash this way, and that's a lot, and I do mean a lot. A hypervisor partition is created, then a network switch, a Linux OS is booted into this partition from a .vhd virtual disk, then it mounts Windows volumes using the 9p filesystem over a virtual network, and it maps Linux rwxrwxrwx attributes to Windows' ACLs in a tricky way, also depending on whether and how WSL is configured in its /etc/wsl.conf... An upgrade to WSL may easily break it in a spectacular way. If Microsoft says don't do it, I better don't.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please see the following error after switching to cmd and Ninja:
https://github.com/csukuangfj/openfst/actions/runs/8794917596/job/24135134756#step:4:4

CMake suite maintained and supported by Kitware (kitware.com/cmake).
CMake Error: CMake was unable to find a build program corresponding to "Ninja".  CMAKE_MAKE_PROGRAM is not set.  You probably need to select a different build tool.
CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
-- Configuring incomplete, errors occurred!

I want to point out that the main purpose of GitHub actions is to check that the code compiles and runs.
It does not matter whether cmd or bash, or Ninja is used. The time for building in GitHub actions can be ignored, though I have never noticed using cmd or Ninja in GitHub actions brings speedup in building.

run: |
ninja --help || true

- name: CMake version
shell: cmd
run: |
cmake --version
cmake --help

- name: Configure CMake
shell: cmd
run: |
cmake --version

mkdir build
cd build
cmake -G Ninja -D CMAKE_BUILD_TYPE=${{ matrix.build_type }} -DCMAKE_INSTALL_PREFIX=./install ..

- name: Build openfst for Windows x64
shell: cmd
run: |
cd build
cmake --build . --config Release -- -j 6
cmake --build . --config Release --target install -- -j 6