👩‍🌾 Windows build failures: Unknown CMake command "PROTOBUF_GENERATE_CPP".

[chapulina]

Environment

• OS Version: Windows
• Source or binary build? source, branches off main (may also happen for ign-gazebo5 branches)

Description

• Expected behavior: CI passes on Windows
• Actual behavior:

CI has been flaky on Windows with this error:

CMake Error at src/msgs/CMakeLists.txt:1 (PROTOBUF_GENERATE_CPP):
  Unknown CMake command "PROTOBUF_GENERATE_CPP".

I'm not sure yet what's causing the issue. See for example these 2 builds with the same setup, but different results:

• 
  • From Visualize joints #961
  • That commit has set(protobuf_MODULE_COMPATIBLE TRUE)
  • Ran on the windows_mad node
• 
  • From Added Spawn plugin #983
  • That commit has set(protobuf_MODULE_COMPATIBLE TRUE)
  • Ran on the windows_mad node

Steps to reproduce

Run Windows CI a few times for branches off main or ign-gazebo5

---

https://github.com/osrf/buildfarmer/issues/224

[Blast545]

I took some time investigating this. I've compared the build logs for these two windows jobs:

ign_gazebo-ign-5-win#35 (Failing with Unknown CMake command "PROTOBUF_GENERATE_CPP")
ign_gazebo-ign-5-win#36 (Building normally)

The PROTOBUF_GENERATE_CPP issue was addressed with: ign-gazebo#957. Some context of the original issue: Cmake can't find protobuf, StackOverflow

However, the actual new issue is the following:

-- Searching for dependencies of ignition-transport10
CMake Warning (dev) at C:/vcpkg/installed/x64-windows/share/protobuf/protobuf-options.cmake:6 (option):
  Policy CMP0077 is not set: option() honors normal variables.  Run "cmake
  --help-policy CMP0077" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  For compatibility with older versions of CMake, option is clearing the
  normal variable 'protobuf_MODULE_COMPATIBLE'.
Call Stack (most recent call first):
  C:/vcpkg/installed/x64-windows/share/protobuf/protobuf-config.cmake:2 (include)
  C:/vcpkg/installed/x64-windows/share/protobuf/vcpkg-cmake-wrapper.cmake:13 (_find_package)
  C:/vcpkg/scripts/buildsystems/vcpkg.cmake:506 (include)
  C:/Jenkins/workspace/ign_gazebo-ign-5-win/ws/install/ignition-cmake2/share/cmake/ignition-cmake2/cmake2/FindIgnProtobuf.cmake:29 (find_package)
  C:/vcpkg/scripts/buildsystems/vcpkg.cmake:555 (_find_package)
  C:/Jenkins/workspace/ign_gazebo-ign-5-win/ws/install/ignition-transport10/lib/cmake/ignition-transport10/ignition-transport10-config.cmake:92 (find_package)
  C:/vcpkg/scripts/buildsystems/vcpkg.cmake:555 (_find_package)
  C:/Jenkins/workspace/ign_gazebo-ign-5-win/ws/install/ignition-cmake2/share/cmake/ignition-cmake2/cmake2/IgnUtils.cmake:189 (find_package)
  CMakeLists.txt:57 (ign_find_package)

The protobuf_MODULE_COMPATIBLE variable is set, but apparently CMake 3.10 does not support this behavior (setting option variables policy) and ignores this option. I think this is flaky because windows CI seems to be using different CMake versions in some builds (I'll take a look into this).

Based on previous discussions, I imagine one of the possible two fixes:

1. Explictly set the policy to enable setting that variable. That is adding this line of code: set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)
2. Change FindIgnProtobuf.cmake to find the Protobuf package in MODULE mode instead of CONFIG.

I'm not a CMake/Protobuf expert so I don't know if any of those could have undesired side effects. I can help opening a PR for any of these two options if someone else thinks one of those could solve the issue.

[chapulina]

Thank you for the investigation @Blast545 ! I suggest a 3rd alternative: consider that our minimal required version is CMake 3.10, and make sure that all CI is run with higher versions.

[Blast545]

@chapulina I think in any case I will iterate with the CI team to ensure the build process is the same consistently.

In terms of the error, I think the cmake_minimum_required should imply that the build won't fail if an user uses that CMake version. As the set policy was added in 3.13, this would mean users building with CMake 3.10, 3.11 and 3.12 will have this problem.

I might be wrong about the implications of cmake_minimum_required, I would say we should at least document it somewhere.

[chapulina]

Ah sorry I misread your comment about the version.

Explictly set the policy to enable setting that variable

Ok, I get it now, this sounds like a good option

[chapulina]

It looks like #1046 didn't do it, we're still getting these failures on main.

[Blast545]

Based on the logs, it seems like there's same policy is required as part of ign-transport because there the CMake file tries to find protobuf as well.

https://github.com/ignitionrobotics/ign-transport/blob/0a813ecf9b2c2153ca7b8aa24ad66fdffad487a4/CMakeLists.txt#L39

I'll open a PR there as well.

[Blast545]

I'll take some time to reproduce this error locally consistently before opening a new fix PR though.

[Blast545]

Information about the problem itself:

It occurs only for win10-nuc: in the buildfarm. It's a flaky failure.

After trying reading more about CMake and these similar issues:
https://gitlab.kitware.com/cmake/cmake/-/issues/20312
https://unix.stackexchange.com/questions/512681/how-do-i-set-a-cmake-policy

I attempted to fix with #1168 and gazebosim/gz-transport#277. However, the policy set on the main ign-gazebo project doesn't seem to affect the protobuf option configure here: https://github.com/protocolbuffers/protobuf/blob/d1eca4e4b421cd2997495c4b4e65cea6be4e9b8a/cmake/protobuf-options.cmake#L6

However, the variable is cleared after the build process reaches this point: https://github.com/protocolbuffers/protobuf/blob/3.13.x/cmake/protobuf-options.cmake#L6

Causing this error:

---
--- stderr: ignition-gazebo6
CMake Warning (dev) at C:/vcpkg/installed/x64-windows/share/protobuf/protobuf-options.cmake:6 (option):
  Policy CMP0077 is not set: option() honors normal variables.  Run "cmake
  --help-policy CMP0077" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  For compatibility with older versions of CMake, option is clearing the
  normal variable 'protobuf_MODULE_COMPATIBLE'.
Call Stack (most recent call first):
  C:/vcpkg/installed/x64-windows/share/protobuf/protobuf-config.cmake:2 (include)
  C:/vcpkg/installed/x64-windows/share/protobuf/vcpkg-cmake-wrapper.cmake:13 (_find_package)
  C:/vcpkg/scripts/buildsystems/vcpkg.cmake:506 (include)
  C:/Jenkins/workspace/ign_gazebo-pr-win/ws/install/ignition-cmake2/share/cmake/ignition-cmake2/cmake2/FindIgnProtobuf.cmake:29 (find_package)
  C:/vcpkg/scripts/buildsystems/vcpkg.cmake:555 (_find_package)
  C:/Jenkins/workspace/ign_gazebo-pr-win/ws/install/ignition-transport11/lib/cmake/ignition-transport11/ignition-transport11-config.cmake:92 (find_package)
  C:/vcpkg/scripts/buildsystems/vcpkg.cmake:555 (_find_package)
  C:/Jenkins/workspace/ign_gazebo-pr-win/ws/install/ignition-cmake2/share/cmake/ignition-cmake2/cmake2/IgnUtils.cmake:189 (find_package)
  CMakeLists.txt:63 (ign_find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Error at src/msgs/CMakeLists.txt:1 (PROTOBUF_GENERATE_CPP):
  Unknown CMake command "PROTOBUF_GENERATE_CPP".

[traversaro]

Instead of dealing with tricky CMake policies, did you considered setting the protobuf_MODULE_COMPATIBLE variable in the CACHE, i.e. :

set(protobuf_MODULE_COMPATIBLE ON CACHE BOOL "")

?

Not sure if this fixes the issue, but it avoids the need of dealing with CMake policies.

[Blast545]

As for now I think there could be 2 possible solutions:

1. Update win-windows_nuc.win10 to match the build environment of win-beefy.win10. The latter has a newer version of vcpkg and protobuf and I imagine that's the reason why this error doesn't appear there.
2. Modify the colcon build script here to include the policy in the command line
   -DCMAKE_POLICY_DEFAULT_CMP0077=NEW I think this would include the policy for the dependencies of ign-gazebo and address the problem.

[Blast545]

did you considered setting the protobuf_MODULE_COMPATIBLE variable in the CACHE

@traversaro I didn't consider it. Does it make a difference when the build process reaches https://github.com/protocolbuffers/protobuf/blob/3.13.x/cmake/protobuf-options.cmake#L6 if the variable is defined in the cache?

I can give it a try.

[traversaro]

@traversaro I didn't consider it. Does it make a difference when the build process reaches https://github.com/protocolbuffers/protobuf/blob/3.13.x/cmake/protobuf-options.cmake#L6 if the variable is defined in the cache?

I may be wrong, but I think that if protobuf_MODULE_COMPATIBLE is in the cache, the option command does nothing (so it does not change the value of protobuf_MODULE_COMPATIBLE) regardless of the value of CMP0077, but I may be wrong.

[chapulina]

It occurs only for win10-nuc: in the buildfarm

I'm seeing it failing on beefy too now 😕

https://build.osrfoundation.org/job/ign_gazebo-pr-win/3364/console

[Blast545]

@traversaro I updated #1168 with your suggestions and still failed:
https://build.osrfoundation.org/job/ign_gazebo-pr-win/3366/

[Blast545]

I'm seeing it failing on beefy too now 😕

@chapulina beefy had a version of vcpkg that didn't match the expected 2020.11 version set here: https://github.com/ignition-tooling/release-tools/blob/6a5f89b80af764c38293acc0a2ee604f03ef2dc7/jenkins-scripts/lib/windows_env_vars.bat#L18 and was silently failing the checks of the script.
I asked @j-rivero to update beefy with vcpkg 2020.11. After checking the log of the job you sent me I can confirm vcpkg has now vcpkg 2020.11.

@j-rivero Do you think it would make sense to instead update the nodes and batch scripts to use vcpkg 2021.05.12? (the one that beefy had before). I didn't see any issues with beefy for ign-gazebo, but I don't have context as to why the vcpkg was tied to that particular version in the batch scripts.

[chapulina]

I asked @j-rivero to update beefy with vcpkg 2020.11. After checking the log of the job you sent me I can confirm vcpkg has now vcpkg 2020.11.

The issue just happened on beefy again 45 mins ago:

https://build.osrfoundation.org/job/ign_gazebo-pr-win/3369

[j-rivero]

@j-rivero Do you think it would make sense to instead update the nodes and batch scripts to use vcpkg 2021.05.12?

Umm, that would change all the base dependencies of the whole family of Ignition libraries. Seems like it was in that state from before without major problems until the date but old versions of Ignitions could potentially fail since the platform is totally new.

It might be have some sense to define a custom vcpkg snapshot for every Ignition collection, but that is something we don't have support for, at least now.

We could try running some experiments with the Ignition versions supported, the https://build.osrfoundation.org/job/_vcpkg_testing_snapshot/ was designed for that. It requires win_testing label in a node. It will run sha ref of ignition-gazebo. A possible path forward would be to verify that all major versions of ignition-gazebo currently supported are fine running on several builds of that job.

The other way forward would be manually patch vcpkg 2020.11 to include cmake protobuf module from 2021.05.12.

[Blast545]

@j-rivero Which option do you think would be a a better solution for the problem? I'd be glad to help you moving this forward.

We could try running some experiments with the Ignition versions supported,

If we only have to test for the ign-gazebo jobs I'm ok with this solution, however I would say we'd have to test with at least other packages like ign-rendering.

The other way forward would be manually patch vcpkg 2020.11 to include cmake protobuf module from 2021.05.12.

I like this one better, but I don't know if it's enough to fix the problem. I'm worried it could be another change with the newer vcpkg fixing the protobuf issue, like something in the cmake toolchain configuration. I think it's worth giving a try though.

[j-rivero]

@j-rivero Which option do you think would be a a better solution for the problem? I'd be glad to help you moving this forward.

Hard to say. First one would roll the ball forward and put us closer to nowadays package versions. Second one is more conservative. @chapulina what do you think?

We could try running some experiments with the Ignition versions supported,

If we only have to test for the ign-gazebo jobs I'm ok with this solution, however I would say we'd have to test with at least other packages like ign-rendering.

Oh sorry, forgot to mention: the colcon workspace will build everything up to ign-gazebo including rendering, gui and all the friends.

The other way forward would be manually patch vcpkg 2020.11 to include cmake protobuf module from 2021.05.12.

I like this one better, but I don't know if it's enough to fix the problem. I'm worried it could be another change with the newer vcpkg fixing the protobuf issue, like something in the cmake toolchain configuration. I think it's worth giving a try though.

Yep, this a more stable solution but could be hard to get if simply copying the protobuf module is not enough.

[chapulina]

@chapulina what do you think?

I say let's do whatever is quicker. We're using vcpkg temporarily (gazebo-tooling/release-tools#373), so I think we shouldn't be putting too much effort into it.

[Blast545]

Thanks for all the feedback on this, this is no longer a problem showing in the buildfarm.