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

Fix/colcon build #2000

Merged
merged 3 commits into from Feb 19, 2019
Merged

Fix/colcon build #2000

merged 3 commits into from Feb 19, 2019

Conversation

sgermanserrano
Copy link

Status

PRODUCTION / DEVELOPMENT

Description

As described in autowarefoundation/autoware_ai#563

@@ -9,7 +9,7 @@ RUN apt-get update && apt-get install -y \
libflann-dev \
libgsl0-dev \
libgoogle-perftools-dev \
libeigen3-dev
libeigen3-dev sudo gconf2
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great. Just a minor thing, could you break this down into separate lines and sort them alphabetically?

@@ -62,7 +66,7 @@ RUN sudo rosdep init \
# Install Autoware
RUN cd && mkdir /home/$USERNAME/Autoware
COPY --chown=autoware ./ /home/$USERNAME/Autoware/
RUN /bin/bash -c 'source /opt/ros/kinetic/setup.bash; cd /home/$USERNAME/Autoware/ros/src; git submodule update --init --recursive; catkin_init_workspace; cd ../; rosdep install -y --from-paths /home/$USERNAME/Autoware/ros/src --ignore-src --rosdistro kinetic; ./catkin_make_release'
RUN /bin/bash -c 'source /opt/ros/kinetic/setup.bash; cd /home/$USERNAME/Autoware/ros/src; git submodule update --init --recursive; cd ../; rosdep install -y --from-paths /home/$USERNAME/Autoware/ros/src --ignore-src --rosdistro kinetic; ./colcon_release'
RUN echo "source /home/$USERNAME/Autoware/ros/devel/setup.bash" >> /home/$USERNAME/.bashrc
Copy link
Member

Choose a reason for hiding this comment

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

needs to point to install space instead of devel

Suggested change
RUN echo "source /home/$USERNAME/Autoware/ros/devel/setup.bash" >> /home/$USERNAME/.bashrc
RUN echo "source /home/$USERNAME/Autoware/ros/install/setup.bash" >> /home/$USERNAME/.bashrc

@@ -62,7 +66,7 @@ RUN sudo rosdep init \
# Install Autoware
RUN cd && mkdir /home/$USERNAME/Autoware
COPY --chown=autoware ./ /home/$USERNAME/Autoware/
RUN /bin/bash -c 'source /opt/ros/kinetic/setup.bash; cd /home/$USERNAME/Autoware/ros/src; git submodule update --init --recursive; catkin_init_workspace; cd ../; rosdep install -y --from-paths /home/$USERNAME/Autoware/ros/src --ignore-src --rosdistro kinetic; ./catkin_make_release'
RUN /bin/bash -c 'source /opt/ros/kinetic/setup.bash; cd /home/$USERNAME/Autoware/ros/src; git submodule update --init --recursive; cd ../; rosdep install -y --from-paths /home/$USERNAME/Autoware/ros/src --ignore-src --rosdistro kinetic; ./colcon_release'
Copy link
Member

@amc-nu amc-nu Feb 19, 2019

Choose a reason for hiding this comment

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

I recommend using sudo -E in the script to allow the build arguments to be passed. This is useful when running behind a proxy.

Suggested change
RUN /bin/bash -c 'source /opt/ros/kinetic/setup.bash; cd /home/$USERNAME/Autoware/ros/src; git submodule update --init --recursive; cd ../; rosdep install -y --from-paths /home/$USERNAME/Autoware/ros/src --ignore-src --rosdistro kinetic; ./colcon_release'
RUN /bin/bash -c 'source /opt/ros/kinetic/setup.bash; cd /home/$USERNAME/Autoware/ros/src; git submodule update --init --recursive; cd ../; sudo -E rosdep install -y --from-paths /home/$USERNAME/Autoware/ros/src --ignore-src --rosdistro kinetic; ./colcon_release'

@sgermanserrano
Copy link
Author

@esteve @amc-nu the main change of the PR is on the CMakeLists for vision_dpm_ttic_detect pacakge. The changes in the Dockerfile will be replaced by PR #1946, I just made them to allow for testing when building the docker image with colcon

@sgermanserrano
Copy link
Author

@esteve @amc-nu I have undone the changes to the Dockerfile to keep the aim of the PR clean. The needed changes are mentioned in autowarefoundation/autoware_ai#563 in any case for testing purposes.

@sgermanserrano sgermanserrano merged commit c1166d9 into autowarefoundation:develop Feb 19, 2019
@sgermanserrano sgermanserrano deleted the fix/colcon_build branch February 19, 2019 11:20
@sgermanserrano sgermanserrano mentioned this pull request Feb 28, 2019
7 tasks
anubhavashok pushed a commit to NuronLabs/autoware.ai that referenced this pull request Sep 7, 2021
@mitsudome-r mitsudome-r added the version:autoware-ai Autoware.AI label Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants