search

moveit / geometric_shapes

Representation of geometric shapes
57 stars 92 forks source link

CMakeLists.txt: making package relocatable #230

Closed ghost closed 5 months ago

ghost commented 1 year ago

To make the package relocatable (i.e. no absolute paths in any .cmake file) some changes are needed. First of all, the usage of eigen3_cmake_module makes things worse, not better. Since QHull and assimp seem to have somewhat usable cmake files, switching over to use them.

The custom QHULL set a different include path then upstream Qhull::qhull_r does, so fixing that as well.

rhaschke commented 1 year ago

Your changes break CI. Can you please detail, which system requirements are needed by this PR? The current cmake config was probably designed for Ubuntu 20.04, which is for sure outdated by now. What exactly do you want to achieve by your changes, i.e. in which sense do you want to make the package relocatable?

ghost commented 1 year ago

Hi Robert! I tried to fix CI, the error is that a PR on eigen_stl_contianers is also pending. I am using yocto linux, since Ubuntu is not suitable to be used in a real product. We need it to be relocatable since we cross compile.

rhaschke commented 1 year ago

The error is that a PR on eigen_stl_contianers is also pending.

Can you please mention and link that in the main comment then?

I am using yocto linux. We need it to be relocatable since we cross compile.

Isn't the usual approach to use a chroot environment to avoid relocation issues? When the Ubuntu .debs are build, they don't install to /opt/ros directly as well...

To get this PR merged with such many basic changes to the build config, your changes need to pass CI flawlessly and you need to provide a very good motivation to convince two maintainers.

ghost commented 1 year ago

Hi Robert!

It was never easy to convince you of anything, was it?

Cross-compile does not need chroot, you do not need root privileges to cross compile. You basically setup a sysroot environment. The path is arbitrary, I have seen it leak into the cmake config files.

Well, since I cannot trigger the CI myself, it is kind of difficult for me to debug and improve the PR to a state that at least confirms to minimal QA requirements. And since you are making the point that you do not want it, I will close it. I just patch it on my end and be done with it.

Regards, Matthias

simonschmeisser commented 1 year ago

You should be able to enable github actions on your fork, maybe by adding a PR to your fork. Then please try to break thinks up into independent commits.

rhaschke commented 1 year ago

I didn't say anything about not wanting your changes. I just stated that a PR needs to pass CI without issues and, yes, I was assuming that you know how to configure GHA on your fork. So, I'm looking forward to your cleaned-up PR.