feat: Added bundler and install scripts (for dev) #8
Conversation
📝 WalkthroughWalkthroughThis pull request introduces build and installation automation for the Tiles CLI project. Two new shell scripts are added: Sequence Diagram(s)sequenceDiagram
participant User
participant Bundler as bundler.sh
participant Cargo
participant Filesystem as dist/
User->>Bundler: Execute bundle recipe
Bundler->>Bundler: Extract version from Cargo.toml
Bundler->>Bundler: Detect OS & architecture
Bundler->>Cargo: cargo build --release
Cargo-->>Bundler: binary compiled
Bundler->>Filesystem: Create dist/tmp staging
Bundler->>Filesystem: Copy binary + server dir
Bundler->>Filesystem: Clean *.pyc & venv
Bundler->>Filesystem: Create .tar.gz archive
Bundler->>Filesystem: Remove dist/tmp
Bundler-->>User: Report tarball path
sequenceDiagram
participant User
participant Installer as install.sh
participant Env as Environment
participant Pkg as Package Manager
participant GitHub as GitHub Releases
participant Local as Local dist/
participant Filesystem as ~/.local/
User->>Installer: Execute install recipe
Installer->>Env: Check Python 3
alt Python not found
Installer->>Pkg: Attempt brew/apt install
Pkg-->>Installer: success or fail
alt Install failed
Installer-->>User: Prompt manual install
end
end
Installer->>Env: Check uv
alt uv not found
Installer->>GitHub: Download uv installer
GitHub-->>Installer: Installer script
end
Installer->>Env: Check ENV variable
alt prod environment
Installer->>GitHub: Download tiles tarball
GitHub-->>Local: Archive received
else dev/local
Installer->>Local: Use local dist/tarball
end
Installer->>Filesystem: Extract to tmp
Installer->>Filesystem: Install binary to ~/.local/bin
Installer->>Filesystem: Stage server to ~/.local/share/tiles/server
Installer->>Filesystem: Run uv sync in server dir
Installer->>Filesystem: Clean temp files
Installer-->>User: Success message + PATH reminder
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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: 5
🧹 Nitpick comments (5)
scripts/bundler.sh (2)
11-12: Consider normalizing ARCH for cross-platform consistency.
uname -moutput varies across platforms (e.g., x86_64 vs amd64, aarch64 vs arm64). While this will match between bundler and installer since both useuname -m, consider normalizing to standard identifiers for consistency with common distribution conventions.Example normalization:
RAW_ARCH=$(uname -m) case "$RAW_ARCH" in x86_64|amd64) ARCH="x86_64" ;; aarch64|arm64) ARCH="aarch64" ;; *) ARCH="$RAW_ARCH" ;; esac OUT_NAME="${BINARY_NAME}-v${VERSION}-${ARCH}-${OS}"
17-19: Add validation for required artifacts.The script assumes the binary and server directory exist after the build. If the build produces the binary in a different location or the server directory is missing, the
cpcommands will fail silently or with unclear errors.Apply this diff to add validation:
mkdir -p "${DIST_DIR}/tmp" +if [[ ! -f "target/${TARGET}/${BINARY_NAME}" ]]; then + echo "❌ Error: Binary not found at target/${TARGET}/${BINARY_NAME}" >&2 + exit 1 +fi +if [[ ! -d "${SERVER_DIR}" ]]; then + echo "❌ Error: Server directory not found at ${SERVER_DIR}" >&2 + exit 1 +fi cp "target/${TARGET}/${BINARY_NAME}" "${DIST_DIR}/tmp/" cp -r "${SERVER_DIR}" "${DIST_DIR}/tmp/"scripts/install.sh (3)
4-7: Consider reading ENV and VERSION from environment variables.Both
ENVandVERSIONare hardcoded, but the comments suggest they should be configurable. For production use, these should be read from environment variables with fallback defaults.Apply this diff:
-ENV="dev" # prod is another env, try taking it from github env +ENV="${TILES_ENV:-dev}" # Set TILES_ENV=prod for production REPO="tilesprivacy/tilekit" -# VERSION="${TILES_VERSION:-latest}" -VERSION="0.1.0" +VERSION="${TILES_VERSION:-0.1.0}"
10-10: Add trap to ensure TMPDIR cleanup on exit.If the script exits early due to an error, the temporary directory at
$TMPDIRis not cleaned up. This can accumulate stale directories over time.Apply this diff to add a cleanup trap:
TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXITThis ensures cleanup happens even on early exit.
59-65: Consider runninguv syncin a subshell to avoid directory change side effects.The
cd "${SERVER_DIR}"changes the current directory for the remainder of the script. While this works, using a subshell is cleaner and prevents potential issues if the script is extended later.Apply this diff:
log "📦 Installing Python server to ${SERVER_DIR}..." mkdir -p "${SERVER_DIR}" cp -r "${TMPDIR}/server"/* "${SERVER_DIR}/" log "🔧 Setting up Python environment..." -cd "${SERVER_DIR}" -uv sync --frozen || err "Dependency setup failed." +(cd "${SERVER_DIR}" && uv sync --frozen) || err "Dependency setup failed."
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
justfile(1 hunks)scripts/bundler.sh(1 hunks)scripts/install.sh(1 hunks)
🔇 Additional comments (10)
scripts/bundler.sh (5)
1-2: LGTM!Strict error handling with
set -euo pipefailis a best practice for build scripts.
4-8: LGTM!Constants are clearly defined and appropriately scoped.
14-15: LGTM!Build command is straightforward and correct.
21-22: LGTM!Cleanup of Python artifacts is appropriate for distribution.
24-29: LGTM!Tarball creation and cleanup are correctly implemented. The
-Cflag ensures the archive contains a clean directory structure.scripts/install.sh (4)
1-2: LGTM!Strict error handling with
set -euo pipefailis essential for installation scripts.
15-16: LGTM!Helper functions for logging are well-implemented with appropriate color coding and error handling.
51-57: LGTM!Extraction and binary installation are correctly implemented with appropriate permissions via the
installcommand.
67-71: LGTM!Cleanup and success messaging are clear and appropriate. The PATH reminder is helpful for users.
justfile (1)
17-21: LGTM!The new
bundleandinstallrecipes are straightforward and correctly delegate to their respective scripts.
| SERVER_DIR="server" | ||
| TARGET="release" | ||
|
|
||
| VERSION=$(grep '^version' Cargo.toml | head -1 | awk -F'"' '{print $2}') |
There was a problem hiding this comment.
Validate VERSION extraction and handle edge cases.
The version extraction assumes a specific format in Cargo.toml and does not validate the result. If the pattern fails to match or the field is malformed, VERSION will be empty, leading to a malformed tarball name.
Apply this diff to add validation:
VERSION=$(grep '^version' Cargo.toml | head -1 | awk -F'"' '{print $2}')
+if [[ -z "$VERSION" ]]; then
+ echo "❌ Error: Could not extract version from Cargo.toml" >&2
+ exit 1
+fi📝 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.
| VERSION=$(grep '^version' Cargo.toml | head -1 | awk -F'"' '{print $2}') | |
| VERSION=$(grep '^version' Cargo.toml | head -1 | awk -F'"' '{print $2}') | |
| if [[ -z "$VERSION" ]]; then | |
| echo "❌ Error: Could not extract version from Cargo.toml" >&2 | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In scripts/bundler.sh around line 9, the VERSION extraction from Cargo.toml can
yield an empty or malformed value if the expected pattern isn't present; update
the script to validate the extracted VERSION is non-empty and matches a
semver-like pattern (e.g., digits and dots), and if validation fails print a
clear error to stderr and exit non‑zero to avoid creating a malformed tarball
name; optionally attempt a safer extraction (use a more robust grep/regex)
before validating.
| echo "🔍 Checking Python..." | ||
| if ! command -v python3 >/dev/null 2>&1; then | ||
| log "⚠️ Python 3.10+ not found." | ||
| if [[ "$OS" == "darwin" ]]; then | ||
| log "Installing via Homebrew..." | ||
| brew install python || err "Could not install Python automatically. Please install manually." | ||
| elif [[ -f /etc/debian_version ]]; then | ||
| log "Installing via apt..." | ||
| sudo apt-get update -y && sudo apt-get install -y python3 python3-venv | ||
| else | ||
| err "Please install Python manually: https://www.python.org/downloads/" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
Python version not validated despite "3.10+" requirement.
The log message mentions "Python 3.10+" but the script only checks if python3 exists, not its version. If an older Python 3 is installed, the installation may fail later during uv sync.
Apply this diff to add version validation:
echo "🔍 Checking Python..."
if ! command -v python3 >/dev/null 2>&1; then
log "⚠️ Python 3.10+ not found."
+ # Install Python
if [[ "$OS" == "darwin" ]]; then
log "Installing via Homebrew..."
brew install python || err "Could not install Python automatically. Please install manually."
elif [[ -f /etc/debian_version ]]; then
log "Installing via apt..."
sudo apt-get update -y && sudo apt-get install -y python3 python3-venv
else
err "Please install Python manually: https://www.python.org/downloads/"
fi
+fi
+
+# Validate Python version
+PYTHON_VERSION=$(python3 -c 'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")' 2>/dev/null || echo "0.0")
+REQUIRED_VERSION="3.10"
+if ! python3 -c "import sys; exit(0 if sys.version_info >= (3, 10) else 1)" 2>/dev/null; then
+ err "Python ${REQUIRED_VERSION}+ is required, but found ${PYTHON_VERSION}. Please upgrade."
fi🏁 Script executed:
#!/bin/bash
# Examine the full install.sh file to understand context
cat scripts/install.shLength of output: 2245
🏁 Script executed:
#!/bin/bash
# Search for Python version requirements in the repository
rg -n "3\.10|python" --type=toml --type=txt --type=yaml -C2 | head -50Length of output: 2587
🏁 Script executed:
#!/bin/bash
# Check if there's a uv.lock or pyproject.toml with Python version spec
fd -e toml -e lock | xargs ls -la 2>/dev/null | head -20Length of output: 333
Python version not validated despite 3.10+ requirement.
The script checks only if python3 exists but never validates its version. The server/pyproject.toml requires >=3.10, so a user with Python 3.9 would pass the check yet fail later during uv sync.
Apply the suggested diff to add version validation after the installation block:
+# Validate Python version
+PYTHON_VERSION=$(python3 -c 'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")' 2>/dev/null || echo "0.0")
+REQUIRED_VERSION="3.10"
+if ! python3 -c "import sys; exit(0 if sys.version_info >= (3, 10) else 1)" 2>/dev/null; then
+ err "Python ${REQUIRED_VERSION}+ is required, but found ${PYTHON_VERSION}. Please upgrade."
+fi📝 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.
| echo "🔍 Checking Python..." | |
| if ! command -v python3 >/dev/null 2>&1; then | |
| log "⚠️ Python 3.10+ not found." | |
| if [[ "$OS" == "darwin" ]]; then | |
| log "Installing via Homebrew..." | |
| brew install python || err "Could not install Python automatically. Please install manually." | |
| elif [[ -f /etc/debian_version ]]; then | |
| log "Installing via apt..." | |
| sudo apt-get update -y && sudo apt-get install -y python3 python3-venv | |
| else | |
| err "Please install Python manually: https://www.python.org/downloads/" | |
| fi | |
| fi | |
| echo "🔍 Checking Python..." | |
| if ! command -v python3 >/dev/null 2>&1; then | |
| log "⚠️ Python 3.10+ not found." | |
| if [[ "$OS" == "darwin" ]]; then | |
| log "Installing via Homebrew..." | |
| brew install python || err "Could not install Python automatically. Please install manually." | |
| elif [[ -f /etc/debian_version ]]; then | |
| log "Installing via apt..." | |
| sudo apt-get update -y && sudo apt-get install -y python3 python3-venv | |
| else | |
| err "Please install Python manually: https://www.python.org/downloads/" | |
| fi | |
| fi | |
| # Validate Python version | |
| PYTHON_VERSION=$(python3 -c 'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")' 2>/dev/null || echo "0.0") | |
| REQUIRED_VERSION="3.10" | |
| if ! python3 -c "import sys; exit(0 if sys.version_info >= (3, 10) else 1)" 2>/dev/null; then | |
| err "Python ${REQUIRED_VERSION}+ is required, but found ${PYTHON_VERSION}. Please upgrade." | |
| fi |
🤖 Prompt for AI Agents
In scripts/install.sh around lines 18 to 30, the script only checks for the
existence of python3 but does not verify the version requirement (>=3.10); after
the install/installation branch completes, run a Python version check using
python3 to ensure sys.version_info >= (3,10) and fail with a clear error and
non-zero exit if the installed python3 is older than 3.10 (or if the check
fails); log a helpful message directing the user to upgrade/install the correct
Python version and stop the script so downstream steps don't break.
| echo "🔍 Checking uv..." | ||
| if ! command -v uv >/dev/null 2>&1; then | ||
| log "⬇️ Installing uv..." | ||
| curl -LsSf https://astral.sh/uv/install.sh | sh | ||
| export PATH="$HOME/.local/bin:$PATH" | ||
| fi |
There was a problem hiding this comment.
Piping remote script to shell without verification poses security risk.
Downloading and executing https://astral.sh/uv/install.sh without checksum or signature verification can be exploited if the download is intercepted or the source is compromised.
Consider adding checksum verification or at least document this security consideration:
log "⬇️ Installing uv..."
# Note: This downloads and executes a script from the internet.
# For enhanced security, consider verifying checksums.
curl -LsSf https://astral.sh/uv/install.sh | shAlternatively, download the script first, verify its checksum, then execute it.
🤖 Prompt for AI Agents
In scripts/install.sh around lines 32 to 37, avoid piping the remote install
script directly to sh; instead download the script to a temporary file,
obtain/expect a checksum or signature (or fetch a published checksum from a
trusted URL), verify the downloaded file against that checksum/signature, and
only then execute the local file (or abort on mismatch). If no checksum is
available, log a clear warning and require explicit user confirmation before
executing the downloaded script; keep exporting PATH after successful install.
Ensure the script fails fast on verification errors and does not execute
unverified remote code.
| if [[ "$ENV" == "prod" ]]; then | ||
| TAR_URL="https://github.com/${REPO}/releases/download/${VERSION}/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" | ||
| curl -fsSL -o "${TMPDIR}/tiles.tar.gz" "$TAR_URL" | ||
| else | ||
| # Installer suppose to ran from tilekit root folder after running the bundler | ||
| mv "dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" "${TMPDIR}/tiles.tar.gz" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate tarball exists before proceeding.
In dev mode, the script assumes the bundled tarball exists but doesn't verify it. If the bundler wasn't run or failed, the mv/cp will fail with an unclear error.
Apply this diff to add validation:
if [[ "$ENV" == "prod" ]]; then
TAR_URL="https://github.com/${REPO}/releases/download/${VERSION}/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz"
curl -fsSL -o "${TMPDIR}/tiles.tar.gz" "$TAR_URL"
else
# Installer suppose to ran from tilekit root folder after running the bundler
+ TARBALL="dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz"
+ if [[ ! -f "$TARBALL" ]]; then
+ err "Tarball not found at $TARBALL. Run 'just bundle' first."
+ fi
- mv "dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" "${TMPDIR}/tiles.tar.gz"
+ cp "$TARBALL" "${TMPDIR}/tiles.tar.gz"
fi📝 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.
| if [[ "$ENV" == "prod" ]]; then | |
| TAR_URL="https://github.com/${REPO}/releases/download/${VERSION}/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" | |
| curl -fsSL -o "${TMPDIR}/tiles.tar.gz" "$TAR_URL" | |
| else | |
| # Installer suppose to ran from tilekit root folder after running the bundler | |
| mv "dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" "${TMPDIR}/tiles.tar.gz" | |
| fi | |
| if [[ "$ENV" == "prod" ]]; then | |
| TAR_URL="https://github.com/${REPO}/releases/download/${VERSION}/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" | |
| curl -fsSL -o "${TMPDIR}/tiles.tar.gz" "$TAR_URL" | |
| else | |
| # Installer suppose to ran from tilekit root folder after running the bundler | |
| TARBALL="dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" | |
| if [[ ! -f "$TARBALL" ]]; then | |
| err "Tarball not found at $TARBALL. Run 'just bundle' first." | |
| fi | |
| cp "$TARBALL" "${TMPDIR}/tiles.tar.gz" | |
| fi |
🤖 Prompt for AI Agents
In scripts/install.sh around lines 43 to 49, the dev branch assumes the bundled
tarball exists and blindly runs mv which will fail with an unclear error if the
bundler didn't produce the file; add an existence check for
"dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" before moving it, and if the file
is missing print a clear error message to stderr and exit with a non-zero status
(otherwise perform the mv to "${TMPDIR}/tiles.tar.gz").
| # Installer suppose to ran from tilekit root folder after running the bundler | ||
| mv "dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" "${TMPDIR}/tiles.tar.gz" |
There was a problem hiding this comment.
Use cp instead of mv to preserve source tarball.
Using mv removes the source tarball from the dist/ directory. If the installer is run multiple times during development or if the installation fails midway, the source tarball will be missing for subsequent attempts.
Apply this diff:
# Installer suppose to ran from tilekit root folder after running the bundler
- mv "dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" "${TMPDIR}/tiles.tar.gz"
+ cp "dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" "${TMPDIR}/tiles.tar.gz" 📝 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.
| # Installer suppose to ran from tilekit root folder after running the bundler | |
| mv "dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" "${TMPDIR}/tiles.tar.gz" | |
| # Installer suppose to ran from tilekit root folder after running the bundler | |
| cp "dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" "${TMPDIR}/tiles.tar.gz" |
🤖 Prompt for AI Agents
In scripts/install.sh around lines 47-48, the installer currently uses mv which
removes the source tarball from dist/; change the command to copy the tarball
instead (use cp -a or cp --preserve=mode,timestamps) to copy
"dist/tiles-v${VERSION}-${ARCH}-${OS}.tar.gz" to "${TMPDIR}/tiles.tar.gz" so the
original remains for retrying or debugging, and ensure the cp command checks for
errors (exit non-zero) as mv did.
No description provided.