search

OpenSimulationInterface / open-simulation-interface

A generic interface for the environmental perception of automated driving functions in virtual scenarios.
Other
267 stars 125 forks source link

Support more protobuf versions in GitHub workflow #766

Open ClemensLinnhoff opened 7 months ago

ClemensLinnhoff commented 7 months ago

#### Reference to a related issue in the repository

765

Add a description

Fix pipeline behavior with different protobuf versions

Take this checklist as orientation for yourself, if this PR is ready for the Change Control Board:

If you can’t check all of them, please explain why. If all boxes are checked or commented and you have achieved at least one positive review, you can assign the label ReadyForCCBReview!

ClemensLinnhoff commented 7 months ago

Protobuf 2.6.1 does not work out of the box, since it does not have '-all' in the link/package: https://github.com/OpenSimulationInterface/open-simulation-interface/actions/runs/7609941012

If '-all' is changed to '', it will download abseil, but cannot install it, since protobuf 2.6.1 has autotools (configure). So the if condition distinguishing between autotools and cmake is not correct: https://github.com/OpenSimulationInterface/open-simulation-interface/actions/runs/7609969004

Also, between 3.0.0 and 3.5.0 there is no package for all environments.

ClemensLinnhoff commented 7 months ago

With my changes, protobuf 2.6.1 can be installed. But now installing python fails: https://github.com/OpenSimulationInterface/open-simulation-interface/actions/runs/7610124644

ClemensLinnhoff commented 7 months ago

Protobuf versions >= 3.5.1 seem to work now.

Proto3 versions below don't work because of a compiler error. See for example https://github.com/OpenSimulationInterface/open-simulation-interface/actions/runs/7613993384/job/20735224384

Protobuf version 2.6.1 fails during the OSI setup. See https://github.com/OpenSimulationInterface/open-simulation-interface/actions/runs/7614005171/job/20735265068

ClemensLinnhoff commented 7 months ago

Proto3 versions below don't work because of a compiler error. See for example https://github.com/OpenSimulationInterface/open-simulation-interface/actions/runs/7613993384/job/20735224384

I fixed this by setting CXXFLAGS=-std=c++11 for configure.

Now, only protobuf 2.6.1 does not work.

ClemensLinnhoff commented 7 months ago

I tried again with protobuf 2.6.1. The problem occurs during building the python files form the .proto files.

When running protoc --python_out osi3/ osi3/osi_version.proto the following error occurs:

google/protobuf/descriptor.proto: File not found.

In osi_version.proto, google/protobuf/descriptor.proto is included. However I checked, and it is located in /usr/include and /usr/local/include. So I have no idea, why it is not found and why this only is an issue with protobuf 2.6.1 and not the other versions.

Has anyone ever used the combination osi - protobuf 2.6.1 - python? @pmai do you have any idea what might be the issue?

jdsika commented 7 months ago

Another sanity check: Do older versions of protobuffer also work with e.g. C++17? I would guess yes

ClemensLinnhoff commented 7 months ago

Another sanity check: Do older versions of protobuffer also work with e.g. C++17? I would guess yes

For the older versions I had to explicitly set a CXXFlag to c++11. But this works for all versions.

ClemensLinnhoff commented 6 months ago

I now managed to get the setup.py to work with protobuf 2.6.1. However, now a unit test fails. Apparently Protobuf 2.6.1 does not support Python 3...

My recommendation: Let's set the minimum proto version to 3.0.0, as it already says in the documentation anyways.

pmai commented 6 months ago

I now managed to get the setup.py to work with protobuf 2.6.1. However, now a unit test fails. Apparently Protobuf 2.6.1 does not support Python 3...

My recommendation: Let's set the minimum proto version to 3.0.0, as it already says in the documentation anyways.

At least officially protobuf states Python 3 support since 2.6.0... The error logs look more like there are changes in the location of the bundled proto include?

jdsika commented 6 months ago

I now managed to get the setup.py to work with protobuf 2.6.1. However, now a unit test fails. Apparently Protobuf 2.6.1 does not support Python 3... My recommendation: Let's set the minimum proto version to 3.0.0, as it already says in the documentation anyways.

At least officially protobuf states Python 3 support since 2.6.0... The error logs look more like there are changes in the location of the bundled proto include?

What about this here? https://github.com/protocolbuffers/protobuf/issues/882

jdsika commented 6 months ago

Considerations:

jdsika commented 4 months ago

@ClemensLinnhoff and @pmai this PR must be closed and we have to check if there is anything missing. See also my questions above. I think we are still inconsistent as the docs still mention proto version >3.0.0 and then my questions with the proto syntax arise!

ClemensLinnhoff commented 4 months ago

Python is now completely decoupled from the protobuf version. So the changes to setup.py are superseded. What is the status on proto 2 support for C++? Then we might need the changes. However, only the CI pipeline has minor changes.

pmai commented 3 months ago

Since this only affects our internal build pipeline, and we have no direct necessity to always build against multiple protobuf versions (we have not had any regressions due to PB changes, only our build pipeline is ever affected), I think this change is not release relevant, and we should look at this more carefully after the release (i.e. what really are our needs here, and how do we want to structure this; the past CI improvements were more for moving forwards). The danger that we inadvertantly blow up some part of the build or release process seems paramount.