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.

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

Conversation

csukuangfj
Copy link
Collaborator

@csukuangfj csukuangfj commented Apr 12, 2024

I find that there is also a PR for GitHub actions about releasing.
#46


I can help add CI tests for

  • other OSes, e.g., macOS and Linux

@csukuangfj
Copy link
Collaborator Author

By the way, I think you have to enable GitHub actions for this repo.

@csukuangfj
Copy link
Collaborator Author

You can find the running logs for this PR at
https://github.com/csukuangfj/openfst/actions/runs/8657834872/job/23740626893

(Note: Once GitHub actions is enabled for this PR, you can also find the logs in this repo)

@csukuangfj
Copy link
Collaborator Author

The error logs can be found at
https://github.com/csukuangfj/openfst/actions/runs/8657834872/job/23740626893#step:4:31931

C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(755,5):
 error MSB6006: "CL.exe" exited with code -1073740791. [D:\a\openfst\openfst\build\src\extensions\lookahead\arc_lookahead-fst.vcxproj]

.github/workflows/windows-x64.yaml Show resolved Hide resolved

mkdir build
cd build
cmake -A x64 -D CMAKE_BUILD_TYPE=${{ matrix.build_type }} -DCMAKE_INSTALL_PREFIX=./install ..
Copy link
Owner

Choose a reason for hiding this comment

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

What is the default generator on Windows? My CMake thinks it's VS2022 MSBuild files. I'm always using Ninja with CMake, even on Windows. Ninja is incomparably faster than an .sln and a heap of .*proj files!

Generators

The following generators are available on this platform (* marks default):
* Visual Studio 17 2022        = Generates Visual Studio 2022 project files.
                                 Use -A option to specify architecture.

You can do -G Ninja. I think CMake files only generate x64 arch. @jtrmal?

Ninja comes with VS2022, CMake payload (Microsoft.VisualStudio.Component.VC.CMake.Project). So if you have CMake, you have Ninja too.

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
https://github.com/csukuangfj/openfst/actions/runs/8780936061/job/24091877962#step:3:121

I think CMake files only generate x64 arch.

Actually, no.

Use -A option to specify architecture.

Please refer to
https://k2-fsa.github.io/sherpa/onnx/install/windows.html#bit-windows-x86
for how we build sherpa-onnx for 32-bit windows on 64-bit windows using CMake.

I will switch to Ninja.

Generators

The following generators are available on this platform (* marks default):
* Visual Studio 17 2022        = Generates Visual Studio 2022 project files.
                                 Use -A option to specify architecture.
  Visual Studio 16 2019        = Generates Visual Studio 2019 project files.
                                 Use -A option to specify architecture.
  Visual Studio 15 2017 [arch] = Generates Visual Studio 2017 project files.
                                 Optional [arch] can be "Win64" or "ARM".
  Visual Studio 14 2015 [arch] = Generates Visual Studio 2015 project files.
                                 Optional [arch] can be "Win64" or "ARM".
  Visual Studio 12 2013 [arch] = Deprecated.  Generates Visual Studio 2013
                                 project files.  Optional [arch] can be
                                 "Win64" or "ARM".
  Visual Studio 9 2008 [arch]  = Deprecated.  Generates Visual Studio 2008
                                 project files.  Optional [arch] can be
                                 "Win64" or "IA64".
  Borland Makefiles            = Generates Borland makefiles.
  NMake Makefiles              = Generates NMake makefiles.
  NMake Makefiles JOM          = Generates JOM makefiles.
  MSYS Makefiles               = Generates MSYS makefiles.
  MinGW Makefiles              = Generates a make file for use with
                                 mingw32-make.
  Green Hills MULTI            = Generates Green Hills MULTI files
                                 (experimental, work-in-progress).
  Unix Makefiles               = Generates standard UNIX makefiles.
  Ninja                        = Generates build.ninja files.
  Ninja Multi-Config           = Generates build-<Config>.ninja files.
  Watcom WMake                 = Generates Watcom WMake makefiles.
  CodeBlocks - MinGW Makefiles = Generates CodeBlocks project files
                                 (deprecated).
  CodeBlocks - NMake Makefiles = Generates CodeBlocks project files
                                 (deprecated).
  CodeBlocks - NMake Makefiles JOM
                               = Generates CodeBlocks project files
                                 (deprecated).
  CodeBlocks - Ninja           = Generates CodeBlocks project files
                                 (deprecated).
  CodeBlocks - Unix Makefiles  = Generates CodeBlocks project files
                                 (deprecated).
  CodeLite - MinGW Makefiles   = Generates CodeLite project files
                                 (deprecated).
  CodeLite - NMake Makefiles   = Generates CodeLite project files
                                 (deprecated).
  CodeLite - Ninja             = Generates CodeLite project files
                                 (deprecated).
  CodeLite - Unix Makefiles    = Generates CodeLite project files
                                 (deprecated).
  Eclipse CDT4 - NMake Makefiles
                               = Generates Eclipse CDT 4.0 project files
                                 (deprecated).
  Eclipse CDT4 - MinGW Makefiles
                               = Generates Eclipse CDT 4.0 project files
                                 (deprecated).
  Eclipse CDT4 - Ninja         = Generates Eclipse CDT 4.0 project files
                                 (deprecated).
  Eclipse CDT4 - Unix Makefiles= Generates Eclipse CDT 4.0 project files
                                 (deprecated).
  Kate - MinGW Makefiles       = Generates Kate project files (deprecated).
  Kate - NMake Makefiles       = Generates Kate project files (deprecated).
  Kate - Ninja                 = Generates Kate project files (deprecated).
  Kate - Ninja Multi-Config    = Generates Kate project files (deprecated).
  Kate - Unix Makefiles        = Generates Kate project files (deprecated).
  Sublime Text 2 - MinGW Makefiles
                               = Generates Sublime Text 2 project files
                                 (deprecated).
  Sublime Text 2 - NMake Makefiles
                               = Generates Sublime Text 2 project files
                                 (deprecated).
  Sublime Text 2 - Ninja       = Generates Sublime Text 2 project files
                                 (deprecated).
  Sublime Text 2 - Unix Makefiles
                               = Generates Sublime Text 2 project files
                                 (deprecated).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

CMake Error at CMakeLists.txt:2 (project):
  Generator
    Ninja
  does not support platform specification, but platform
    x64
  was specified.
CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
-- Configuring incomplete, errors occurred!

Ninja does not support specifying the arch to build.

I am going to extend the build script to windows 32 so I will not switch to Ninja to keep the code consistent both for
windows64 and windows32.

Copy link
Owner

Choose a reason for hiding this comment

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

That was what I meant: The concept of a Platform exists in a VS project file, so that -A is specific to this generator. This doesn't mean that Ninja doesn't support 32-bit builds (you can build 32-bit executables for 64-bit Linux, too), it just means Ninja isn't built with that platform concept in mind. In cross-compilation (and compiling a 32-bit binary on a 64-bit platform is a kind of cross-compilation), Linux-style equivalent of the MS-specific "Platform" is the architecture triple. And while CMake indeed supports cross-compilation, it requires programming it into CMakeFiles. We better avoid that.

The simpler approach is to set up the environment following the VS command-line toolset setup routine. VS installs shortcuts that run cmd and make it source an environment setup script.

All these use the same script, only its argument is different. The issue (or non-issue) is locating this environment setup script robustly is tricky: cmd is a weird language, pwsh is easier, but still takes some contortions. But there is an easy hack to change the environment from one type to another, since the path to the script is already in an environment variable. I'm assuming you are using cmd as the shell; it simply won't work in WSL bash.

  1. Figure out the toolchain's host arch: run echo %VSCMD_ARG_HOST_ARCH% in the runner to see if it's x64 (64-bit) or x86 (32-bit). This is to preserve the host arch at step 3. I can't imagine that this value could ever change in the runner image, so we can simply hard-code it.
  2. Reset a few environment variables that the setup script blindly augments:
set EXTERNAL_INCLUDE=
set INCLUDE=
set LIB=
set LIBPATH=
set PATH=%__VSCMD_PREINIT_PATH%
  1. Find out the correct argument to the setup script:
    a. If the host arch is x64, the argument is x64 to build 64-bit binaries, and x64_x86 to cross-build 32-bit binaries.
    b. If the host arch is x86, the argument is x86_x64 to cross-build 64-bit binaries, and x86 to build 32-bit binaries.
  2. Call the setup script with that argument (call to cmd is what's source to Bash):
call "%VCINSTALLDIR%\Auxiliary\Build\vcvarsall" <arg>

This is all to it. Further, you can even skip steps 1 and 3, and simply use x64 or x86 for the <arg> at step 4. This is the same as opening the shell set up with the "native" environment using the shortcuts in the picture for both 32 and 64 bit builds. The whole kaboodle is only to modify the environment as little as possible. Personally, I wouldn't care about preserving the host arch, and would do this, the simpler way.

Comment on lines 46 to 47
cmake --build . --config Release -- -m:2
cmake --build . --config Release --target install -- -m:2
Copy link
Owner

Choose a reason for hiding this comment

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

For Ninja, the verbose switch after -- is -v.

fetch-depth: 0

- name: Configure CMake
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.

.github/workflows/windows-x64.yaml Show resolved Hide resolved
@kkm000
Copy link
Owner

kkm000 commented Apr 21, 2024

@csukuangfj, I enabled actions, but I don't know if they require hosting a runner. The runner list is empty. If that's required, can you do that? I can host one in GCP, too.

@csukuangfj
Copy link
Collaborator Author

@csukuangfj, I enabled actions,

Thanks! GitHub actions won't run until you merge this PR into the main branch ,i.e., winport, of this repo.

but I don't know if they require hosting a runner.

No, you don't. GitHub provides free runners and that is enough for building openfst.


The GitHub action logs for this PR can be found at
https://github.com/csukuangfj/openfst/actions/runs/8781456660

@kkm000
Copy link
Owner

kkm000 commented Apr 23, 2024

GitHub provides free runners and that is enough for building openfst.

Cool! One fewer headache.

The GitHub action logs for this PR can be found at https://github.com/csukuangfj/openfst/actions/runs/8781456660

I'll look at it later this week, still swamped in my urgent project, sorry. My guess is something simple have gone very wrong.

@kkm000
Copy link
Owner

kkm000 commented Apr 23, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants