-
Notifications
You must be signed in to change notification settings - Fork 100
Fix issue #45: Resolve awk newline error when adding Java profile #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix issue #45: Resolve awk newline error when adding Java profile #55
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
main.sh
Outdated
| # 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: