Fix the broken Docker build and remove dead root config - #371
Merged
Conversation
The CPU image the README documents has not been buildable. Both Dockerfiles
fail on their uv invocation, in different ways:
.devcontainer/Dockerfile:43 uv pip install .
-> error: No virtual environment found; ... pass `--system`
Dockerfile:48 uv pip install --system --no-root .
-> error: unexpected argument '--no-root' found
`--no-root` belongs to `uv sync`, not `uv pip install`, and uv declines to
touch the system interpreter without `--system`. The two files were
near-duplicates that had drifted, so consolidate onto the root one and fix it:
- install with `uv pip install --system .`
- COPY the source before installing. The version is read from
src/cbfkit/VERSION via [tool.setuptools.dynamic], and setuptools only warns
when that file is missing, so installing from pyproject.toml alone quietly
produced an empty cbfkit 0.0.0.
- point docker-compose at the surviving Dockerfile, matching how
gpu.Dockerfile is already referenced
That puts `COPY . .` back on the live path, which makes context weight matter,
so tighten .dockerignore: drop generated media, prose directories and local
tool state, and fix the `mypy_cache` entry that never matched the real
`.mypy_cache` (~200 MB by itself). Context goes from 405 MB to 11 MB.
README.md and LICENSE stay, since setuptools reads both for wheel metadata.
Remove two files that do nothing:
- .gitattributes, empty since 6e08796 two years ago
- .devcontainer/devcontainer.json, superseded by cbfkit-container/, which is
what the README's dev-container instructions already point at
Correct two stale descriptions. conftest.py's docstring claimed it configured
mypy, ruff and bytecode caches and XLA memory; it does none of that.
.env.example advertised MYPY_CACHE_DIR, RUFF_CACHE_DIR and PYTHONPYCACHEPREFIX,
none of which can work from a .env file: the first two are separate processes
that never load it, and Python reads the third at interpreter startup, before
conftest.py runs. JAX_ENABLE_X64 was the file's only uncommented line and is
already set in src/cbfkit/__init__.py.
Test suite unchanged: 492 passed, 4 skipped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The CPU container the README tells people to build does not build. Both Dockerfiles fail on their
uvline, each in a different way:--no-rootis auv syncflag, not auv pip installone, and uv refuses to install into the system interpreter unless you ask it to with--system. The two files started as copies of each other and drifted, so only one is referenced bydocker-compose.ymland neither works.What this does
Consolidates onto the root
Dockerfileand fixes it. The install is nowuv pip install --system ., and the source is copied before the install runs. That second part matters more than it looks: the version comes fromsrc/cbfkit/VERSIONthrough[tool.setuptools.dynamic], and setuptools treats a missing version file as a warning rather than an error, so installing frompyproject.tomlalone quietly produced an emptycbfkit 0.0.0.docker-compose.ymlnow points at the surviving file, which also makes it symmetric with howgpu.Dockerfilewas already referenced.Bringing
COPY . .back onto the live path makes the build context weight real, so.dockerignoregot tighter: generated media, prose directories and local tool state are excluded, and themypy_cacheentry that never matched the actual.mypy_cacheis fixed. That one directory was about 200 MB on its own. The context drops from 405 MB to 11 MB.README.mdandLICENSEstay in, since setuptools reads both for wheel metadata.Two files are deleted because they do nothing:
.gitattributeshas been empty since6e08796, two years ago. An empty one has no effect..devcontainer/devcontainer.jsonis superseded by.devcontainer/cbfkit-container/, which is what the README's dev-container instructions already name. The nested one also carries a debug config the top-level one lacks.Finally, two descriptions that had drifted from their code.
conftest.py's docstring claimed it configured mypy, ruff and bytecode caches plus XLA memory settings; it does none of those..env.exampleadvertisedMYPY_CACHE_DIR,RUFF_CACHE_DIRandPYTHONPYCACHEPREFIX, none of which can work from a.env: the first two are separate processes that never load the file, and Python reads the third at interpreter startup, beforeconftest.pyruns.JAX_ENABLE_X64was the file's only uncommented line and is already set insrc/cbfkit/__init__.py. The file now says plainly which variables work and which have to be exported in your shell.Verification
pytest -m "not slow" testsgives 492 passed, 4 skipped. black and ruff are clean onconftest.py.docker-compose.ymlparses and both services resolve. Nothing references the deleted paths. The files setuptools needs (pyproject.toml,uv.lock,README.md,LICENSE,VERSION,py.typed, and all seven codegen.j2templates) survive the new.dockerignore.The container build itself is not verified. I had no Docker daemon available, so the two
uvfixes were checked by running the exact invocations against uv 0.9.12 rather than through a real build. One assumption is worth naming:--systemrequires the base image's Python not to be PEP 668 externally-managed.ros:humble-ros-base-jammyis Ubuntu 22.04, which predates that marker, so it should be fine. Worth confirming before merge:Since this build appears to have been broken for some time, there may be a second failure sitting behind the first.