Dockerfile: fix issue with LLVM path and other improvements #296
+131
−40
Conversation
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
Allow PostgreSQL's configure script to auto-detect llvm-config instead of hardcoding the path to /usr/bin/llvm-config-64. This improves portability across different systems and LLVM installations, as the hardcoded path may not exist on all distributions. The --with-llvm flag is sufficient for configure to find the appropriate llvm-config binary automatically. Based on commit e41e201 from dockerfile-stable-llvm-fix branch.
Replace the anti-pattern of setting environment variables via .bashrc with proper Docker ENV directives. This ensures: - Environment variables are available in all RUN commands - No need to source .bashrc in build steps - More reliable and Docker-native approach - Cleaner and more maintainable Dockerfile Variables moved to ENV: - PATH: Includes PostgreSQL bin directory - LD_LIBRARY_PATH: Includes PostgreSQL lib directory - PG_CONFIG: Points to pg_config binary This simplifies the Spock build step significantly.
Reformat the configure command from a single unreadable line with eval and quoted options to a clean multi-line format. Changes: - Remove unnecessary eval (potential security concern) - Remove shell variable with quoted options pattern - Use direct ./configure with multi-line arguments - Each option on its own line for easy review - Improved maintainability for future option changes This makes it much easier to: - Review which features are enabled - Add or remove configure options - Understand the build configuration at a glance
Merge multiple RUN commands into single layers to reduce the number of layers in the final Docker image. This optimization: - Reduces image size by combining related operations - Improves build efficiency - Better utilizes Docker layer caching - Follows Dockerfile best practices Changes: - PostgreSQL clone + chmod: 2 RUN → 1 RUN - pgedge setup: 3 RUN → 1 RUN - PostgreSQL compile: 2 RUN → 1 RUN - Spock compile: 3 RUN → 1 RUN - Added descriptive comments for each build stage - Removed duplicate WORKDIR directive Total reduction: ~6 fewer layers
Introduce a build argument to control the number of parallel make jobs
instead of using hardcoded values that differed between PostgreSQL
and Spock builds.
Changes:
- Add ARG MAKE_JOBS=4 (default value)
- Replace hardcoded -j4 (PostgreSQL) with -j${MAKE_JOBS}
- Replace hardcoded -j16 (Spock) with -j${MAKE_JOBS}
- Ensures consistent parallelism across all builds
Benefits:
- Can override at build time: --build-arg MAKE_JOBS=16
- Consistent behavior between PostgreSQL and Spock
- Easy to tune for different build environments
- Default of 4 is conservative and works on most systems
The previous inconsistency (j4 vs j16) is now eliminated.
Replace the two-step pattern of COPY + RUN chmod with the modern COPY --chmod directive available in BuildKit. Changes: - COPY --chmod=755 instead of COPY + RUN sudo chmod +x - Eliminates one RUN layer - Removes unnecessary sudo usage Benefits: - One fewer layer in the final image - Cleaner and more concise Dockerfile - Uses modern Docker/BuildKit features - Atomic operation (no window where files lack execute permission) This is the recommended approach in modern Dockerfiles.
Major improvements to the base image to fix several critical issues: 1. **Inline package list** - Removed dependency on lib-list.txt file - Eliminates need for build context with external files - Base image can now be built independently - All dependencies explicitly listed in Dockerfile 2. **Fix SSH key location** - SSH keys now created for pgedge user - Previously created in /root/.ssh (broken!) - Now correctly created in /home/pgedge/.ssh - Proper permissions (700 for .ssh, 600 for files) - Actually usable for SSH-based testing 3. **Set WORKDIR** - Added WORKDIR /home/pgedge - Child images no longer need to set it immediately - More sensible default than / 4. **Remove unused directory creation** - Removed mkdir /home/pgedge/spock - Child images create this when COPYing anyway - Eliminates wasted layer 5. **Better documentation** - Enhanced header comments - Inline comments for each section - Additional LABEL for description - Clarifies image purpose and contents 6. **Cleanup dnf cache** - Added dnf clean all - Reduces final image size This makes the base image self-contained and actually usable as a standalone build artifact.
Clarify and fix the user context throughout the build process to eliminate confusion and properly handle permissions. Changes: 1. **Explicit USER root at start** - Base image ends as USER pgedge - Step-1 needs root for installations - Now explicitly documented with comment 2. **Proper ownership after COPY** - Added chown after copying spock source - Ensures pgedge user can access files - Prevents permission issues during build 3. **Remove sudo, use su instead** - Changed: sudo -u pgedge → su - pgedge -c - More explicit about running as different user - Added chown before running install script 4. **Better comments on USER switching** - Documented why we switch to root - Documented why we switch back to pgedge - Makes build process clear Why this matters: - Eliminates confusion about current user context - Proper permissions throughout build - Better suited for CI/CD (GitHub Actions) - More maintainable and debuggable The image now has clear user context at each stage: - Start: root (for installations) - Runtime: pgedge (for testing)
mason-sharp
approved these changes
Dec 27, 2025
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.
Solving the LLVM issue, spend some time to analyse the whole approach - see (almost) cosmetic changes.