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 docker nightly build #2643

Merged
merged 16 commits into from
Jan 6, 2025
Merged
2 changes: 1 addition & 1 deletion docker/Dockerfile.base
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM ubuntu:jammy
FROM ubuntu:noble

RUN apt-get update \
&& apt-get install -y \
Expand Down
2 changes: 1 addition & 1 deletion docker/Dockerfile.nightly
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ COPY docker/scripts/enable_nightly.sh scripts/enable_nightly.sh
RUN scripts/enable_nightly.sh

RUN apt-get update \
&& apt-get install --no-install-recommends -y \
&& apt-get install -y \
libgz-cmake4-dev \
libgz-common6-dev \
libgz-fuel-tools10-dev \
Expand Down
4 changes: 2 additions & 2 deletions docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use the Gazebo code found in the current source tree.
4. Alternatively, you can directly run Gazebo using

```
./docker/run.bash gz-sim:nightly gz-sim-server -v 4
./docker/run.bash gz-sim:nightly gz sim -v 4
azeey marked this conversation as resolved.
Show resolved Hide resolved
```

## Gazebo Using Debians In Docker
Expand All @@ -75,7 +75,7 @@ distribution using debians.
image of Gazebo Garden:

```
./build.bash gz ./Dockerfile.gz
./build.bash gz-ionic ./Dockerfile.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, but now we need to update the instruction right above to say "Gazebo Ionic"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll roll-back it here to garden and lines below too, due to build.bash not working properly with Ionic AFAIU. It executes Dockerfile.gz, which is based on nvidia/opengl:1.2-glvnd-devel-ubuntu20.04 image.

```

2. Run the docker image using `run.bash`, and pass in the name of the docker
Expand Down
14 changes: 2 additions & 12 deletions docker/run.bash
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,8 @@ ARGS=("$@")
# This is necessary so Gazebo can create a context for OpenGL rendering
# (even headless).
XAUTH=/tmp/.docker.xauth
if [ ! -f $XAUTH ]
ntfshard marked this conversation as resolved.
Show resolved Hide resolved
then
xauth_list=$(xauth nlist :0 | sed -e 's/^..../ffff/')
if [ ! -z "$xauth_list" ]
then
echo $xauth_list | xauth -f $XAUTH nmerge -
else
touch $XAUTH
fi
chmod a+r $XAUTH
fi
xauth nlist $DISPLAY | sed -e 's/^..../ffff/' | xauth -f $XAUTH nmerge -
chmod 777 $XAUTH

docker run -it \
-e DISPLAY \
Expand All @@ -40,7 +31,6 @@ docker run -it \
-v "/etc/localtime:/etc/localtime:ro" \
-v "/dev/input:/dev/input" \
--rm \
--gpus all \
ntfshard marked this conversation as resolved.
Show resolved Hide resolved
--security-opt seccomp=unconfined \
$IMG \
${@:2}
6 changes: 3 additions & 3 deletions docker/scripts/install_common_deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ sudo apt-get install --no-install-recommends -y \
cppcheck \
curl \
git \
g++11 \
g++ \
lcov \
pkg-config

sudo apt-get install --no-install-recommends -y \
clang-tidy-14 \
clang-tidy \
python3-yaml

sudo apt-get install --no-install-recommends -y \
libbenchmark-dev \
libbenchmark1
libbenchmark1.8.3
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can get rid of libbenchmark1.8.3. It should be automatically installed with libbenchmark-dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this entire libbenchmark dependency is redundant, It's compiling fine even w/o this installation.
But as I said earlier, initial idea was to keep this PR as same as possible with previous implementation.

I'd like to remove it completely, if no objections

Copy link
Contributor

Choose a reason for hiding this comment

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

libbenchmark is an optional dependency. There are tests that use it, but they're disabled if libbenchmark is not installed

if (GzBenchmark_FOUND)

So I think we should keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted, commend added


sudo apt-get clean
sudo rm -rf /var/lib/apt/lists/*