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

Feature/fix broken images #36

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

MSECode
Copy link
Member

@MSECode MSECode commented Dec 23, 2024

This PR add some fixes to the basic images.
Specifically we introduced a fix to make working the image superbuild-icubhead-withuser and the images that uses robometry.
Moreover we improved the workflows and clean the Dockerfile of the images removing building of appsAway repository that is not needed anymore and fixing some of the instructions to make the DockerFile cleaner.

Major changes made:

  • remove https://github.com/icub-tech-iit/appsAway.git from image build since not needed anymore
  • cleanup of arg variable
  • cleanup of superbuild tags keeping just the needed ones
  • check of multistage build
  • move superbuild-gazebo to stale images since it needs revision
  • removed YCM_EP_INSTALL_DIR
  • change superbuild installation folder to default folder robotology-superbuild/build/install

@MSECode MSECode requested a review from valegagge December 23, 2024 10:05
@MSECode MSECode self-assigned this Dec 23, 2024
@MSECode MSECode marked this pull request as draft December 23, 2024 10:57
@MSECode
Copy link
Member Author

MSECode commented Dec 24, 2024

Since I'm encountering other errors during the build process of the images that need to re-build the robotology-superbuild starting from a not empty cache of it, such as the image superbuild-icubhead-robometry that uses superbuild-icubhead-withuser as starting image, I'll keep this PR in draft until those errors gets cleaned.
Anyways, this problem needs to be investigated more considering that it is not just an issue in the building of the children images but you can have the same issue if you desire to run a container based on that image and you desire to re-build the superbuild, as an example when you need to test your code and then you need to build in devel mode.
In particular, the error I'm now talking about is shown by the screen below. I've encountered it while re-build the robotology-superbuild starting from the image superbuild-icubhead-withuser and enabling the option ROBOTOLOGY_ENABLE_DYNAMICS.
I've got the same exact issue either if setting CMAKE_INSTALL_PREFIX to the standard path: robotology-superbuild/build/install or to another custom directory. Thinking that it could have been a cmake version problem I've also tried to generate the base image not using cmake 3.22.1, which is the one given by ubuntu:jammy by default, but removing it and re-installing cmake and cmake-guid version 3.29.0 during the generation of the image. In both cases the outcome was the same.

image

@valegagge
Copy link
Member

Hi @MSECode ,
thanks for your contribution!
Can you summarize the major changes in bullet points hereafter?

In addition please take into account this issue #37 during your tests.

@MSECode MSECode force-pushed the feature/fixBrokenImages branch from e41c259 to c65e5d0 Compare January 13, 2025 10:33
@MSECode MSECode marked this pull request as ready for review January 13, 2025 11:49
@MSECode
Copy link
Member Author

MSECode commented Jan 13, 2025

Hi @MSECode , thanks for your contribution! Can you summarize the major changes in bullet points hereafter?

In addition please take into account this issue #37 during your tests.

Update the PR body

@MSECode MSECode force-pushed the feature/fixBrokenImages branch from c65e5d0 to 08b64a7 Compare January 13, 2025 13:55
Update superbuild base, withuser and robometry images to check fix on superbuidl with robometry compilation
Add superbuild-gazebo to stale images. It needs to be revised.
@MSECode MSECode force-pushed the feature/fixBrokenImages branch from 08b64a7 to 3ad7cdf Compare January 13, 2025 14:14
Copy link
Member

@valegagge valegagge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MSECode already tested the image. All changes are fine for me. Let's merge!

@valegagge valegagge merged commit 4392bba into icub-tech-iit:master Jan 13, 2025
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