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

Use --merge-install for colcon so that gz can be found during build #1063

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Nov 9, 2023

Our windows builds are not running UNIT_gz_TEST or similar tests because gz is not found at build time. This is because we are using isolated builds and colcon doesn't seem to expose the bin directories of each of the dependencies at build time. Using --merge-install fixes this and this is also what we've been recommending for our users (see https://gazebosim.org/docs/harmonic/install_windows_src#building-the-gazebo-libraries)

@azeey azeey requested a review from j-rivero as a code owner November 9, 2023 21:02
@azeey
Copy link
Contributor Author

azeey commented Nov 9, 2023

Test build: Build Status

@j-rivero
Copy link
Contributor

Let's discuss first if we are prepared to merge this since it can potentially trigger new test / new test failures all across the Windows builds.

@j-rivero j-rivero marked this pull request as draft November 10, 2023 11:58
@mjcarroll
Copy link
Contributor

This is because we are using isolated builds and colcon doesn't seem to expose the bin directories of each of the dependencies at build time.

If we preserve isolated installs, we can also generate colcon hooks to set the path. There may be some value in doing this to make sure that we aren't inadvertently depending on a merged install for one reason or another.

@azeey
Copy link
Contributor Author

azeey commented Nov 10, 2023

If we preserve isolated installs, we can also generate colcon hooks to set the path. There may be some value in doing this to make sure that we aren't inadvertently depending on a merged install for one reason or another.

The issue is that gz is not found by CMake during configuration. Do colcon hooks apply during build time?

@mjcarroll
Copy link
Contributor

Do colcon hooks apply during build time?

I thought that they did such that CMAKE_PREFIX_PATH/LD_LIBRARY_PATH, etc would be set correctly for dependencies, but I haven't verified it first hand.

It does have the disadvantage of adding a colcon-specific mechanism in this case, but the benefits may outweigh that.

@azeey
Copy link
Contributor Author

azeey commented Nov 10, 2023

Do colcon hooks apply during build time?

I thought that they did such that CMAKE_PREFIX_PATH/LD_LIBRARY_PATH, etc would be set correctly for dependencies, but I haven't verified it first hand.

It does have the disadvantage of adding a colcon-specific mechanism in this case, but the benefits may outweigh that.

So CMAKE_PREFIX_PATH/LD_LIBRARY_PATH appear to be set properly without any hooks. That's why the build succeeds. But I don't think PATH is being set. I'm guessing that only happens at runtime (e.g. colcon test), but I might be completely off.

@azeey
Copy link
Contributor Author

azeey commented Apr 19, 2024

Adding package.xmls fixes this problem because colcon knows how to add the correct paths based on the package.xml as opposed to scraping find_package calls. So maybe we can merge this once all the Harmonic package.xml PRs are in with green CI.

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