Fix issue #45: Resolve awk newline error when adding Java profile#55
Fix issue #45: Resolve awk newline error when adding Java profile#55fletchgqc wants to merge 2 commits into
Conversation
The awk command used for placeholder substitution in Dockerfiles was failing with 'newline in string' errors when processing multi-line profile installations. This particularly affected the Java profile. Changes: - Replace awk-based substitution with pure bash line-by-line processing - Fix syntax error: add missing || operator in placeholder guard clause - Ensure reliable handling of multi-line content in profile installations This approach is more portable and avoids awk's limitations with multi-line string variables passed via -v. Fixes RchGrav#45 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer's GuideReplaces the awk-based Dockerfile placeholder substitution with a robust bash line-by-line loop to properly inject multi-line content, and corrects the guard clause by adding the missing logical OR between placeholder checks. Sequence diagram for Dockerfile placeholder substitution processsequenceDiagram
participant main_sh as main.sh
participant Dockerfile as Dockerfile
participant Bash as Bash
main_sh->Dockerfile: Read base_dockerfile
main_sh->Bash: Process each line
Bash->main_sh: For each line
alt PROFILE_INSTALLATIONS placeholder
Bash->main_sh: Inject profile_installations content
else LABELS placeholder
Bash->main_sh: Inject labels content
else Other lines
Bash->main_sh: Keep line unchanged
end
main_sh->Dockerfile: Write final_dockerfile
main_sh->main_sh: Check for unreplaced placeholders
main_sh->main_sh: Error if any remain
Class diagram for updated Dockerfile substitution logic in main.shclassDiagram
class MainSh {
- base_dockerfile: string
- profile_installations: string
- labels: string
- final_dockerfile: string
+ processDockerfile()
}
MainSh : processDockerfile() replaces awk with bash loop
MainSh : processDockerfile() checks for unreplaced placeholders
MainSh : processDockerfile() writes final_dockerfile
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider using readarray (mapfile) to load base_dockerfile into an array and iterate, which can simplify the loop and eliminate manual newline handling.
- Instead of accumulating final_dockerfile in memory, stream the substituted lines directly to the output file (e.g. via a here-doc or process substitution) for better memory efficiency on larger templates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using readarray (mapfile) to load base_dockerfile into an array and iterate, which can simplify the loop and eliminate manual newline handling.
- Instead of accumulating final_dockerfile in memory, stream the substituted lines directly to the output file (e.g. via a here-doc or process substitution) for better memory efficiency on larger templates.
## Individual Comments
### Comment 1
<location> `main.sh:619` </location>
<code_context>
# Guard: ensure no unreplaced placeholders remain
- if grep -q '{{PROFILE_INSTALLATIONS}}' <<<"$final_dockerfile" grep -q '{{LABELS}}' <<<"$final_dockerfile"; then
- error "Unreplaced placeholders remain in generated Dockerfile"
+ if grep -q '{{PROFILE_INSTALLATIONS}}' <<<"$final_dockerfile" || grep -q '{{LABELS}}' <<<"$final_dockerfile"; then
+ error "Unreplaced placeholders remain in generated Dockerfile"
fi
</code_context>
<issue_to_address>
Corrected logic for placeholder check, but could be simplified.
Consider using a single grep command with an extended regex to check for both placeholders at once: grep -qE '{{PROFILE_INSTALLATIONS}}|{{LABELS}}'.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # Guard: ensure no unreplaced placeholders remain | ||
| if grep -q '{{PROFILE_INSTALLATIONS}}' <<<"$final_dockerfile" grep -q '{{LABELS}}' <<<"$final_dockerfile"; then | ||
| error "Unreplaced placeholders remain in generated Dockerfile" | ||
| if grep -q '{{PROFILE_INSTALLATIONS}}' <<<"$final_dockerfile" || grep -q '{{LABELS}}' <<<"$final_dockerfile"; then |
There was a problem hiding this comment.
suggestion: Corrected logic for placeholder check, but could be simplified.
Consider using a single grep command with an extended regex to check for both placeholders at once: grep -qE '{{PROFILE_INSTALLATIONS}}|{{LABELS}}'.
Use a single grep command with extended regex to check for both placeholders instead of two separate grep commands with ||. This is more efficient and cleaner while maintaining the same functionality.
|
Claude response to review:
|
|
No operation ID found for this PR |
Applies critical fixes from RchGrav/claudebox to enable ClaudeBox to run properly on macOS Tahoe (Sequoia 15.x): - Fix unbound variable errors on macOS (PR RchGrav#78) - Add :- operator to all array expansions for Bash 3.2 compatibility - Prevents "unbound variable" errors with set -u flag - Fix MCP cleanup trap scoping issue (PR RchGrav#73) - Consolidate MCP temp files into trap-accessible array - Eliminates exit errors when MCP servers are configured - Fix Python profile error handling (PR RchGrav#74) - Add proper error handling without masking failures - Implement broken symlink detection and auto-recovery - Ensure Bash 3.2 compatibility in docker-entrypoint - Fix awk newline error in Dockerfile substitution (PR RchGrav#55) - Replace awk-based substitution with pure bash processing - Handles multi-line LABEL and PROFILE_INSTALLATIONS content - Fixes "awk: newline in string" error Modified files: - build/docker-entrypoint: Python profile error handling - lib/*.sh: Array expansion fixes across all library modules - main.sh: Dockerfile template substitution fix - tooling/profiles/rust.sh: Array expansion fix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
## Summary Cherry-picked fix for the awk newline error that occurs when building Docker images with certain profiles (e.g., Java). **Original author:** [@fletchgqc](https://github.com/fletchgqc) ## Cherry-picked commits - `dc29541` - Fix issue RchGrav#45: Resolve awk newline error when adding Java profile - `8ad65c3` - Simplify placeholder check with extended regex ## Attribution This fix was developed by @fletchgqc in their fork. We cherry-picked it into our sibling fork since the upstream PR is still pending review. **References:** - Upstream PR: [RchGrav#55](RchGrav#55) - Original branch: [fletchgqc/fix/issue-45-java-profile-awk-error](https://github.com/fletchgqc/claudebox/tree/fix/issue-45-java-profile-awk-error) - Original issue: [RchGrav#45](RchGrav#45) ## Changes Replaces awk-based Dockerfile placeholder substitution with pure bash line-by-line processing, fixing multi-line content handling while maintaining Bash 3.2 compatibility for macOS. --------- Co-authored-by: John Fletcher <john.fletcher@codecentric.de> Co-authored-by: Claude <noreply@anthropic.com>
Summary
This PR fixes issue #45 where users encountered an
awk: newline in stringerror when adding the Java profile to ClaudeBox.Problem
The awk command used for Dockerfile placeholder substitution was failing when processing multi-line content in the
profile_installationsvariable. The error manifested as:Solution
||operator was missing between twogrepconditions in the guard clauseTesting
The fix has been tested with the Java profile and successfully builds without errors.
Fixes #45
🤖 Generated with Claude Code
Summary by Sourcery
Replace awk-based Dockerfile placeholder substitution with a bash line-by-line loop to reliably handle multi-line PROFILE_INSTALLATIONS and avoid newline-in-string errors
Bug Fixes:
Enhancements: