Conversation
…p of existing options
Add JAVA_OPTS support for configurable memory settings in Dockerfiles
There was a problem hiding this comment.
Pull request overview
This PR performs a version bump for the BoxLang Docker project, incrementing the version from 1.7.0 to 1.8.0, along with some code improvements including JAVA_OPTS handling and whitespace cleanup in Dockerfiles.
Key Changes:
- Version bumped to 1.8.0 in
version.json(though changelog shows 1.7.0) - Added changelog entry for version 1.7.0 with release date
- Introduced new JAVA_OPTS configuration logic in the startup script to handle custom memory settings
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| version.json | Updates BOXLANG_VERSION and IMAGE_VERSION to 1.8.0 |
| changelog.md | Adds version 1.7.0 entry with date 2025-11-04 and updates unreleased comparison link |
| builds/base/JRE21.Web.Nginx.Dockerfile | Adds missing newline at end of file |
| builds/base/JRE21.Web.Dockerfile | Adds missing newline at end of file |
| builds/base/JRE21.Dockerfile | Removes trailing blank line |
| builds/base/Alpine.JRE21.Web.Dockerfile | Adds missing newline at end of file |
| builds/base/Alpine.JRE21.Dockerfile | Removes trailing blank line |
| build/run.sh | Adds JAVA_OPTS configuration logic to handle MIN_MEMORY and MAX_MEMORY environment variables with dynamic heap size management |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "BOXLANG_VERSION": "1.8.0", | ||
| "IMAGE_VERSION": "1.8.0" |
There was a problem hiding this comment.
Version mismatch: version.json is being bumped to 1.8.0, but changelog.md only adds an entry for version 1.7.0. These should be consistent - either both should reference 1.7.0 or both should reference 1.8.0.
| "BOXLANG_VERSION": "1.8.0", | |
| "IMAGE_VERSION": "1.8.0" | |
| "BOXLANG_VERSION": "1.7.0", | |
| "IMAGE_VERSION": "1.7.0" |
|
|
||
| # Handle JAVA_OPTS configuration | ||
| # If MIN_MEMORY or MAX_MEMORY are provided and differ from defaults, rebuild JAVA_OPTS with those values | ||
| if [[ -n "${MIN_MEMORY}" && "${MIN_MEMORY}" != "512m" ]] || [[ -n "${MAX_MEMORY}" && "${MAX_MEMORY}" != "512m" ]]; then |
There was a problem hiding this comment.
Logic issue: The condition checks if MIN_MEMORY or MAX_MEMORY differ from "512m", but this creates inconsistent behavior. If a user sets only MIN_MEMORY=1g (leaving MAX_MEMORY unset), both will be set to different values (MIN_MEMORY=1g, MAX_MEMORY=512m from the default on line 119), which may not be the intended behavior. Consider checking if either variable is explicitly set rather than comparing against default values, or ensure both are set to the same value when only one is provided.
| if [[ -n "${MIN_MEMORY}" && "${MIN_MEMORY}" != "512m" ]] || [[ -n "${MAX_MEMORY}" && "${MAX_MEMORY}" != "512m" ]]; then | |
| # If either MIN_MEMORY or MAX_MEMORY is set, ensure both are set consistently | |
| if [[ -n "${MIN_MEMORY}" || -n "${MAX_MEMORY}" ]]; then | |
| if [[ -n "${MIN_MEMORY}" && -n "${MAX_MEMORY}" ]]; then | |
| # Both are set, use as-is | |
| : | |
| elif [[ -n "${MIN_MEMORY}" ]]; then | |
| # Only MIN_MEMORY is set, use it for both | |
| MAX_MEMORY="${MIN_MEMORY}" | |
| elif [[ -n "${MAX_MEMORY}" ]]; then | |
| # Only MAX_MEMORY is set, use it for both | |
| MIN_MEMORY="${MAX_MEMORY}" | |
| fi |
| # Remove any existing heap settings and rebuild JAVA_OPTS | ||
| JAVA_OPTS_CLEAN="" | ||
| for opt in $JAVA_OPTS; do | ||
| if [[ $opt != -Xms* && $opt != -Xmx* ]]; then | ||
| JAVA_OPTS_CLEAN="$JAVA_OPTS_CLEAN $opt" | ||
| fi | ||
| done |
There was a problem hiding this comment.
The comment says "Remove any existing heap settings and rebuild JAVA_OPTS", but the logic iterates through $JAVA_OPTS without quotes, which will cause issues if any option contains spaces. Consider using proper array handling or quoting: for opt in "$JAVA_OPTS" or use an array-based approach to handle options that may contain spaces.
version bump