Development setup script and analogous Dockerfile
#36
Conversation
greenc-FNAL
commented
Dec 10, 2025
greenc-FNAL
commented
- feat: Create script to set up cetmodules development environment
- Prelims for container definition
- feat(dev): Add Docker-based development environment
- fix(dev): Add --break-system-packages to Dockerfile
- fix(dev): Avoid UID conflict in Dockerfile
- Dockerfile tweaks and documentation
This script automates the setup of a development environment for cetmodules on Ubuntu and AlmaLinux. It performs the following steps: - Detects the operating system (Ubuntu or AlmaLinux) and uses the appropriate package manager. - Installs system dependencies such as git, doxygen, graphviz, and build tools. - Sets up a Python virtual environment and installs CMake, Sphinx, and other required Python packages. - Clones the Catch2 repository, builds it, and installs it. - Clones the cetmodules repository, configures the build with documentation enabled, builds the project, and runs the tests. - Builds the cetmodules documentation.
Adds a Dockerfile and instructions for building and running a containerized development environment for cetmodules. The new configuration, located in `dev/container/`, provides a consistent and reproducible environment with all necessary dependencies installed. It is configured to support an out-of-source build workflow and includes a non-root user to avoid file ownership issues when mounting the source directory. A `README.md` is included with detailed instructions for building the image and running the container.
This commit resolves a build failure in the development container. Recent versions of Python in Ubuntu/Debian images prevent `pip` from installing packages globally to avoid conflicts with the system's package manager. This change adds the `--break-system-packages` flag to the `pip3 install` command in the `Dockerfile` to override this protection, which is a safe and necessary step within a self-contained container.
This commit resolves a build failure in the development container caused by a UID conflict. The `useradd` command failed because UID 1000 was already in use in the base Ubuntu image. This change modifies the UID for the `developer` user to 5000 to avoid this conflict.
greenc-FNAL
changed the title
Dockerfile
There was a problem hiding this comment.
Pull request overview
This PR adds development environment setup tools for cetmodules, including a bash script for local setup and a Docker-based containerized environment. The changes facilitate easier onboarding and consistent development environments across different systems.
Key changes:
- New bash script (
setup_cetmodules_dev.sh) to automate development environment setup on Ubuntu and AlmaLinux - Docker-based development environment with Dockerfile and documentation for containerized builds
- Added
.gitignoreentries for build artifacts and virtual environment directories
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| dev/setup_cetmodules_dev.sh | Bash script automating installation of dependencies, Python packages, Catch2, and building cetmodules |
| dev/container/Dockerfile | Docker image definition with build dependencies, Python tools, and Catch2 pre-installed |
| dev/container/README.md | Documentation for building and running the development container |
| .gitignore | Added entries to ignore build directories, logs, and Python virtual environments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RUN pip3 install --no-cache-dir --break-system-packages \ | ||
| cmake \ | ||
| sphinx \ | ||
| sphinxcontrib-moderncmakedomain \ | ||
| sphinx-design \ | ||
| sphinx-toolbox \ | ||
| sphinxcontrib-jquery |
There was a problem hiding this comment.
The --break-system-packages flag bypasses Python's externally-managed-environment protection. This is a security and maintainability risk. Consider using a virtual environment inside the container or using the OS package manager's Python packages instead. For example:
RUN python3 -m venv /opt/venv
ENV PATH="/opt/venv/bin:$PATH"
RUN pip3 install --no-cache-dir cmake sphinx ...
dev/container/Dockerfile
Outdated
| rm -rf /tmp/Catch2 | ||
|
|
||
| # Create a non-root user | ||
| RUN useradd -m -u 5000 -s /bin/bash developer && \ |
There was a problem hiding this comment.
[nitpick] The hard-coded UID 5000 may conflict with existing users on the host system. The README suggests running the container with --user "$(id -u):$(id -g)", which will override this anyway. Consider using a higher UID (e.g., 10000+) to reduce collision probability, or document why 5000 was chosen.
| RUN useradd -m -u 5000 -s /bin/bash developer && \ | |
| RUN useradd -m -u 10000 -s /bin/bash developer && \ |
dev/container/Dockerfile
Outdated
| RUN useradd -m -u 5000 -s /bin/bash developer && \ | ||
| echo "developer ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers |
There was a problem hiding this comment.
Granting passwordless sudo access to the developer user (NOPASSWD:ALL) creates a security risk. Since the README recommends running with --user flag to match host UID/GID, this sudo configuration may not be necessary. If sudo access is needed, consider limiting it to specific commands or requiring a password.
| RUN useradd -m -u 5000 -s /bin/bash developer && \ | |
| echo "developer ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers | |
| RUN useradd -m -u 5000 -s /bin/bash developer |
|
|
||
| # --- Catch2 Installation --- | ||
| echo "Cloning and installing Catch2..." | ||
| git clone https://github.com/catchorg/Catch2.git |
There was a problem hiding this comment.
The Catch2 repository is cloned without specifying a version or tag. This means the installation will use the latest commit on the default branch, which could introduce breaking changes or incompatibilities. Consider pinning to a specific release tag, for example:
git clone --branch v3.5.2 --depth 1 https://github.com/catchorg/Catch2.git| git clone https://github.com/catchorg/Catch2.git | |
| git clone --branch v3.5.2 --depth 1 https://github.com/catchorg/Catch2.git |
| sphinxcontrib-jquery | ||
|
|
||
| # Install Catch2 | ||
| RUN git clone https://github.com/catchorg/Catch2.git /tmp/Catch2 && \ |
There was a problem hiding this comment.
The Catch2 repository is cloned without specifying a version or tag, which means the build will use whatever is on the default branch at the time. This could introduce breaking changes or incompatibilities. Consider pinning to a specific release tag for reproducibility:
RUN git clone --branch v3.5.2 --depth 1 https://github.com/catchorg/Catch2.git /tmp/Catch2 && \| RUN git clone https://github.com/catchorg/Catch2.git /tmp/Catch2 && \ | |
| RUN git clone --branch v3.5.2 --depth 1 https://github.com/catchorg/Catch2.git /tmp/Catch2 && \ |
| @@ -0,0 +1,40 @@ | |||
| # Cetmodules Development Container | |||
There was a problem hiding this comment.
Inconsistent capitalization: "Cetmodules" should be "cetmodules" (lowercase) to match the project's naming convention used throughout the repository.
| # Cetmodules Development Container | |
| # cetmodules Development Container |
| # ... and optionally push with destination tag "latest" | ||
|
|
||
| # Base image | ||
| FROM ubuntu:latest |
There was a problem hiding this comment.
Using ubuntu:latest as a base image is not recommended for production containers. The latest tag is not pinned to a specific version, which can lead to unpredictable behavior and breaking changes when Ubuntu releases a new version. Consider pinning to a specific version like ubuntu:22.04 or ubuntu:24.04.
| FROM ubuntu:latest | |
| FROM ubuntu:22.04 |
There was a problem hiding this comment.
@copilot This is not a production container; it is a development container.
| @@ -0,0 +1,58 @@ | |||
| # Dockerfile for building Cetmodules development image. | |||
There was a problem hiding this comment.
Typo in the comment: "Cetmodules" should be "cetmodules" (lowercase) to match the project's naming convention used throughout the repository and documentation.
| # Dockerfile for building Cetmodules development image. | |
| # Dockerfile for building cetmodules development image. |
| cd cetmodules | ||
| cmake -S . -B build -DBUILD_DOCS=ON | ||
| cmake --build build | ||
| ctest --test-dir build | ||
|
|
||
| # --- Documentation Build --- | ||
| echo "Building cetmodules documentation..." | ||
| cmake --build build --target doc-cetmodules-reference | ||
| cd .. |
There was a problem hiding this comment.
The script changes directory to cetmodules but never changes back. If the script is sourced instead of executed, this will leave the user in an unexpected directory. Consider adding cd .. at the end or wrapping the cetmodules operations in a subshell.
| cd cetmodules | |
| cmake -S . -B build -DBUILD_DOCS=ON | |
| cmake --build build | |
| ctest --test-dir build | |
| # --- Documentation Build --- | |
| echo "Building cetmodules documentation..." | |
| cmake --build build --target doc-cetmodules-reference | |
| cd .. | |
| ( | |
| # Enter cetmodules directory in a subshell to avoid affecting user's shell if sourced | |
| cd cetmodules | |
| cmake -S . -B build -DBUILD_DOCS=ON | |
| cmake --build build | |
| ctest --test-dir build | |
| # --- Documentation Build --- | |
| echo "Building cetmodules documentation..." | |
| cmake --build build --target doc-cetmodules-reference | |
| ) |
| # --- cetmodules Build and Test --- | ||
| echo "Cloning, building, and testing cetmodules..." | ||
| git clone https://github.com/FNALssi/cetmodules.git | ||
| cd cetmodules | ||
| cmake -S . -B build -DBUILD_DOCS=ON | ||
| cmake --build build | ||
| ctest --test-dir build | ||
|
|
||
| # --- Documentation Build --- | ||
| echo "Building cetmodules documentation..." | ||
| cmake --build build --target doc-cetmodules-reference | ||
| cd .. |
There was a problem hiding this comment.
The script clones a fresh copy of the cetmodules repository and builds it, but this doesn't align with typical development workflow. A developer running this script from the cetmodules repository would expect to build and test their current working copy, not a fresh clone from GitHub. This makes the script unsuitable for local development and testing of changes.
|
@greenc-FNAL I've opened a new pull request, #37, to work on those changes. Once the pull request is ready, I'll request review from you. |
This commit introduces a more robust and user-friendly development container setup. It replaces the previous mechanism of relying on the `--user` flag with an entrypoint script that dynamically adjusts the in-container `developer` user's UID and GID to match the host user's. This is a best-practice pattern for pre-built images that solves file ownership issues on mounted volumes. The key changes include: - An `entrypoint.sh` script to manage user mapping at runtime. - Use of `gosu` for secure privilege dropping. - An updated `Dockerfile` to incorporate the entrypoint. - Revised `README.md` with clear instructions for running the container with both `/source` and `/build` volumes mounted. fix(devcontainer): Use non-reserved variable names in entrypoint This commit fixes a bug in the `entrypoint.sh` script that caused the container to fail on startup. The script was using the reserved bash variable `UID`, which is readonly. This resulted in an error when the script attempted to assign a new value to it. The fix renames the conflicting variables from `UID` and `GID` to `TARGET_UID` and `TARGET_GID` respectively, resolving the runtime error. fix(devcontainer): Prevent entrypoint from changing host source permissions This commit corrects a critical bug in the `entrypoint.sh` script that caused it to recursively change the ownership of the mounted `/source` directory, altering the user's project files on the host. The `chown -R` command in the script is now restricted to only the `/build` and `/home/developer` directories. This ensures that the container has correct write permissions for build artifacts and its home directory, while leaving the ownership of the user's source code untouched. This change makes the development container safe to use and prevents it from having unintended and destructive side effects on the host system. revert(devcontainer): Remove entrypoint and use --user for UID/GID This commit reverts the entrypoint-based approach and returns to a simpler, more compatible method for handling user permissions that works correctly with both Docker and Podman. The previous entrypoint-based solution, while a common pattern for Docker, is incompatible with Podman's user namespace mapping (`--userns=keep-id`). This incompatibility led to incorrect file ownership on the host's build directory. The corrected approach is to: 1. Remove the `entrypoint.sh` script entirely. 2. Simplify the `Dockerfile` by removing the `gosu` dependency and entrypoint logic. 3. Make the `/source` and `/build` directories inside the container world-writable (`chmod 777`). 4. Use the standard `--user "$(id -u):$(id -g)"` flag when running the container. This method allows the user specified on the command line to write to the mounted volumes without permission errors and ensures that the created files have the correct ownership on the host, which is the desired behavior for both Podman and Docker. The `README.md` has been updated to reflect this final, correct usage. fix(devcontainer): Remove developer user for Podman compatibility This commit provides the final, correct implementation for the development container, ensuring compatibility with Podman's user namespace features. The previous approach of creating a `developer` user inside the `Dockerfile` conflicted with Podman's user mapping when the `--user` flag was used. This resulted in the user inside the container not having the expected ownership of the mounted `/build` volume. The solution is to remove the `developer` user entirely from the `Dockerfile`. The image is now prepared with world-writable `/source` and `/build` directories, making it a generic environment ready to be used by any user ID passed via the `--user` flag. This is the simplest and most robust solution, guaranteeing correct file ownership on mounted volumes for both Docker and Podman users. docs(devcontainer): Add correct run instructions for Docker and Podman This commit updates the `README.md` to provide clear and distinct instructions for running the development container with Docker and rootless Podman. The previous instructions were a source of confusion and permission errors due to fundamental differences in how Docker and rootless Podman handle user namespaces. The new documentation clarifies that: - Docker users should continue to use `--user "$(id -u):$(id -g)"` to ensure correct file ownership. - Rootless Podman users should run as the default `root` user inside the container. Podman's user namespace will safely map this to their user on the host, ensuring correct file ownership on mounted volumes. This change, combined with the simplified `Dockerfile`, provides a robust and easy-to-use solution for all users.
