Release docker changes#816
Merged
Merged
Conversation
m-reuter
reviewed
May 20, 2026
m-reuter
reviewed
May 20, 2026
m-reuter
reviewed
May 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Docker/Apptainer build and release workflow to support building images from a backend-neutral, pinned requirements.txt (while still selecting the PyTorch backend at build time), and improves the runtime UX by detecting missing Docker --user mapping instead of crashing.
Changes:
- Extend
tools/export_pip-r.shto export backend-neutral pinned requirements from Docker or Singularity/Apptainer images (filtering backend-specific packages). - Add
--pinned_requirementsto the Docker build script and wire it through the Dockerfile to install fromrequirements.txtwhen enabled. - Introduce a default
nonrootimage user and add a guard that exits with a helpful message when users forget-u $(id -u):$(id -g).
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/export_pip-r.sh | Adds Apptainer support, tmp-file output, and filtering to generate backend-neutral pinned requirements. |
| tools/Docker/README.md | Documents the required --user mapping behavior and the new --pinned_requirements build option. |
| tools/Docker/Dockerfile | Adds a pinned-requirements install path, includes requirements in the build stage, and creates a real nonroot user. |
| tools/Docker/build.py | Adds --pinned_requirements and passes PINNED_REQUIREMENTS build arg; minor builder selection logic tweak. |
| run_fastsurfer.sh | Updates comment to reflect expanded docker/root user validation. |
| requirements.txt | Regenerated pinned dependency set intended for backend-neutral installs. |
| recon_surf/functions.sh | Extends check_allow_root to also detect missing Docker --user mapping via default-user guard. |
| long_fastsurfer.sh | Updates comment to reflect expanded docker/root user validation. |
| .dockerignore | Re-includes root requirements.txt in Docker build context while keeping broader ignore patterns. |
Comments suppressed due to low confidence (1)
tools/export_pip-r.sh:123
- If the final
mvfails, the script still clears the EXIT trap (trap - EXIT), which can leave the temporary file behind and silently continue with a stale$target. Consider checkingmv's exit status and only disabling the trap after a successful rename (or keep the trap andexit 1on failure).
} >> "$tmp_target"
mv "$tmp_target" "$target"
trap - EXIT
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
This PR adds support for building Docker images from the pinned, backend-neutral
requirements.txtinstead of alwaysresolving dependencies from
pyproject.toml.It also updates the generated
requirements.txtfrom the tested release apptainer image and introduces an error message when users forget to map their host UID/GID into the Docker container instead of a crash.Changes
tools/export_pip-r.shsupport for exporting backend-neutral pinned requirements from Docker or Singularity/Apptainer images.
requirements.txtfromfastsurfer-dev-3c52a21.sif.tools/Docker/build.py --pinned_requirements.PINNED_REQUIREMENTS=trueinto the Dockerfile and install viauv pip compile --torch-backend=<device> requirements.txt.requirements.txtin the Docker build context despite the broader.dockerignorerequirements pattern.nonrootuser in the Docker image.nonrootuser with a message asking users to pass-u $(id -u):$(id -g).--allow_rootbehavior.Testing
--pinned_requirements.-u $(id -u):$(id -g)message.--allow_rootwarns and continues.long_fastsurfer.shalso triggers the new default nonroot guard.torch 2.7.1+cu118,torchvision 0.22.1+cu118, CUDA11.8.--pinned_requirements --dry_runemitsPINNED_REQUIREMENTS=truefor another CUDA backend.