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.

Sign up for GitHub

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

Support ROS-independent CI Builds #818

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Support ROS-independent CI Builds #818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d99bc6e
Updated scripts to support ROS-independent builds
marip8 Mar 2, 2023
2f4c8ab
Updated documentation
marip8 Mar 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ Required environment variables:

* ``ROS_DISTRO``: Version of ROS in all lower case. E.g.: ``indigo``. If it is set in the custom Docker (base) image, it might be omitted in the script call.

* Note: a ROS-independent CI build can be executed if ``ROS_DISTRO=false``. In this case ``/opt/ros/<distro>`` will not be sourced or used as the underlay during the build. This can be useful for projects that are used in the ROS ecosystem but do not have an explicit dependency on ROS.

Optional environment variables
++++++++++++++++++++++++++++++++

Expand Down
3 changes: 3 additions & 0 deletions industrial_ci/src/isolation/docker.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,16 @@ PYLINT_ARGS
PYLINT_CHECK
PYLINT_EXCLUDE
PYTHONUNBUFFERED
PYTHON_VERSION_NAME
ROSDEP_SKIP_KEYS
ROSINSTALL_FILENAME
ROS_DISTRO
ROS_PYTHON_VERSION
ROS_REPO
ROS_REPOSITORY_KEY
ROS_REPOSITORY_PATH
ROS_VERSION
ROS_VERSION_EOL
TARGET_CMAKE_ARGS
TARGET_REPO_NAME
TARGET_REPO_PATH
Expand Down
11 changes: 9 additions & 2 deletions industrial_ci/src/isolation/docker.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,15 @@ function ici_isolate() {
fi

if [ -z "${ROS_DISTRO:-}" ]; then
ROS_DISTRO=$(docker image inspect --format "{{.Config.Env}}" "${DOCKER_IMAGE}" | grep -o -P "(?<=ROS_DISTRO=)[a-z]*") || ici_error "ROS_DISTRO is not set"
elif [ "${ROS_DISTRO}" = "false" ]; then
# Attempt to extract ROS_DISTRO from the docker image
ROS_DISTRO=$(docker image inspect --format "{{.Config.Env}}" "${DOCKER_IMAGE}" | grep -o -P "(?<=ROS_DISTRO=)[a-z]*")
if [ -z "${ROS_DISTRO:-}" ]; then
ici_log "Could not extract environment variable 'ROS_DISTRO' from the docker image '${DOCKER_IMAGE}'; proceeding without 'ROS_DISTRO'"
unset ROS_DISTRO
else
ici_log "Extracted 'ROS_DISTRO=${ROS_DISTRO}' from docker image '${DOCKER_IMAGE}'"
fi
elif [ "${ROS_DISTRO:-}" = "false" ]; then
unset ROS_DISTRO
fi

Expand Down
5 changes: 4 additions & 1 deletion industrial_ci/src/ros.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ function _set_ros_defaults {
_ros2_defaults "jammy"
;;
"false")
export BUILDER=${BUILDER:-colcon}
export ROS_VERSION=0
export ROS_VERSION_EOL=false
export ROS_PYTHON_VERSION=${ROS_PYTHON_VERSION:-3}
Comment on lines +91 to +94
Copy link
Member

Choose a reason for hiding this comment

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

Those are questionable defaults.

Copy link
Member Author

@marip8 marip8 Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What are your suggested alternatives? Python2 is EOL, so Python3 seems reasonable. colcon is the only builder I am familar with that can build code such that a downstream ROS1 or ROS2 workspace can use it. The ROS version cannot be EOL since this is for a ROS-independent build. ROS_VERSION is required by other parts of ICI, so anything < 1 seems okay

unset ROS_DISTRO
;;
*)
Expand All @@ -100,7 +104,6 @@ function _set_ros_defaults {
elif [ "$ROS_PYTHON_VERSION" = 3 ]; then
export PYTHON_VERSION_NAME=python3
fi

}

function _use_snapshot() {
Expand Down
2 changes: 1 addition & 1 deletion industrial_ci/src/tests/source_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ function run_source_tests {

ici_step "setup_rosdep" ici_setup_rosdep

extend=${UNDERLAY:?}
extend=${UNDERLAY:-}
Copy link
Member

Choose a reason for hiding this comment

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

This knocks out one of the sanity tests.


if [ -n "$UPSTREAM_WORKSPACE" ]; then
ici_with_ws "$upstream_ws" ici_build_workspace "upstream" "$extend" "$upstream_ws"
Expand Down
6 changes: 3 additions & 3 deletions industrial_ci/src/workspace.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ function ici_setup_rosdep {

if [ "$ROS_DISTRO" = "indigo" ] || [ "$ROS_DISTRO" = "jade" ]; then
ici_apt_install "ros-$ROS_DISTRO-roslib"
else
elif [ -n "${ROS_DISTRO:-}" ]; then
ici_apt_install "ros-$ROS_DISTRO-ros-environment"
fi

Expand All @@ -331,7 +331,7 @@ function ici_setup_rosdep {
fi

update_opts=()
if [ -z "${ROSDISTRO_INDEX_URL:-}" ]; then
if [ -z "${ROSDISTRO_INDEX_URL:-}" ] && [ -n "$ROS_DISTRO" ]; then
update_opts+=(--rosdistro "$ROS_DISTRO")
fi
if [ "$ROS_VERSION_EOL" = true ]; then
Expand Down Expand Up @@ -406,7 +406,7 @@ function ici_source_setup {
local extend=$1; shift
if [ ! -f "$extend/setup.bash" ]; then
if [ "$extend" != "/opt/ros/$ROS_DISTRO" ]; then
ici_error "'$extend' is not a devel/install space"
ici_log "'$extend' is not a devel/install space; skipping source"
Copy link
Member

Choose a reason for hiding this comment

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

Again, this is an important sanity check to test against misspelled parameters.

fi
else
ici_with_unset_variables source "$extend/setup.bash"
Expand Down