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

Migrate Windows CI to GitHub Actions. #6312

Merged
merged 2 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 22 additions & 1 deletion .github/workflows/osrm-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@ env:
ENABLE_NODE_BINDINGS: "ON"

jobs:
windows:
Copy link
Member

@mjjbell mjjbell Aug 19, 2022

Choose a reason for hiding this comment

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

I forgot about this with the Docker job.

ci-complete is the meta-job that PRs will block merging on. We should add windows and docker to the list of needs, in the unlikely case that all the Linux/Mac jobs succeed before those two complete/fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, will do. Btw while we are waiting for someone to disable Appveyor check, may be just return that job and make it no-op(i.e. always green?). It would unblock this PR from being merged and would allow to work on other PRs which depend on it.

Copy link
Member

Choose a reason for hiding this comment

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

Good idea 👍

needs: format-taginfo-docs
runs-on: windows-2022
continue-on-error: false
steps:
- uses: actions/checkout@v3
- run: pip install conan==1.50.0
- run: conan --version
- run: cmake --version
- uses: actions/setup-node@v3
with:
node-version: 16
- run: node --version
- run: npm --version
- run: npm install --ignore-scripts
- run: npm link --ignore-scripts
- uses: microsoft/setup-msbuild@v1.1
- name: Build
run: |
.\scripts\ci\windows-build.bat

format-taginfo-docs:
runs-on: ubuntu-20.04
steps:
Expand Down Expand Up @@ -642,6 +663,6 @@ jobs:

ci-complete:
runs-on: ubuntu-18.04
needs: build-test-publish
needs: [build-test-publish, docker-image, windows]
steps:
- run: echo "CI complete"
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- FIXED: Fix bug with reading Set values from Lua scripts. [#6285](https://github.com/Project-OSRM/osrm-backend/pull/6285)
- FIXED: Bug in bicycle profile that caused exceptions if there is a highway=bicycle in the data. [#6296](https://github.com/Project-OSRM/osrm-backend/pull/6296)
- Build:
- CHANGED: Migrate Windows CI to GitHub Actions. [#6312](https://github.com/Project-OSRM/osrm-backend/pull/6312)
- ADDED: Add smoke test for Docker image. [#6313](https://github.com/Project-OSRM/osrm-backend/pull/6313)
- CHANGED: Update libosmium to version 2.18.0. [#6303](https://github.com/Project-OSRM/osrm-backend/pull/6303)
- CHANGED: Remove EXACT from find_package if using Conan. [#6299](https://github.com/Project-OSRM/osrm-backend/pull/6299)
Expand Down
17 changes: 16 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,6 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "Intel")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -static-intel -wd10237 -Wall -ipo -fPIC")
elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
# using Visual Studio C++
set(BOOST_COMPONENTS ${BOOST_COMPONENTS} zlib)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /bigobj") # avoid compiler error C1128 from scripting_environment_lua.cpp
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /DWIN32_LEAN_AND_MEAN") # avoid compiler error C2011 from dual #include of winsock.h and winsock2.h
add_dependency_defines(-DBOOST_LIB_DIAGNOSTIC)
Expand Down Expand Up @@ -446,6 +445,11 @@ add_subdirectory(${FLATBUFFERS_SRC_DIR}
${CMAKE_CURRENT_BINARY_DIR}/flatbuffers-build
EXCLUDE_FROM_ALL)

# see https://stackoverflow.com/questions/70898030/boost-link-error-using-conan-find-package
if (MSVC)
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Let's add this to the file too, for future reference.

add_definitions(-DBOOST_ALL_NO_LIB)
endif()

if(ENABLE_CONAN)
message(STATUS "Installing dependencies via Conan")

Expand All @@ -462,6 +466,16 @@ if(ENABLE_CONAN)
set(CONAN_TBB_VERSION 2020.3)

set(CONAN_SYSTEM_INCLUDES ON)

# TODO:
# if we link TBB dynamically osrm-extract.exe finishes on the first access to any TBB symbol
# with exit code = -1073741515, which means that program cannot load required DLL.
if (MSVC)
set(TBB_SHARED False)
else()
set(TBB_SHARED True)
endif()

conan_cmake_run(
REQUIRES
boost/${CONAN_BOOST_VERSION}
Expand All @@ -475,6 +489,7 @@ if(ENABLE_CONAN)
KEEP_RPATHS
NO_OUTPUT_DIRS
OPTIONS boost:filesystem_version=3 # https://stackoverflow.com/questions/73392648/error-with-boost-filesystem-version-in-cmake
tbb:shared=${TBB_SHARED}
)

add_dependency_includes(${CONAN_INCLUDE_DIRS_BOOST})
Expand Down
194 changes: 0 additions & 194 deletions appveyor-build.bat

This file was deleted.

27 changes: 3 additions & 24 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,36 +1,15 @@
environment:
matrix:
- configuration: Release
# - configuration: Debug

install:
- ps: Install-Product node 6

# scripts that are called at very beginning, before repo cloning
init:
- git config --global core.autocrlf input

os: Visual Studio 2019

# clone directory
clone_folder: c:\projects\osrm

platform: x64

# no-op for the time being until someone with access to GitHub checks settings will remove integration with AppVeyor
# https://github.com/Project-OSRM/osrm-backend/pull/6312#issuecomment-1217237055
build_script:
- CALL appveyor-build.bat

before_test:
- node --version
- npm --version
- npm install --ignore-scripts
- npm link --ignore-scripts
- SET PATH=%CD%\osrm-deps\lib;%PATH%
- SET OSRM_BUILD_DIR=build\%Configuration%
# TODO tests fail with "JavaScript heap out of memory", need a better host?
# - npm test
- EXIT 0

branches:
only:
- master

Loading