Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
ARG GO_VERSION=1.24
ARG LLAMA_SERVER_VERSION=latest
ARG LLAMA_SERVER_VARIANT=cpu
ARG TARGETARCH=${BUILDARCH}
ARG LLAMA_BINARY_PATH=/com.docker.llama-server.native.linux.${LLAMA_SERVER_VARIANT}.${TARGETARCH}
ARG BASE_IMAGE=ubuntu:24.04

Expand Down
19 changes: 17 additions & 2 deletions scripts/apt-install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,25 @@ main() {
local vulkan_version=1.4.321.1
local arch
arch=$(uname -m)
apt-get install -y wget xz-utils
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change introduces an additional apt-get install command. The script now contains multiple apt-get install calls (here, for build dependencies, and for ca-certificates). For better efficiency and to optimize Docker image layers, it's recommended to consolidate all package installations into a single apt-get install command at the top of the script, right after apt-get update.

wget -qO /tmp/vulkan-sdk.tar.xz https://sdk.lunarg.com/sdk/download/$vulkan_version/linux/vulkan-sdk-linux-"$arch"-$vulkan_version.tar.xz
mkdir -p /opt/vulkan
tar -xf /tmp/vulkan-sdk.tar.xz -C /tmp --strip-components=1
mv /tmp/"$arch"/* /opt/vulkan/
tar -xf /tmp/vulkan-sdk.tar.xz -C /tmp

if [ "$arch" != "x86_64" ]; then
# TODO: uninstall build time deps after building the SDK
apt-get install -y libglm-dev cmake libxcb-dri3-0 libxcb-present0 libpciaccess0 \
libpng-dev libxcb-keysyms1-dev libxcb-dri3-dev libx11-dev g++ gcc \
libwayland-dev libxrandr-dev libxcb-randr0-dev libxcb-ewmh-dev \
git python-is-python3 bison libx11-xcb-dev liblz4-dev libzstd-dev \
ocaml-core ninja-build pkg-config libxml2-dev wayland-protocols python3-jsonschema \
clang-format qtbase5-dev qt6-base-dev
pushd /tmp/"${vulkan_version}"
# TODO: we don't need the whole SDK to run stuff, so eventually only build necessary targets here
./vulkansdk --no-deps -j "$(nproc)"
fi
Comment on lines +18 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The current implementation for building the Vulkan SDK on non-x86_64 architectures has a few issues that should be addressed:

  • Missing popd: There's a pushd without a corresponding popd, which can cause subsequent commands to fail. This is a critical bug.
  • Build dependencies are not removed: This will significantly bloat the final Docker image. The TODO comment highlights this.
  • Missing --no-install-recommends: Using this flag with apt-get install is a best practice for Docker images to minimize their size.
  • Readability: The long list of packages can be stored in a variable to improve readability.

Here is a suggested replacement that addresses these points.

Suggested change
if [ "$arch" != "x86_64" ]; then
# TODO: uninstall build time deps after building the SDK
apt-get install -y libglm-dev cmake libxcb-dri3-0 libxcb-present0 libpciaccess0 \
libpng-dev libxcb-keysyms1-dev libxcb-dri3-dev libx11-dev g++ gcc \
libwayland-dev libxrandr-dev libxcb-randr0-dev libxcb-ewmh-dev \
git python-is-python3 bison libx11-xcb-dev liblz4-dev libzstd-dev \
ocaml-core ninja-build pkg-config libxml2-dev wayland-protocols python3-jsonschema \
clang-format qtbase5-dev qt6-base-dev
pushd /tmp/"${vulkan_version}"
# TODO: we don't need the whole SDK to run stuff, so eventually only build necessary targets here
./vulkansdk --no-deps -j "$(nproc)"
fi
if [ "$arch" != "x86_64" ]; then
local build_deps="libglm-dev cmake libxcb-dri3-0 libxcb-present0 libpciaccess0 \
libpng-dev libxcb-keysyms1-dev libxcb-dri3-dev libx11-dev g++ gcc \
libwayland-dev libxrandr-dev libxcb-randr0-dev libxcb-ewmh-dev \
git python-is-python3 bison libx11-xcb-dev liblz4-dev libzstd-dev \
ocaml-core ninja-build pkg-config libxml2-dev wayland-protocols python3-jsonschema \
clang-format qtbase5-dev qt6-base-dev"
apt-get install -y --no-install-recommends $build_deps
pushd /tmp/"${vulkan_version}"
# TODO: we don't need the whole SDK to run stuff, so eventually only build necessary targets here
./vulkansdk --no-deps -j $(nproc)
popd
apt-get purge -y --auto-remove $build_deps
fi


mv /tmp/"${vulkan_version}"/"$arch"/* /opt/vulkan/
rm -rf /tmp/*
fi

Expand Down
Loading