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 Protobuf and ODE as ExternalProjects to cmake to fix platform compatibility issues #182

Merged
merged 9 commits into from
Feb 24, 2024

Conversation

owlkward
Copy link

Builds Protobuf from source if the version is too new.
I'm not sure which protobuf version was the first to break, but if someone notices an incompatibility in the future they can just adjust the maximum allowed version.

For ODE I didn't find a way to automatically detect incompatibility, because it doesn't seem to be completely dependent on the version number, but could also depend on build configurations of the package, like enabling double precision or not.
e.g. if double precision is not enabled grSim crashes instantly, but with a different error than when using the Manjaro ode package.
So it ended up as an optional flag to enable building ODE from source.
Personally I'd make building ODE from source the default, but that seems more invasive than keeping the current behavior as default.

I don't know how GitHub Actions works, so I didn't test whether the pipeline works, yet.

Michel Schmid added 2 commits February 13, 2024 18:08
Since at least Protobuf 3.21 the version is incompatible causing various
include and linker errors. Maybe the incompatibility was introduced before that,
but I don't know. If that is the case the version number can just be set lower
than that in the future.
The approach is pretty much just copied from ER-Forces framework
(https://github.com/robotics-erlangen/framework) and adapted to fit the
structure of this repository.
On some systems the packaged ODE (e.g. the Manjaro ode package at time of
writing) causes grSim to crash almost instantly.
This *may* be caused by the package having been built without double precision
support, but I don't know what exactly breaks.
Additionally add this as a hint to INSTALL.md.
@g3force
Copy link
Member

g3force commented Feb 13, 2024

For GitHub Actions, you can configure the build here: https://github.com/RoboCup-SSL/grSim/blob/master/.github/workflows/build.yaml#L33

You can remove ode and protobuf from brew installation and instead set the cmake variable to use external ODE.

@owlkward
Copy link
Author

macos build should be fine now, I hope.
I also tried to fix the failing windows build by telling cmake where to look for the Qt5 it apparently isn't finding, but I'm not sure if that actually works to just add the vcpkg to CMAKE_PREFIX_PATH or if it needs to be the specific path to the Qt5 installation.

Michel Schmid added 3 commits February 14, 2024 04:30
Also changed vcpkg dependency from qt5 to qt5-base, because that should be
enough and vcpkg seems to build from source (?) and no one has time to build Qt
completely.
Includes some hacks to get around the fact that the macos pipeline does not set
the path to the QtConfig.cmake correctly.
@owlkward
Copy link
Author

nevermind, I was wrong, the dependencies on windows were not actually ever installed? Fixed that as well, I think.
Maybe windows works now, but I don't know yet, because the pipeline has been running for an hour, because apparently vcpkg compiles all dependencies from source and the dependency listed qt5 instead of qt5-base (I also changed that now, but I'm not going to wait until it finishes, I need sleep), which takes hours to compile even on a good machine.

macos now also definitely works (pipeline on my fork is successful for both macos versions), but it includes a hack for the PATH to the Qt5Config.cmake, which I'm not happy about, I have no idea on how paths on macos works and especially not on macos inside a github runner.

Michel Schmid added 3 commits February 15, 2024 16:53
Apparently some weird issues are caused by using the INSTALL_DIR parameter
of ExternalProjects when using the Ninja generator.
Makes you wonder why that is even there, but cmake works in mysterious ways,
I guess.
Adding the install part as a substep and explicitly telling cmake that the
necessary libs and executables are produced by this step fixes this issue.
Windows pipeline is now doing what the samples from run-vcpkg and run-cmake
suggest it should do and vcpkg claims that it should handle cmake find_package
calls correctly if you use their vcpkg.cmake as the toolchain.
It's not working, because for some reason the find_package call for Qt still
fails with cmake claiming it can't find a Qt5config.cmake file.
@owlkward
Copy link
Author

Apart from the stuff that should in theory fix the windows pipeline, but doesn't actually fix it, ODE now also gets built from source if the flag is not set, but only if find_package can't find it.

@owlkward
Copy link
Author

oh and this pull request should fix #176 and make #179 unnecessary

@g3force g3force merged commit aed1509 into RoboCup-SSL:master Feb 24, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants