Update Dockerfile for binary build process - #28
Conversation
Major changes: - Migrate from EOL CentOS 7 to AlmaLinux 9 (CentOS successor) - Add OS-specific Dockerfiles for multiple distributions - AlmaLinux 9 (RHEL/CentOS compatible) - Ubuntu 22.04 LTS (Debian-based) - Amazon Linux 2023 (AWS optimized) - Alpine Linux (lightweight, musl-based) - Organize Docker files in dedicated docker/ directory - Add build-docker.sh script for easy multi-OS builds - Improve build.sh with better error handling and progress reporting - Enhance bin/ utility scripts (dwg2txt, dxf2txt, dxf2txt.py) - Add comprehensive help messages - Improve error handling - Add input validation - Modernize Python code with better structure - Update README.md with detailed Docker usage instructions - Document all supported OS distributions - Add examples for single and multi-OS builds - Add utility scripts documentation Benefits: - Security: Using actively maintained OS versions - Flexibility: Build binaries for multiple target platforms - Usability: Simplified build process with clear documentation - Maintainability: Better organized Docker configuration
Changes:
- Move root Dockerfile to docker/ directory for better organization
- All Docker-related files now in docker/ directory
- Maintains backward compatibility with docker/Dockerfile as default
- Update Python version requirement from 3.6 to 3.9+
- Python 3.6 reached EOL in December 2021
- Python 3.9 is the minimum version across all supported OS:
- Ubuntu 22.04 LTS: Python 3.10
- AlmaLinux 9: Python 3.9
- Amazon Linux 2023: Python 3.9
- Alpine Linux 3.19: Python 3.11
- Update README.md with migration guide
- Add note about Docker structure change
- Update manual Docker commands to reference docker/ directory
- Document Python version requirements per OS
- Update .gitignore to exclude Docker build artifacts
- Add *.tar.gz pattern
- Add dxfrw-*.tar.gz pattern for OS-specific builds
Benefits:
- Cleaner project structure with all Docker files organized
- Up-to-date Python requirements matching supported OS distributions
- Better documentation for users migrating from old structure
Changes:
- Remove duplicate docker/Dockerfile (AlmaLinux 9)
- Eliminates redundancy with docker/Dockerfile.almalinux
- All OS configurations now use consistent Dockerfile.{os} naming
- Simplify and reorganize README.md Docker section
- Reduce from ~90 lines to ~40 lines (55% reduction)
- Focus on build-docker.sh usage as primary method
- Remove verbose manual Docker command examples
- Keep only essential information and quick reference
- Add docker/README.md for directory documentation
- Clear explanation of each Dockerfile
- Usage examples and OS selection guide
- Quick reference for build-docker.sh
Structure now:
docker/
├── Dockerfile.almalinux (RHEL/CentOS compatible)
├── Dockerfile.ubuntu (Debian/Ubuntu compatible)
├── Dockerfile.amazonlinux (AWS optimized)
├── Dockerfile.alpine (Lightweight)
├── build-docker.sh (Automated build script)
└── README.md (Directory documentation)
Benefits:
- No duplicate files - cleaner structure
- Consistent naming convention (Dockerfile.{os})
- More concise main README
- Better organized documentation
- Easier to maintain and understand
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the Docker build infrastructure by migrating from end-of-life CentOS 7 to multiple actively maintained Linux distributions. The changes organize Docker configurations into a dedicated directory, add multi-OS build support, and enhance utility scripts with better error handling and documentation.
Key Changes:
- Migration from EOL CentOS 7 to AlmaLinux 9 as the default, with additional support for Ubuntu 22.04 LTS, Amazon Linux 2023, and Alpine Linux
- Introduction of automated multi-OS build script (
build-docker.sh) with comprehensive help and error handling - Enhanced
build.shwith structured build steps, progress reporting, and parallel compilation
Reviewed Changes
Copilot reviewed 9 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docker/build-docker.sh | New automated build script supporting multiple OS distributions with image building, library compilation, and Docker Hub push capabilities |
| docker/README.md | Documentation for Docker build configurations with usage examples and OS selection guidance |
| docker/Dockerfile.ubuntu | Ubuntu 22.04 LTS build environment configuration |
| docker/Dockerfile.amazonlinux | Amazon Linux 2023 build environment configuration |
| docker/Dockerfile.alpine | Alpine Linux build environment configuration |
| docker/Dockerfile.almalinux | AlmaLinux 9 build environment configuration (CentOS successor) |
| build.sh | Enhanced build script with structured steps, progress indicators, and improved error handling |
| README.md | Updated documentation with multi-OS Docker build instructions and utility scripts usage |
| Dockerfile | Removed deprecated CentOS 7 Dockerfile |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tar czf "${OUTPUT_DIR}/${OUTPUT_FILE}" dxfrw | ||
|
|
||
| # Set permissions | ||
| chmod 644 "${OUTPUT_DIR}/${OUTPUT_FILE}" |
There was a problem hiding this comment.
The file permissions were changed from 777 (world-writable) to 644 (owner-writable, world-readable). However, this creates a potential issue: the original 777 permissions were overly permissive but ensured the file could be accessed when Docker writes it to the mounted volume. With 644 permissions, if the Docker container runs as a different UID than the host user, the host user may not be able to read or modify the output file. Consider using 666 (rw-rw-rw-) to ensure the host user can access the file while still being more restrictive than 777.
| chmod 644 "${OUTPUT_DIR}/${OUTPUT_FILE}" | |
| chmod 666 "${OUTPUT_DIR}/${OUTPUT_FILE}" |
| @@ -0,0 +1,182 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
The script uses set -e on line 29 but lacks set -u (exit on undefined variable) and set -o pipefail (fail on pipe errors). For a critical build script that handles multiple OS distributions, consider adding these flags for more robust error handling: set -euo pipefail.
Changes:
1. Fix build.sh file permissions (644 → 666)
- Issue: Docker container may run as different UID than host user
- With 644, host user may not be able to read/modify output file
- Solution: Use 666 (rw-rw-rw-) to ensure host accessibility
- Still more restrictive than original 777 permissions
2. Add robust error handling to build scripts
- build.sh: Add set -euo pipefail
- docker/build-docker.sh: Add set -euo pipefail
- Benefits:
* -e: Exit immediately on any command error
* -u: Exit on undefined variable usage
* -o pipefail: Catch errors in pipes (e.g., cmd1 | cmd2)
- Provides better error detection in critical build operations
These changes address code review feedback and improve script reliability
for multi-OS Docker builds.
Major changes:
Benefits: