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 Windows builds to CI #92

Closed
gavanderhoorn opened this issue May 10, 2020 · 8 comments · Fixed by #101
Closed

Add Windows builds to CI #92

gavanderhoorn opened this issue May 10, 2020 · 8 comments · Fixed by #101
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@gavanderhoorn
Copy link
Member

As the library does support Windows (see also #44, #70 and #91), it would be good to add Windows builds to the CI configuration.

industrial_ci will probably not be able to help, but we could look at other ROS (ROS2?) packages which have Windows builds as part of their CI and mimic their setup.

@gavanderhoorn gavanderhoorn added enhancement New feature or request help wanted Extra attention is needed labels May 10, 2020
@gavanderhoorn
Copy link
Member Author

I really don't use ROS on Windows, so this would not be something I would take on, unless I can more-or-less copy a configuration from somewhere else.

@traversaro
Copy link
Contributor

traversaro commented May 10, 2020

Related docs on CI for ROS on Windows:

For non-ROS compilation, as mentioned in #70 (comment) it would be great to have a job for checking that case, it should be sufficient to have a simple GitHub Action like:

name: vcpkg Workflow

on:
  push:
  pull_request:
  schedule:
  # * is a special character in YAML so you have to quote this string
  # Execute a "nightly" build at 2 AM UTC 
  - cron:  '0 2 * * *'

jobs:
  build:
    name: 'vcpkg job'
    runs-on: windows-latest

    steps:
    - uses: actions/checkout@master
        
    - name: Install vcpkg Dependencies
      shell: bash
      run: |
        ${VCPKG_INSTALLATION_ROOT}/vcpkg.exe install --triplet x64-windows boost-asio boost-math boost-smart-ptr protobuf 

    - name: Build the project 
      shell: bash
      run: | 
        mkdir  build
        cd build
        cmake -DCMAKE_TOOLCHAIN_FILE=${VCPKG_INSTALLATION_ROOT}/scripts/buildsystems/vcpkg.cmake ..
        cmake --config Release --build .
        cmake --install .

For the non-ROS vcpkg case, I would be happy to open a PR.

@gavanderhoorn
Copy link
Member Author

For the non-ROS vcpkg case, I would be happy to open a PR.

We can setup Actions here on this repository.

If you could submit a PR, that would be great.

@gavanderhoorn
Copy link
Member Author

@traversaro: there is no real pressing need, but would you happen to have had a chance to take a look at the Windows CI setup?

@traversaro
Copy link
Contributor

@traversaro: there is no real pressing need, but would you happen to have had a chance to take a look at the Windows CI setup?

Still on my todo list, Latest update was in #93, I did not investigate in details but it seems that the latest protobuf and/or CMake has some kind of problem in the producing the correct path of the generated headers when the .proto file is not in the same directory of the CMake script that invokes protobuf_generate_cpp . Probably we could bypass this problem by using ROSOnWindows also for CI (that ships a rather old protobuf version), but for our use the long term goal is to have abb_libegm that works fine with vcpkg provided dependencies.

@gavanderhoorn
Copy link
Member Author

it seems that the latest protobuf and/or CMake has some kind of problem in the producing the correct path of the generated headers when the .proto file is not in the same directory of the CMake script that invokes protobuf_generate_cpp

Hm.

I seem to remember running into this myself.

We could create a sub CMakeLists.txt inside the proto dir, if that would help?

@traversaro
Copy link
Contributor

We could create a sub CMakeLists.txt inside the proto dir, if that would help?

Actually that is explicitly not supported by FindProtobuf.cmake (unless the proto generated files are not moved in their own library, but I am not sure it is desirable), see https://cmake.org/cmake/help/v3.15/module/FindProtobuf.html :

The protobuf_generate_cpp and protobuf_generate_python functions and add_executable() or add_library() calls only work properly within the same directory.

@gavanderhoorn
Copy link
Member Author

Hm. Blah.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Development

Successfully merging a pull request may close this issue.

2 participants