feat: add one-line installer and v0.1.0 release prep#381
Conversation
Lower the friction of the public install story (Issue #136) and clear the way for a v0.1.0 Hex publish. - scripts/install.sh: new POSIX installer that resolves the latest release tag via the GitHub API, downloads the released escript into `$HOME/.local/bin/sqlode`, warns about missing Erlang/OTP with per-distro install hints, and notes when the install directory is not on PATH. Respects `SQLODE_VERSION` and `SQLODE_INSTALL_DIR`. - README.md: promote the one-line `curl | sh` install as the primary path, reorganise the manual escript and `gleam run` options as fallbacks, and add Hex version, Hex downloads, and CI badges in the style already used by nao1215/oaspec. - .github/workflows/release.yml: pipe `I am not using semantic versioning` into `gleam publish -y`, matching oaspec's workaround for the pre-1.0 semver confirmation prompt so the v0.1.0 publish step does not hang. Closes #136
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 21 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new installation script enables one-line installation of sqlode binaries from GitHub Releases, supporting version selection and customizable install directories. README documentation is updated to promote this method as the recommended option alongside existing installation paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
scripts/install.sh (3)
66-75: GitHub API rate limiting will break unauthenticatedlatestresolution.The unauthenticated GitHub REST API is limited to 60 requests/hour per IP. From shared networks (corporate NATs, CI runners, Codespaces) users can easily hit the limit, in which case
curl -fsSLfails with 403 and users see only the generic "could not determine latest release tag" error.Two small improvements:
- Mention the rate-limit possibility in the die message (e.g. "…or GitHub API rate limit exceeded; retry later or pin SQLODE_VERSION").
- Optionally fall back to resolving
latestvia the redirect fromhttps://github.com/${REPO}/releases/latest/download/…orcurl -sI .../releases/latestand parsing theLocation:header, which is not rate-limited the same way.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/install.sh` around lines 66 - 75, The resolve_version function should handle GitHub API rate limits and provide a clearer error: when tag resolution fails, update the die message (used in resolve_version) to mention GitHub API rate limiting and advise retrying later or pinning VERSION/SQLODE_VERSION; additionally implement an optional fallback inside resolve_version that, if the API call returns empty, performs a HEAD request to "https://github.com/${REPO}/releases/latest" (e.g., curl -sI) and parses the Location header to extract the redirect tag and set VERSION accordingly, falling back to the improved die message only if both methods fail; reference resolve_version, VERSION, REPO, and die in your changes.
69-69: Minor: sed pattern assumes pretty-printed JSON.
sed -n 's/.*"tag_name": *"\([^"]*\)".*/\1/p' | head -n1works because GitHub's API currently pretty-prints responses (one JSON key per line). If that ever changes to minified output, this picks the firsttag_nameon a single line, which is still the release object's tag — likely still correct, but fragile. Acceptable for now given the "no jq" constraint; leaving a note for future hardening.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/install.sh` at line 69, The current extraction for tag (the tag="$(curl ... | sed -n 's/.*"tag_name": *"\([^"]*\)".*/\1/p' | head -n1)") relies on pretty-printed JSON; replace the sed+head approach with a more robust single-line extractor such as using grep -oP (e.g. grep -oP -m1 '"tag_name"\s*:\s*"\K[^"]+') or an equivalent POSIX-safe pattern that matches "tag_name":<optional spaces>"<value>" anywhere in the stream so tag is correctly captured even if the JSON is minified.
77-94: Temp file can leak on interrupt between download and install_bin.With
set -euand notrap, if the user hits Ctrl-C afterdownload()completes but before/duringinstall_bin(), or ifmkdir -p "$INSTALL_DIR"fails (e.g. permission denied when users follow the README'sSQLODE_INSTALL_DIR=/usr/local/binsuggestion without sudo), themktemp'd file in$TMPDIRis left behind.♻️ Suggested cleanup trap
download() { url="https://github.com/${REPO}/releases/download/${VERSION}/${BIN_NAME}" info "downloading ${BIN_NAME} ${VERSION} from ${url}" tmp="$(mktemp -t sqlode.XXXXXX)" + # shellcheck disable=SC2064 + trap "rm -f \"$tmp\"" EXIT INT HUP TERM if ! curl -fsSL -o "$tmp" "$url"; then rm -f "$tmp" die "failed to download ${url}. Check SQLODE_VERSION or visit https://github.com/${REPO}/releases." fi DOWNLOAD="$tmp" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/install.sh` around lines 77 - 94, The download() function creates a temporary file with mktemp and can leak it if the script is interrupted before install_bin() moves/removes it; add a cleanup trap so the temp file is removed on EXIT/ERR/INT if it's still present. Implement a trap that checks the DOWNLOAD variable and rm -f "$DOWNLOAD" on script exit/failure (set via trap '...' EXIT ERR INT) or register a per-download trap that is cleared after successful mv in install_bin(); update download(), install_bin(), and the top-level script init to set/clear that trap so temporary files are reliably deleted even if mkdir -p or mv fails. Ensure references to the DOWNLOAD variable and functions download() and install_bin() are used when adding and clearing the trap.README.md (1)
20-22: Pipingcurl | shis the documented install path — consider offering a safer alternative.Standard nit for public installers: recommend documenting a two-step alternative next to the one-liner so security-conscious users can inspect the script first:
```console curl -fsSL https://raw.githubusercontent.com/nao1215/sqlode/main/scripts/install.sh | sh
+Prefer to inspect first? Download, review, then run:
+console +curl -fsSL -o install.sh https://raw.githubusercontent.com/nao1215/sqlode/main/scripts/install.sh +sh install.sh +<details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 20 - 22, Add a safer two-step install alternative
next to the existing one-liner curl | sh command in the README: keep the
original curl -fsSL ... | sh example, then add a short paragraph titled like
"Prefer to inspect first?" followed by the two-step approach that downloads the
script (using curl -o or similar) so users can review the file before executing,
and then runs it (sh install.sh); update the surrounding text to present both
options clearly so security-conscious users can choose the inspect-then-run
flow.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@README.md:
- Line 24: Update the README to note that overriding SQLODE_INSTALL_DIR to a
system path like /usr/local/bin requires root, and show a safe example (e.g.
prepend sudo: sudo SQLODE_INSTALL_DIR=/usr/local/bin sh ./scripts/install.sh) or
prefer a user-writable example such as $HOME/.local/bin; reference the installer
behavior in install_bin() so readers understand mkdir/mv/chmod operations will
fail without elevated privileges.In
@scripts/install.sh:
- Around line 77-86: The download() function currently saves the release asset
without integrity checks; update it to fetch a released checksum (e.g.,
${BIN_NAME}.sha256 or ${VERSION}.sha256) from the same GitHub release and verify
the downloaded file before returning. Modify download() to: after successful
curl of the binary, attempt to curl the checksum file, detect available verifier
(sha256sum or shasum -a 256), validate the binary against the checksum, rm -f
and die on mismatch or missing verifier, and only set DOWNLOAD="$tmp" if
verification passes; keep a clear error message mentioning the checksum URL and
verification failure. Ensure behavior degrades gracefully (fail fast) when
checksum is missing or verification tools are unavailable.- Around line 88-94: The install_bin function currently assumes INSTALL_DIR is
writable and will produce a raw permission error if not; update install_bin to
first check that INSTALL_DIR is writable/creatable (e.g., test -w "$INSTALL_DIR"
or attempt mkdir -p and check its exit) and if it is not writable print a clear,
actionable error that suggests re-running with elevated privileges (for example
"re-run with sudo -E sh -c 'SQLODE_INSTALL_DIR=/usr/local/bin ...'") or using a
user-local path like "$HOME/.local/bin", then exit non-zero; reference symbols:
install_bin, INSTALL_DIR, BIN_NAME, DOWNLOAD, INSTALLED so the check happens
before mv/chmod and no raw mv permission-denied is shown.
Nitpick comments:
In@README.md:
- Around line 20-22: Add a safer two-step install alternative next to the
existing one-liner curl | sh command in the README: keep the original curl -fsSL
... | sh example, then add a short paragraph titled like "Prefer to inspect
first?" followed by the two-step approach that downloads the script (using curl
-o or similar) so users can review the file before executing, and then runs it
(sh install.sh); update the surrounding text to present both options clearly so
security-conscious users can choose the inspect-then-run flow.In
@scripts/install.sh:
- Around line 66-75: The resolve_version function should handle GitHub API rate
limits and provide a clearer error: when tag resolution fails, update the die
message (used in resolve_version) to mention GitHub API rate limiting and advise
retrying later or pinning VERSION/SQLODE_VERSION; additionally implement an
optional fallback inside resolve_version that, if the API call returns empty,
performs a HEAD request to "https://github.com/${REPO}/releases/latest" (e.g.,
curl -sI) and parses the Location header to extract the redirect tag and set
VERSION accordingly, falling back to the improved die message only if both
methods fail; reference resolve_version, VERSION, REPO, and die in your changes.- Line 69: The current extraction for tag (the tag="$(curl ... | sed -n
's/."tag_name": "([^"])"./\1/p' | head -n1)") relies on pretty-printed
JSON; replace the sed+head approach with a more robust single-line extractor
such as using grep -oP (e.g. grep -oP -m1 '"tag_name"\s*:\s*"\K[^"]+') or an
equivalent POSIX-safe pattern that matches "tag_name":""
anywhere in the stream so tag is correctly captured even if the JSON is
minified.- Around line 77-94: The download() function creates a temporary file with
mktemp and can leak it if the script is interrupted before install_bin()
moves/removes it; add a cleanup trap so the temp file is removed on EXIT/ERR/INT
if it's still present. Implement a trap that checks the DOWNLOAD variable and rm
-f "$DOWNLOAD" on script exit/failure (set via trap '...' EXIT ERR INT) or
register a per-download trap that is cleared after successful mv in
install_bin(); update download(), install_bin(), and the top-level script init
to set/clear that trap so temporary files are reliably deleted even if mkdir -p
or mv fails. Ensure references to the DOWNLOAD variable and functions download()
and install_bin() are used when adding and clearing the trap.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `5323803f-96eb-4a39-9d65-0ecd09cd5e08` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 6c99a2b52ff7f1df661ae0dda6023ed2faf490c5 and bf91fbb4930b6f2d4d0817322b24e0177c18dae5. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `.github/workflows/release.yml` * `README.md` * `scripts/install.sh` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| download() { | ||
| url="https://github.com/${REPO}/releases/download/${VERSION}/${BIN_NAME}" | ||
| info "downloading ${BIN_NAME} ${VERSION} from ${url}" | ||
| tmp="$(mktemp -t sqlode.XXXXXX)" | ||
| if ! curl -fsSL -o "$tmp" "$url"; then | ||
| rm -f "$tmp" | ||
| die "failed to download ${url}. Check SQLODE_VERSION or visit https://github.com/${REPO}/releases." | ||
| fi | ||
| DOWNLOAD="$tmp" | ||
| } |
There was a problem hiding this comment.
Consider adding checksum/signature verification for the downloaded binary.
This script is intended to be run via curl ... | sh, and it then fetches and chmod +xes another binary from the internet without any integrity check. HTTPS from github.com is a reasonable baseline, but a compromised release asset, a proxy MITM, or a partial download (unlikely with -f, but possible) would be silently installed and executed by verify(). Publishing a sqlode.sha256 alongside each release and verifying it here (when sha256sum/shasum is available) would materially improve the security posture of the public install path, especially since this is the path promoted as "recommended" in the README.
Not a blocker for v0.1.0, but worth tracking as a follow-up before adoption grows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/install.sh` around lines 77 - 86, The download() function currently
saves the release asset without integrity checks; update it to fetch a released
checksum (e.g., ${BIN_NAME}.sha256 or ${VERSION}.sha256) from the same GitHub
release and verify the downloaded file before returning. Modify download() to:
after successful curl of the binary, attempt to curl the checksum file, detect
available verifier (sha256sum or shasum -a 256), validate the binary against the
checksum, rm -f and die on mismatch or missing verifier, and only set
DOWNLOAD="$tmp" if verification passes; keep a clear error message mentioning
the checksum URL and verification failure. Ensure behavior degrades gracefully
(fail fast) when checksum is missing or verification tools are unavailable.
- Add EXIT/INT/HUP/TERM trap so the mktemp'd download is always removed if the script dies between download() and install_bin() (Ctrl-C, failing mkdir, etc). - Fall back to parsing the Location header from /releases/latest when the JSON API call returns empty. The API is rate limited to 60 req/h for unauthenticated clients; shared networks and CI runners hit that limit. The HEAD-redirect path is not subject to the same limit, and the die message now mentions both the rate-limit possibility and pinning via SQLODE_VERSION. - Detect non-writable INSTALL_DIR up front in install_bin() and emit a sudo example instead of letting `mv` fail with a raw permission error. Clear DOWNLOAD after a successful move so the cleanup trap does not try to rm the now-installed binary. README changes: - Add a two-step "inspect then run" install snippet alongside the curl | sh one-liner for security-conscious users. - Replace the bare `SQLODE_INSTALL_DIR=/usr/local/bin` example with a `sudo SQLODE_INSTALL_DIR=/usr/local/bin sh` form, and call out that system paths need elevated privileges. - Move environment variable docs into a bullet list so the sudo note is visible alongside the path example. Deferred to follow-up: - Checksum/signature verification needs release.yml to also publish sqlode.sha256 alongside the escript; too large for this PR. - sed `tag_name` extraction is fragile if GitHub minifies the API response, but grep -P is not POSIX and the current API still pretty-prints, so the regex stays.
1 participant
Summary
Lower the friction of the public install story (Issue #136) and make the repo ready for a clean v0.1.0 Hex publish. All changes are docs / scripts / CI — no source code touched.
Changes
scripts/install.sh(new): POSIX installer invoked viacurl -fsSL ... | sh. Resolves the latest release tag through the GitHub API, downloads the escript into$HOME/.local/bin/sqlode, chmods it, and warns when Erlang/OTP is missing (with per-distro install hints: apt / dnf / pacman / apk / brew). RespectsSQLODE_VERSIONandSQLODE_INSTALL_DIRoverrides. Notes when the install directory is not onPATH.README.md: promote the one-line installer as the primary path. Kept the manual escript download and thegleam runworkflow as alternatives. Added Hex version, Hex downloads, and CI badges in the same layoutnao1215/oaspecuses..github/workflows/release.yml: pipeI am not using semantic versioningintogleam publish -y, matching oaspec's workaround so the interactive pre-1.0 semver confirmation does not hang the publish step.Design Decisions
sed(nojqdependency) so the installer runs on minimal environments.install.shis committed unexecuted-on-fetch — users invoke it via a pipedsh. The raw-github URL in the README targetsmain, so tagged releases are unaffected if we later move the script.Limitations
curlandshon the host; works on Linux and macOS, not evaluated on Windows (expected to work from Git Bash / WSL).just all(828 gleam tests + 20 shellspec examples) green. The installer's download path cannot be smoke-tested until a v0.1.0 release asset exists.Closes #136
Summary by CodeRabbit
New Features
Documentation