ROS 2 Bouncy: Bionic + colcon source dockefile#128
ROS 2 Bouncy: Bionic + colcon source dockefile#128mikaelarguedas wants to merge 10 commits intomasterfrom
Conversation
c037578 to
afc1c25
Compare
| --event-handler console_cohesion+ | ||
|
|
||
| # setup bashrc | ||
| ENV ROS2_WS /root/ros2/ros2_ws |
There was a problem hiding this comment.
You're totally right.
Sorry for wasting your review time, this is currently playground and far from being ready for review. I just pushed a rebased version because someone else needed to do some debugging on bionic.
I will open a PR once this branch is closer to a reviewable state
There was a problem hiding this comment.
If you prefer I can do my playground on a fork in the future to avoid spurious email notifications ?
There was a problem hiding this comment.
Sorry for wasting your review time
no, not at all, I like following what going on here.
carry on with the WIP, ping when ready...
| qtbase5-dev \ | ||
| uncrustify \ | ||
| wget \ | ||
| xorg-dev \ |
There was a problem hiding this comment.
What needs xorg-dev? Is this for rviz_ogre_vendor? It built fine for me after only installing libxrandr-dev.
There was a problem hiding this comment.
Thanks for checking that. Yeah narrowing down what's needed to build and run rviz deps and tests is the last remaining item for the bionic migration. Once confirmed on the ros2/ci PR we can push the corresponding changes here
| RUN wget https://raw.githubusercontent.com/colcon/colcon.readthedocs.org/master/colcon.repos && \ | ||
| vcs import src < colcon.repos | ||
| RUN ./src/colcon-core/bin/colcon build --paths src/* | ||
| RUN . install/prefix.sh && colcon build |
There was a problem hiding this comment.
Master is now setup.sh instead of prefix.sh. I don't think @dirk-thomas has done a release with that change yet, but when he does how about using pip instead of bootstraping?
There was a problem hiding this comment.
yes I have these changes locally but haven;t pushed them yet as this is not ready for review. Do you need an up-to-date working version pushed here ?
There was a problem hiding this comment.
but when he does how about using pip instead of bootstraping?
As I use this for development, I bootstrap it for now because I almost always need features that are not yet released. We can definitely move to pip when this is more stable and thee is a one-line update path for users
| ENV LANG C.UTF-8 | ||
| ENV LC_ALL C.UTF-8 | ||
|
|
||
| RUN apt-get update && apt-get install -q -y tzdata \ |
There was a problem hiding this comment.
Do any packages besides tzdata ask for user input when they're being configured? If not I think the ENV DEBIAN_FRONTEND noninteractive line above could be removed by making this line RUN DEBIAN_FRONTEND=noninteractive apt-get update && ...
There was a problem hiding this comment.
Especially as currently dockerfile environment variables can't just be unset later by end users, and passing them inline seems to be a common compromise.
There was a problem hiding this comment.
Yeah totally, I also have that change locally but waited for the final state of the ros2/ci PR to be merged to push all the changes to that branch
|
|
||
| # setup ros2 environment | ||
| source "$ROS2_WS/install_isolated/local_setup.bash" | ||
| source "$ROS2_WS/install/prefix.bash" |
|
closing in favor of #160 |
WIP: this is experimental to prepare the bouncy release
main changes: