fix: actionable instructions when sandbox-binary directory is missing#114
Conversation
…ssing Closes #110. Previously `dbdeployer unpack` exited with only "You should create it or provide an alternate base directory using --sandbox-binary", leaving users to figure out the path and command themselves. The error now points to `dbdeployer init --skip-all-downloads` (the canonical setup command) and includes a ready-to-copy `mkdir -p <basedir>` as a manual alternative. No behavior change beyond the error text: the directory is still not created automatically.
📝 WalkthroughWalkthroughThe PR improves user experience by replacing a terse error message with detailed, multi-line guidance when the ChangesError Message Enhancement
🎯 1 (Trivial) | ⏱️ ~3 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/unpack.go`:
- Line 74: The help text prints an unquoted mkdir command which breaks for
Basedir values with spaces or special chars; update the fmt.Sprintf call that
builds the help string (the line using fmt.Sprintf(" mkdir -p %s", Basedir)
in cmd/unpack.go) to produce a shell-safe path by quoting or escaping Basedir
(e.g., wrap it in single quotes or escape shell metacharacters) so the printed
command can be copy-pasted safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| "Create it by running:", | ||
| " dbdeployer init --skip-all-downloads", | ||
| "or manually with:", | ||
| fmt.Sprintf(" mkdir -p %s", Basedir), |
There was a problem hiding this comment.
Make the suggested mkdir command shell-safe.
The help text currently prints mkdir -p <path> without quoting, so copy-paste may fail when the base directory contains spaces/shell characters. Quote or escape Basedir in the displayed command.
Suggested fix
- fmt.Sprintf(" mkdir -p %s", Basedir),
+ fmt.Sprintf(" mkdir -p %q", Basedir),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.Sprintf(" mkdir -p %s", Basedir), | |
| fmt.Sprintf(" mkdir -p %q", Basedir), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/unpack.go` at line 74, The help text prints an unquoted mkdir command
which breaks for Basedir values with spaces or special chars; update the
fmt.Sprintf call that builds the help string (the line using fmt.Sprintf("
mkdir -p %s", Basedir) in cmd/unpack.go) to produce a shell-safe path by quoting
or escaping Basedir (e.g., wrap it in single quotes or escape shell
metacharacters) so the printed command can be copy-pasted safely.
There was a problem hiding this comment.
Code Review
This pull request improves the error message in cmd/unpack.go when the base directory is missing by providing specific commands for initialization and manual directory creation. Feedback was provided to wrap the directory path in double quotes within the suggested mkdir -p command to ensure it handles paths with spaces or special characters correctly.
| "Create it by running:", | ||
| " dbdeployer init --skip-all-downloads", | ||
| "or manually with:", | ||
| fmt.Sprintf(" mkdir -p %s", Basedir), |
There was a problem hiding this comment.
The suggested mkdir -p command may fail or behave unexpectedly if the Basedir path contains spaces or special characters. It is safer to wrap the path in double quotes to ensure the command is robust and copy-pasteable for all valid directory paths.
| fmt.Sprintf(" mkdir -p %s", Basedir), | |
| fmt.Sprintf(" mkdir -p \"%s\"", Basedir), |
Summary
dbdeployer unpackfails because$HOME/opt/mysql(or a custom--sandbox-binary) doesn't exist, the error now tells the user exactly how to fix it instead of just saying "You should create it".dbdeployer init --skip-all-downloads(the canonical setup command) and includes a copy-pasteablemkdir -p <basedir>as a manual alternative.Before
After
Test plan
go build ./...passesgo test ./cmd/... ./unpack/...passesHOME: error text matches the "After" block above and exit code is still 1Summary by CodeRabbit