Closed esteve closed 2 years ago
@ooeygui cool! I've managed to get the Linux build working, but colcon has issues with the packages in both UWP builds. Moreover the desktop build can't build rcutils, I'll spin a Windows VM and try building that locally.
Thank you for your work on this!
I experimented on different approaches to CI - including attempting to convert the solution completely to ros-tooling. I think building all of ros is contributing to the circular dependency. Perhaps leveraging RoboStack could improve the CI round trip and eliminate the circular reference. FYI: I am out of office until January.
Looking at this.
@ooeygui did you have time to look into this? Anything I can help with?
Hi @esteve, I'm working on it in https://github.com/ms-iot/ros2_dotnet/tree/dev/test_ci. (so I don't spam this repo with CI tests). I was attempting to use the official ROS2 action for building. I think there's a problem with it, so set up a github custom runner. However, the official documentation for setting up a windows github runner doesn't work for hyper-v, so I had to create a derivative. This is now functional, and I'm working on debugging the CI action this week.
I set up a local build of Github and figured out why the builds aren't working with the ROS2 action. There's a pending change in https://github.com/ros-tooling/action-ros-ci/pull/712 which corrects the build environment for windows. I think there is still a bug with checked in vcs files which I'm currently debugging.
@ooeygui thank you so much for so much work. I'd like to make some changes to the CI to make it look cleaner, but I can wait until you're done with the fixes. Thanks again!
I'm testing the 712 PR and in general it looks good. However, I'm debugging why local_setup.ps1 isn't getting generated, which causes the build to fail.
colcon_package_source_powershell_script : not found: (traceback)
'C:\Users\...\github\ros2_dotnet\ros_ws\install\share/ament_cmake_export_assemblies/local_setup.ps1'
At C:\Users\...\github\ros2_dotnet\ros_ws\install\share\ament_cmake_export_assemblies\package.ps1:63 char:1
+ colcon_package_source_powershell_script "$env:COLCON_CURRENT_PREFIX\s ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : NotSpecified: (:) [Write-Error], WriteErrorException
+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,colcon_package_source_powershell_script
I'm debugging why $
In order to handle the build race condition with the generator, we need to use --merge-install. However, merge-install executes a codepath which runs in the generate phase - where <$CONFIG> cannot be evaluated. The dotnet_cmake_module project uses <$CONFIG> to switch between Release and Debug, because CMAKE_BUILD_TYPE on the msbuild generator is a complex string, not a simple Debug/Release like on other generators. Ninja is a has a simple build type, but difficult to set using colcon through the CI scripts (not as simple as --use-ninja).
I experimented with trying to use ninja in the build, but CI is unhappy with that.
Looking at fixing the dotnet_cmake_module now.
I was able to resolve the build issue; except it looks like github released a new CI image a couple of days ago which impacts OpenSSL:
CMake Error at C:/Program Files/CMake/share/cmake-3.22/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the
system variable OPENSSL_ROOT_DIR (missing: OPENSSL_CRYPTO_LIBRARY) (found
version "1.1.1i")
Call Stack (most recent call first):
C:/Program Files/CMake/share/cmake-3.22/Modules/FindPackageHandleStandardArgs.cmake:594 (_FPHSA_FAILURE_MESSAGE)
C:/Program Files/CMake/share/cmake-3.22/Modules/FindOpenSSL.cmake:574 (find_package_handle_standard_args)
C:/dev/foxy/ros2-windows/share/fastrtps/cmake/fastrtps-config.cmake:52 (find_package)
C:/dev/foxy/ros2-windows/share/rosidl_typesupport_fastrtps_c/cmake/rosidl_typesupport_fastrtps_c-extras.cmake:6 (find_package)
C:/dev/foxy/ros2-windows/share/rosidl_typesupport_fastrtps_c/cmake/rosidl_typesupport_fastrtps_cConfig.cmake:41 (include)
C:/dev/foxy/ros2-windows/share/rosidl_typesupport_c/cmake/rosidl_typesupport_c-extras.cmake:13 (find_package)
C:/dev/foxy/ros2-windows/share/rosidl_typesupport_c/cmake/rosidl_typesupport_cConfig.cmake:41 (include)
D:/a/ros2_dotnet/ros2_dotnet/ros_ws/install/share/rosidl_default_generators/cmake/rosidl_default_generators-extras.cmake:21 (find_package)
D:/a/ros2_dotnet/ros2_dotnet/ros_ws/install/share/rosidl_default_generators/cmake/rosidl_default_generatorsConfig.cmake:41 (include)
CMakeLists.txt:14 (find_package)
It looks like this is resolved in the master branch, but not a tagged release.
This change relies on in progress PRs which have been outstanding for a bit. Looking for guidance: @esteve - do you want to hold off making CI changes until the ROS actions are stable, or use these one offs and revisit when these changes are released?
This now builds:
name: Build (Desktop)
on: [push, pull_request]
jobs:
build-desktop:
runs-on: windows-latest
env:
VisualStudioVersion: '16.0'
steps:
- name: Checkout source
uses: actions/checkout@v2
- name: Setup ROS2
uses: ros-tooling/setup-ros@master
with:
required-ros-distributions: foxy
- uses: wmmc88/action-ros-ci@wmmc88/fix-windows-builds
with:
package-name: rcldotnet_examples
target-ros2-distro: foxy
vcs-repo-file-url: ${{github.workspace}}/.github/workflows/ci.repos
extra-cmake-args: -DCMAKE_SYSTEM_VERSION=10.0.19041.0 -Wno-dev
The PR for wmmc88's change is in master now - https://github.com/ros-tooling/action-ros-ci/pull/712. @esteve are you ok with having an untagged version for the action in the CI?
The PR for wmmc88's change is in master now - ros-tooling/action-ros-ci#712. @esteve are you ok with having an untagged version for the action in the CI?
@ooeygui sorry for the slow response, I missed your message. Thank you so much for tracking this issue, no problem with using an untagged version, though I assume 0.2.4, which was released two weeks ago should contain this fix. In any case, feel free to update the action and then we can move onto the other PRs. Thanks!
Hi @esteve, I'm sorry for the delayed reply, I'm currently on caregiver leave. I'll be out of office for a few more weeks, but I'll work finishing up this PR as time allows.
I've confirmed that the tagged versions work correctly. @esteve, I wanted to confirm your ask - do you want me to patch this PR with the following? OR do you want me to create a new PR?
name: Build (Desktop)
on: [push, pull_request]
jobs:
build-desktop:
runs-on: windows-2019
env:
VisualStudioVersion: '16.0'
steps:
- name: Checkout source
uses: actions/checkout@v3
- name: Setup ROS2
uses: ros-tooling/setup-ros@master
with:
required-ros-distributions: foxy
- uses: ros-tooling/action-ros-ci@0.2.4
with:
package-name: rcldotnet_examples
target-ros2-distro: foxy
vcs-repo-file-url: ${{github.workspace}}/.github/workflows/ci.repos
extra-cmake-args: -DCMAKE_SYSTEM_VERSION=10.0.19041.0 -Wno-dev
I'm sorry for the delayed reply, I'm currently on caregiver leave. I'll be out of office for a few more weeks, but I'll work finishing up this PR as time allows.
Sorry to hear that, I hope everything is ok and let me know if you'd like me to pick up this PR.
I've confirmed that the tagged versions work correctly. @esteve, I wanted to confirm your ask - do you want me to patch this PR with the following? OR do you want me to create a new PR?
That's awesome! Push to this PR is fine, thanks!
I'm going to iterate on the UWP builds on ms-iot so I don't pollute this change.
@esteve I've been working on the UWP CI. There are dependencies on work that I need to upstream from ms-iot in other repositories which prevent it from working in this repo until that work has been completed.
Disable the UWP CI build in ROS2.net
The Hololens 2 uses a version of Windows called WCOS, which is not quite Windows 10 and not quite Windows 11. It is also based on ARM64. The UWP build for Hololens 2 requires significant changes to ROS2 itself, as well as changes to dependencies such as FastDDS. This work was done in the ms-iot organization, and being prepared for upstreaming.
In order to unblock ROS2.net development, I'd like to disable the UWP CI until that other work can be up streamed. Once upstreamed, separate work can be done in this repo to reenable UWP.
@ooeygui thanks for looking into that. I'm checking the logs and it's rather weird, because ament
does not seem to find VisualStudio. Anyway, let's disable the UWP jobs for now. Do you have an estimate when the WCOS contributions will be upstreamed?
@esteve thanks for the reply. I'll patch this change to remove UWP.
I had been iterating on the UWP CI here - https://github.com/ms-iot/ros2_dotnet/actions/runs/2151382523. The first problem is that the github runner windows-latest
does not have Visual Studio 16 installed which is why it couldn't be found. When I replaced it with windows-2019
, other errors manifested which I had corrected in various branches.
I'm working through the process of upstreaming the following forks which have WindowsStore
support:
https://github.com/ms-iot/Fast-DDS.git
https://github.com/ms-iot/rcpputils.git
https://github.com/ms-iot/rcutils.git
https://github.com/ms-iot/libyaml_vendor.git
https://github.com/ms-iot/poco_vendor.git
Most are simple fixes, but first have to go into the main branches, before being cherry picked into the foxy branches. I don't have an estimate of the upstream velocity in each repo just yet.
@esteve Since we both made changes to this PR, I'd like to get final approval from you before pulling it.
@ooeygui thanks for the links. I can only say, good luck getting the fixes into Fast-DDS, it took them an entire year (literally, 365 days between since I created the PR until it got reviewed) to get my UWP changes merged (https://github.com/eProsima/Fast-DDS/pull/26) and then broke them again, so I had to fix the UWP build (https://github.com/eProsima/Fast-DDS/pull/247). You'd probably have a better experience and have more chances of getting UWP to work by switching to Cyclone DDS, additionally their codebase is simpler and better organized.
Anyway, let me know if you need reviewers for the ROS2 stuff, happy to chime in.
@esteve Since we both made changes to this PR, I'd like to get final approval from you before pulling it.
I can't approve this PR because I created it, but feel free to merge it :+1:
Posting as an FYI: I came across this project to run github actions locally for fast iteration - https://github.com/nektos/act. I'm looking into it for working with the ROS2 Github action.