Migrate Windows CI to GitHub Actions. #6312

SiarheiFedartsou · 2022-08-16T21:30:20Z

Issue

It is quite difficult to maintain current Windows CI, because it uses pre-compiled dependencies stored on wolt.com server and there is low chance to find someone who is able to update it.

Needed for #6300

Tasklist

CHANGELOG.md entry (How to write a changelog entry)
update relevant Wiki pages
add tests (see testing documentation)
review
adjust for comments
cherry pick to release branch

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

SiarheiFedartsou · 2022-08-16T21:32:27Z

scripts/ci/windows-build.bat

@@ -0,0 +1,99 @@
+@ECHO OFF


It is modified old appveyor-build.bat

SiarheiFedartsou · 2022-08-16T21:35:44Z

CMakeLists.txt

@@ -474,6 +484,7 @@ if(ENABLE_CONAN)
    GENERATORS cmake_find_package
    KEEP_RPATHS
    NO_OUTPUT_DIRS
+    OPTIONS tbb:shared=${TBB_SHARED}


Tbh I spent several days trying to understand why osrm-extract.exe silently finishes with -1073741515 exit code(Google says that it is something about DLL loading), then I found that it happens on the first access of any TBB symbol. I tried to statically compile it and it worked. I believe it is possible to make it working with dynamic linkage, but I have no Windows machine and had to debug all these things directly on CI, so debugging is really limited for me.

👍
Can you add a comment to that effect in the file?

SiarheiFedartsou · 2022-08-16T21:36:17Z

CMakeLists.txt

@@ -446,6 +445,10 @@ add_subdirectory(${FLATBUFFERS_SRC_DIR}
        ${CMAKE_CURRENT_BINARY_DIR}/flatbuffers-build
        EXCLUDE_FROM_ALL)

+if (MSVC)


https://stackoverflow.com/questions/70898030/boost-link-error-using-conan-find-package

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

SiarheiFedartsou · 2022-08-16T21:39:10Z

@mjjbell please take a look when you have a chance

SiarheiFedartsou · 2022-08-16T21:46:04Z

Also probably someone with corresponding access rights should disable integration with AppVeyor:

mjjbell · 2022-08-16T22:20:09Z

I don't have access to the status checks. However I have access to the appveyor project, so I've removed the integration from that side.

mjjbell · 2022-08-16T22:32:45Z

I don't have access to the status checks. However I have access to the appveyor project, so I've removed the integration from that side.

I think we still need someone to disable it in the repo. @danpat @TheMarex would you be able to disable the Appveyor check in the repo settings?

mjjbell · 2022-08-19T10:41:41Z

.github/workflows/osrm-backend.yml

@@ -20,6 +20,27 @@ env:
  ENABLE_NODE_BINDINGS: "ON"

 jobs:
+  windows:


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.

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.

Good idea 👍

mjjbell

LGTM 🚀

One step closer to being able to publish Windows releases too.

DennisOSRM · 2022-08-21T09:22:40Z

Removed the appveyor web hook

v5.27.0 - Changes from 5.26.0 - API: - ADDED: Add Flatbuffers support to NodeJS bindings. [Project-OSRM#6338](Project-OSRM#6338) - CHANGED: Add `data_version` field to responses of all services. [Project-OSRM#5387](Project-OSRM#5387) - FIXED: Use Boost.Beast to parse HTTP request. [Project-OSRM#6294](Project-OSRM#6294) - FIXED: Fix inefficient osrm-routed connection handling [Project-OSRM#6113](https://gihub.com/Project-OSRM/osrm-backend/pull/6113) - FIXED: Fix HTTP compression precedence [Project-OSRM#6113](Project-OSRM#6113) - NodeJS: - FIXED: Support `skip_waypoints` in Node bindings [Project-OSRM#6060](Project-OSRM#6060) - Misc: - ADDED: conanbuildinfo.json for easy reading of dependencies [Project-OSRM#6388](Project-OSRM#6388) - CHANGED: Improve performance of JSON rendering. Fix undefined behaviour in JSON numbers formatting. [Project-OSRM#6380](Project-OSRM#6380) - ADDED: Add timestamps for logs. [Project-OSRM#6375](Project-OSRM#6375) - CHANGED: Improve performance of map matching via getPathDistance optimization. [Project-OSRM#6378](Project-OSRM#6378) - CHANGED: Optimize RestrictionParser performance. [Project-OSRM#6344](Project-OSRM#6344) - ADDED: Support floats for speed value in traffic updates CSV. [Project-OSRM#6327](Project-OSRM#6327) - CHANGED: Use Lua 5.4 in Docker image. [Project-OSRM#6346](Project-OSRM#6346) - CHANGED: Remove redundant nullptr check. [Project-OSRM#6326](Project-OSRM#6326) - CHANGED: missing files list is included in exception message. [Project-OSRM#5360](Project-OSRM#5360) - CHANGED: Do not use deprecated Callback::Call overload in Node bindings. [Project-OSRM#6318](Project-OSRM#6318) - FIXED: Fix distance calculation consistency. [Project-OSRM#6315](Project-OSRM#6315) - FIXED: Fix performance issue after migration to sol2 3.3.0. [Project-OSRM#6304](Project-OSRM#6304) - CHANGED: Pass osm_node_ids by reference in osrm::updater::Updater class. [Project-OSRM#6298](Project-OSRM#6298) - FIXED: Fix bug with reading Set values from Lua scripts. [Project-OSRM#6285](Project-OSRM#6285) - FIXED: Bug in bicycle profile that caused exceptions if there is a highway=bicycle in the data. [Project-OSRM#6296](Project-OSRM#6296) - FIXED: Internal refactoring of identifier types used in data facade [Project-OSRM#6044](Project-OSRM#6044) - CHANGED: Update docs to reflect recent build and dependency changes [Project-OSRM#6383](Project-OSRM#6383) - Build: - REMOVED: Get rid of Mason. [Project-OSRM#6387](Project-OSRM#6387) - CHANGED: Use clang-format from CI base image. [Project-OSRM#6391](Project-OSRM#6391) - ADDED: Build Node bindings on Windows. [Project-OSRM#6334](Project-OSRM#6334) - ADDED: Configure cross-compilation for Apple Silicon. [Project-OSRM#6360](Project-OSRM#6360) - CHANGED: Use apt-get to install Clang on CI. [Project-OSRM#6345](Project-OSRM#6345) - CHANGED: Fix TBB in case of Conan + NodeJS build. [Project-OSRM#6333](Project-OSRM#6333) - CHANGED: Migrate to modern TBB version. [Project-OSRM#6300](Project-OSRM#6300) - CHANGED: Enable performance-move-const-arg clang-tidy check. [Project-OSRM#6319](Project-OSRM#6319) - CHANGED: Use the latest node on CI. [Project-OSRM#6317](Project-OSRM#6317) - CHANGED: Migrate Windows CI to GitHub Actions. [Project-OSRM#6312](Project-OSRM#6312) - ADDED: Add smoke test for Docker image. [Project-OSRM#6313](Project-OSRM#6313) - CHANGED: Update libosmium to version 2.18.0. [Project-OSRM#6303](Project-OSRM#6303) - CHANGED: Remove EXACT from find_package if using Conan. [Project-OSRM#6299](Project-OSRM#6299) - CHANGED: Configure Undefined Behaviour Sanitizer. [Project-OSRM#6290](Project-OSRM#6290) - CHANGED: Use Conan instead of Mason to install code dependencies. [Project-OSRM#6284](Project-OSRM#6284) - CHANGED: Migrate to C++17. Update sol2 to 3.3.0. [Project-OSRM#6279](Project-OSRM#6279) - CHANGED: Update macOS CI image to macos-11. [Project-OSRM#6286](Project-OSRM#6286) - CHANGED: Enable even more clang-tidy checks. [Project-OSRM#6273](Project-OSRM#6273) - CHANGED: Configure CMake to not build flatbuffers tests and samples. [Project-OSRM#6274](Project-OSRM#6274) - CHANGED: Enable more clang-tidy checks. [Project-OSRM#6270](Project-OSRM#6270) - CHANGED: Configure clang-tidy job on CI. [Project-OSRM#6261](Project-OSRM#6261) - CHANGED: Use Github Actions for building container images [Project-OSRM#6138](Project-OSRM#6138) - CHANGED: Upgrade Boost dependency to 1.70 [Project-OSRM#6113](Project-OSRM#6113) - CHANGED: Upgrade Ubuntu CI builds to 20.04 [Project-OSRM#6119](Project-OSRM#6119) - CHANGED: Make building osrm-routed optional [Project-OSRM#6144](Project-OSRM#6144) - FIXED: Run all unit tests in CI [Project-OSRM#5248](Project-OSRM#5248) - FIXED: Fix installation of Mason CMake and 32 bit CI build [Project-OSRM#6170](Project-OSRM#6170) - FIXED: Fixed Node docs generation check in CI. [Project-OSRM#6058](Project-OSRM#6058) - CHANGED: Docker build, enabled arm64 build layer [Project-OSRM#6172](Project-OSRM#6172) - CHANGED: Docker build, enabled apt-get update/install caching in separate layer for build phase [Project-OSRM#6175](Project-OSRM#6175) - FIXED: Bump CI complete meta job to ubuntu-20.04 [Project-OSRM#6323](Project-OSRM#6323) - CHANGED: Node packages are now scoped by @Project-OSRM [Project-OSRM#6386](Project-OSRM#6386) - Routing: - CHANGED: Lazily generate optional route path data [Project-OSRM#6045](Project-OSRM#6045) - FIXED: Completed support for no_entry and no_exit turn restrictions. [Project-OSRM#5988](Project-OSRM#5988) - ADDED: Add support for non-round-trips with a single fixed endpoint. [Project-OSRM#6050](Project-OSRM#6050) - FIXED: Improvements to maneuver override processing [Project-OSRM#6125](Project-OSRM#6125) - ADDED: Support snapping to multiple ways at an input location. [Project-OSRM#5953](Project-OSRM#5953) - FIXED: Fix snapping target locations to ways used in turn restrictions. [Project-OSRM#6339](Project-OSRM#6339) - ADDED: Support OSM traffic signal directions. [Project-OSRM#6153](Project-OSRM#6153) - FIXED: Ensure u-turn exists in intersection view. [Project-OSRM#6376](Project-OSRM#6376) - FIXED: Gracefully handle no-turn intersections in guidance processing. [Project-OSRM#6382](Project-OSRM#6382) - Profile: - CHANGED: Bicycle surface speeds [Project-OSRM#6212](Project-OSRM#6212) - Tools: - CHANGED: Do not generate intermediate .osrm file in osrm-extract. [Project-OSRM#6354](Project-OSRM#6354)

It is quite difficult to maintain current Windows CI, because it uses pre-compiled dependencies stored on wolt.com server and there is low chance to find someone who is able to update it.

SiarheiFedartsou commented Aug 16, 2022

View reviewed changes

scripts/ci/windows-build.bat

@@ -0,0 +1,99 @@

@ECHO OFF

Copy link

Member Author

SiarheiFedartsou Aug 16, 2022

Choose a reason for hiding this comment

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

It is modified old appveyor-build.bat

SiarheiFedartsou commented Aug 16, 2022

View reviewed changes

SiarheiFedartsou force-pushed the sf-windows4 branch from 59f4479 to 580f998 Compare August 16, 2022 21:38

SiarheiFedartsou requested a review from mjjbell August 16, 2022 22:24

SiarheiFedartsou mentioned this pull request Aug 19, 2022

Remove references to project-osrm.wolt.com #6314

Merged

mjjbell reviewed Aug 19, 2022

View reviewed changes

Migrate Windows CI to GitHub Actions.

4635664

SiarheiFedartsou force-pushed the sf-windows4 branch from 3bdba33 to dc46932 Compare August 19, 2022 14:07

Migrate Windows CI to GitHub Actions.

9bd035b

SiarheiFedartsou force-pushed the sf-windows4 branch from dc46932 to 9bd035b Compare August 19, 2022 15:54

SiarheiFedartsou requested a review from mjjbell August 19, 2022 15:55

mjjbell approved these changes Aug 19, 2022

View reviewed changes

mjjbell merged commit 8f0cd5c into Project-OSRM:master Aug 19, 2022

SiarheiFedartsou mentioned this pull request Aug 20, 2022

Project status? #5209

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Windows CI to GitHub Actions. #6312

Migrate Windows CI to GitHub Actions. #6312

SiarheiFedartsou commented Aug 16, 2022

SiarheiFedartsou Aug 16, 2022

SiarheiFedartsou Aug 16, 2022 •

edited

Loading

mjjbell Aug 16, 2022

SiarheiFedartsou Aug 16, 2022

SiarheiFedartsou Aug 16, 2022

mjjbell Aug 16, 2022

SiarheiFedartsou commented Aug 16, 2022

SiarheiFedartsou commented Aug 16, 2022

mjjbell commented Aug 16, 2022

mjjbell commented Aug 16, 2022

mjjbell Aug 19, 2022 •

edited

Loading

SiarheiFedartsou Aug 19, 2022

mjjbell Aug 19, 2022

mjjbell left a comment

DennisOSRM commented Aug 21, 2022

Migrate Windows CI to GitHub Actions. #6312

Migrate Windows CI to GitHub Actions. #6312

Conversation

SiarheiFedartsou commented Aug 16, 2022

Issue

Tasklist

Requirements / Relations

SiarheiFedartsou Aug 16, 2022

Choose a reason for hiding this comment

SiarheiFedartsou Aug 16, 2022 • edited Loading

Choose a reason for hiding this comment

mjjbell Aug 16, 2022

Choose a reason for hiding this comment

SiarheiFedartsou Aug 16, 2022

Choose a reason for hiding this comment

SiarheiFedartsou Aug 16, 2022

Choose a reason for hiding this comment

mjjbell Aug 16, 2022

Choose a reason for hiding this comment

SiarheiFedartsou commented Aug 16, 2022

SiarheiFedartsou commented Aug 16, 2022

mjjbell commented Aug 16, 2022

mjjbell commented Aug 16, 2022

mjjbell Aug 19, 2022 • edited Loading

Choose a reason for hiding this comment

SiarheiFedartsou Aug 19, 2022

Choose a reason for hiding this comment

mjjbell Aug 19, 2022

Choose a reason for hiding this comment

mjjbell left a comment

Choose a reason for hiding this comment

DennisOSRM commented Aug 21, 2022

SiarheiFedartsou Aug 16, 2022 •

edited

Loading

mjjbell Aug 19, 2022 •

edited

Loading