Optimized dockerfile #54
Conversation
- Made build time quicker - Switch to rolling - Used drake-noble-1.30.0
|
Starting with the MoveIt dockerfile certainly optimizes out all those builds, which is great. Will let @sjahr determine whether we want this to be on Rolling or one of the LTS distros though. |
|
For distro issue, i wanna say that project uses main branch, but compiled with humble. I think this can create some problems in the future. Btw sorry for closing pr, it was a mistake. |
| @@ -0,0 +1,9 @@ | |||
| repositories: | |||
There was a problem hiding this comment.
There already is a moveit_drake.repos file at the top level. Can probably just keep one of these 2 files.
There was a problem hiding this comment.
Actually inspired from upstream.repos of moveit2_tutorials. While installing via docker, we don't need to clone repositories like moveit2 (they already installed in MoveIt's docker image) . In case of local installation, user need to clone additional repositories in addition of upstream.repos. Shortly, i have tried to support both local installation method and docker installation. But if you don't mind, we can only update main moveit_drake.repos
There was a problem hiding this comment.
Yes this seems to be the likely way to go forward. I am in favor of keeping both for now. This gives users the option of doing a single VCS import if working locally.
Could you add some instructions on README, under the local installation section to point users to this effect? May also be beneficial to specify that the build time is large in this case.
There was a problem hiding this comment.
Nice, thank you @CihatAltiparmak! I think enabling both, docker and local installation is a great idea and I think using rolling would be a good idea too, given that moveit main is also on rolling and humble support might be dropped in the future. Still need to test the docker & @kamiradi do you mind reviewing the docker code since you created it in the first place?
There was a problem hiding this comment.
Thanks for this addition @CihatAltiparmak. Pending local test on my machine and a readme instruction under local installation, LGTM.
In terms of versioning, I think we can make the decision when we make the first release? Having an LTS release and a rolling release will be helpful. @sea-bass @sjahr you guys are more experienced here.
| @@ -0,0 +1,9 @@ | |||
| repositories: | |||
There was a problem hiding this comment.
Yes this seems to be the likely way to go forward. I am in favor of keeping both for now. This gives users the option of doing a single VCS import if working locally.
Could you add some instructions on README, under the local installation section to point users to this effect? May also be beneficial to specify that the build time is large in this case.
There was a problem hiding this comment.
I have built the container on my machine. This has definitely reduced the build time. Nice work @CihatAltiparmak. I'll merge this once you haved added the Readem local install instrcution and removed the drake debian from the workspace.
|
Cool, i've applied all of your suggetions, @kamiradi . Btw now maybe you can consider to bump drake version because i believe with switching to ubuntu noble, it's simpler than before. Unfortunatelly, seems with this PR, we cannot support humble at the moment owing to the fact that humble branch of moveit doesnt run properly with this project (gives compilation error). FYI |
|
You guys could consider taking in the Drake version and the ROS Distro as arguments to Docker build. That would give flexibility. |
| LABEL maintainer="Aditya Kamireddypalli kamireddypalliaditya@gmail.com" | ||
| ARG ROS_DISTRO=rolling | ||
| ARG RMW_IMPLEMENTATION=rmw_cyclonedds_cpp | ||
| ARG DRAKE_VERSION=1.30.0-1 |
There was a problem hiding this comment.
I guess these are default values, maybe give it through docker compose?
There was a problem hiding this comment.
Overall done it with a couple of details: I couldn't build docker image without setting default argument inside dockerfile. So i just have updated docker-compose yaml instead of changing dockerfile as well. Now we can change args through compose file.
Hello, i dunno you are interested in optimizing dockerfile but i think this is more suitable way to serve demo. In this changes, i have installed drake via apt packages. But with some major changes
I have tried and seems it works. Do you have a chance to give a try?