Skip to content

replace REQUIRE_GL with libgl-devel and REQUIRE_OPENGL with libopengl-devel#555

Merged
Tobias-Fischer merged 4 commits into
RoboStack:mainfrom
baszalmstra:opengl-devel
Oct 28, 2025
Merged

replace REQUIRE_GL with libgl-devel and REQUIRE_OPENGL with libopengl-devel#555
Tobias-Fischer merged 4 commits into
RoboStack:mainfrom
baszalmstra:opengl-devel

Conversation

@baszalmstra

Copy link
Copy Markdown
Contributor

@traversaro

Copy link
Copy Markdown
Member

I think we need to only add dependency on Linux only, as on macOS and Windows OpenGL is provided by the operating system. Furthermore, as libopengl-devel does not depend on libgl-devel, perhaps we may want to change REQUIRE_OPENGL with libgl-devel and libopengl-devel, not just libopengl-devel as just by adding libopengl-devel find_package(OpenGL REQUIRED) would fail.

@baszalmstra

Copy link
Copy Markdown
Contributor Author

Done! If you like this I can do the same thing for other distros.

@traversaro

Copy link
Copy Markdown
Member

I do not know if the "${{ \"libgl-devel\" if linux }}" syntax (a bit like the old REQUIRE_GL to be honest) is actually compatible with rosdep/bloom. Other dependencies do something like:

festival:
  robostack:
    linux: [festival, festvox-kallpc16k]
    osx: []
    win64: []

that is not ideal as non-linux dependencies are duplicated, and to be honest I am not 100% sure this still works in rosdep/bloom context.

@baszalmstra

Copy link
Copy Markdown
Contributor Author

Id be happy to just duplicate the dependencies. I noticed there is also robostack: ["${{ \"lttng-ust\" if linux }}"] which is why I added it.

@traversaro

Copy link
Copy Markdown
Member

Id be happy to just duplicate the dependencies. I noticed there is also robostack: ["${{ \"lttng-ust\" if linux }}"] which is why I added it.

This is a good point, but I suspect that is indeed a vinca-ism that would not work on rosdep or bloom.

@baszalmstra

Copy link
Copy Markdown
Contributor Author

I changed it back. This adds some duplication but should work for rosdep/bloom.

@baszalmstra

Copy link
Copy Markdown
Contributor Author

I see vinca also adds

- if: unix
  then: 
  - xorg-libx11
  - xorg-libxext

See https://github.com/RoboStack/vinca/blob/84494ae511217e1c8267b4d0d458969a53d588f6/vinca/main.py#L551

Should that also be added?

@traversaro

Copy link
Copy Markdown
Member

Thanks! I do not actually know if this actually works for rosdep/bloom, but that is indipendent of this PR. Ok for me if it is ok also for @Tobias-Fischer .

@traversaro

Copy link
Copy Markdown
Member

I see vinca also adds

- if: unix
  then: 
  - xorg-libx11
  - xorg-libxext

See https://github.com/RoboStack/vinca/blob/84494ae511217e1c8267b4d0d458969a53d588f6/vinca/main.py#L551

Should that also be added?

To be honest, I do not think this is the case. If those dependencies are actually required, it should be added for the dependencies that actually need them or the dependencies need to be added in some other way. The probblem is that we will not realize if that is the case until we do a full rebuild, but I think that is probably ok.

@Tobias-Fischer

Copy link
Copy Markdown
Collaborator

Amazing, thank you! Would be very happy to see the change mirrored in the other distros :)

@Tobias-Fischer Tobias-Fischer merged commit f05807a into RoboStack:main Oct 28, 2025
5 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.

3 participants