RSDK-9077: Docker containers for module development #310
Conversation
jckras
left a comment
jckras
left a comment
There was a problem hiding this comment.
I am still learning about docker and development environments so I left a few questions for my own understanding. Otherwise LGTM
| run: | | ||
| docker build -t cpp . -f etc/docker/tests/Dockerfile.debian.bullseye | ||
| docker run -e BUILD_SHARED=${{ matrix.BUILD_SHARED }} --rm -i -w /tmp cpp /bin/bash | ||
| docker build -t cpp . -f etc/docker/base-images/Dockerfile.debian.bullseye |
There was a problem hiding this comment.
is this creating a base image if you don't already have one? otherwise what is this docker build doing compared to the one below it/why do we need two?
There was a problem hiding this comment.
yeah this is creating the base image which the step below then uses to build the SDK. The idea is to have a modular, many-to-one type scenario where we can have a bunch of base images (the ones in base-images) which, if built, can then be used as input to a single "build the sdk on top of a base" image. from what I found researching this requires having stuff in separate Dockerfiles which are in turn built with separate invocations of docker build
| @@ -0,0 +1,47 @@ | |||
| # This Dockerfile is meant to be built on top of an existing image from the base-images directory. | |||
| # Suggested usage, from the SDK root: | |||
There was a problem hiding this comment.
I'm not sure if I understand correctly but "Suggested commands to build and run from the SDK root" might be more clear here?
| cmake .. -G Ninja \ | ||
| -DCMAKE_BUILD_TYPE=${BUILD_TYPE} \ | ||
| -DBUILD_SHARED_LIBS=${BUILD_SHARED} \ | ||
| -DVIAMCPPSDK_OFFLINE_PROTO_GENERATION=ON \ | ||
| -DVIAMCPPSDK_BUILD_TESTS=${BUILD_TESTS} \ | ||
| -DVIAMCPPSDK_BUILD_EXAMPLES=${BUILD_EXAMPLES} \ | ||
| ${EXTRA_CMAKE_ARGS} |
There was a problem hiding this comment.
why are we setting this build flags here instead of the run.sh file like before?
There was a problem hiding this comment.
so it used to be that CI ran tests by using a separate Dockerfile (the deleted one) which copy-pasted everything from the Dockerfile.debian.bullseye but with the addition of an ENTRYPOINT to run_test.sh, but really what that script was doing was like build_the_sdk_and_run_test. So with this PR we have a Dockerfile which now takes care of the build_the_sdk part, and it can use the existing Dockerfile.debian.bullseye without needing the duplicated one
| ninja all | ||
| ninja install | ||
| INSTALL_DIR="$(pwd)/install" | ||
| BUILD_SHARED=$(grep 'BUILD_SHARED_LIBS' CMakeCache.txt | cut -d '=' -f 2) |
There was a problem hiding this comment.
are we adding this line because of the other shared library changes in #309 that are happening?
There was a problem hiding this comment.
no not super related except to the extent that this is part of the ongoing saga of having shared/static builds properly supported :) but in particular this script is building with CMake rather than Conan so it already works properly
njooma
left a comment
njooma
left a comment
There was a problem hiding this comment.
I think this is a great starting point. Eventually, we might want to release our base tags online so that the end user doesn't have to build it themselves ahead of time.
3 participants
This implements a minimal proof of concept version of docker containers for module development. The old images have been moved to a
base-imagesdirectory, and now theDockerfile.sdk-buildcan be used off an existing build of a base image.This is validated by the changes to the
test.ymlworkflow, removing the duplicated dockerfile we used to use.