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

Support configuration against protobuf 3.11 on Windows #91

Merged

Conversation

traversaro
Copy link
Contributor

First of all, it is important to mention that the problem of not finding protoc on ROSOnWindowsis an upstream bug, reported in ms-iot/ROSOnWindows#218 .

Until now, the use of the HINTS parameter in find_package make the CMake use the installed protobuf-config.cmake instead of the CMake's provided FindProtobuf.cmake that was used on non-WIN32. Note that the actual value of the parameter passed to HINTS does not matter, actually on my ROSOnWindows setup ${_CMAKE_INSTALL_DIR}/tools/protobuf was actually the non-existing path C:\opt\python27amd64\Scripts\tools\protobuf.

Unfortunately, the variables and macros exported by the protobuf 3.11's protobuf-config.cmake are not compatible with the one used in the project, so the configuration was not working correctly when using a modern protobuf such as the one installed by vcpkg.

To have a CMake that is able to configure with protobuf and CMake installed in Ubuntu 16.04, againtt the protobuf installed by ROSOnWindows and also the latest protobuf installed by vcpkg,
we use also on Windows the default signature of find_package, and as a workaround for ms-iot/ROSOnWindows#218 we add to CMAKE_PROGRAM_PATH the C:/opt/rosdeps/x64/tools/protobuf if C:/opt/rosdeps/x64 is part of the the CMAKE_PREFIX_PATH . While the use of hardcoded absolute path may seems a error-prone strategy, considering that ROSOnWindows do not support being installed in a prefix different from C:/opt, it is a good way of detecting reliably if the project is being configured under ROSOnWindows .

Before this modification, the use of the HINTS parameter in find_package
make the CMake use the installed protobuf-config.cmake instead of
the CMake's provided FindProtobuf.cmake that was used on non-WIN32.
Unfortunatly, the variables and macros exported by the protobuf 3.11's
protobuf-config.cmake are not compatible with the one used in the project,
so also on Windows we use the default signature of find_package.
Copy link
Member

@gavanderhoorn gavanderhoorn left a comment

Choose a reason for hiding this comment

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

I can't test this myself, but I have read the linked issue(s) and I believe this will not impact the Linux side of things, so I don't expect any regressions.

Neither on Windows hosts.

@gavanderhoorn
Copy link
Member

Thanks @traversaro 👍

@gavanderhoorn gavanderhoorn merged commit 5cad6d5 into ros-industrial:master May 10, 2020
@gavanderhoorn
Copy link
Member

While the use of hardcoded absolute path may seems a error-prone strategy, considering that ROSOnWindows do not support being installed in a prefix different from C:/opt, it is a good way of detecting reliably if the project is being configured under ROSOnWindows .

Yes, I agree.

It's a work-around, but let's see what upstream does.

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

Successfully merging this pull request may close these issues.

None yet

2 participants