chore: move installation scripts from docs repo to here#1951
chore: move installation scripts from docs repo to here#1951lc6464 wants to merge 2 commits intosipeed:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Moves end-user installation tooling into this repository so releases can be installed without relying on the separate docs repo.
Changes:
- Added a cross-platform Bash installer for picoclaw releases (user/system mode, GitHub/CDN sources).
- Added a PowerShell installer covering Windows/Linux/macOS/bsd flows, including PATH updates.
- Added a MaixCam-focused Python installer/uninstaller for arm64/riscv64 targets.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| scripts/install/picoclaw/install-picoclaw.sh | New Bash-based installer that downloads and installs release artifacts, with user/system layouts. |
| scripts/install/picoclaw/install-picoclaw.ps1 | New PowerShell installer that downloads/extracts artifacts and updates PATH for Windows and Unix-like systems. |
| scripts/install/maixcam/install_picoclaw.py | New Python installer/uninstaller tailored for MaixCam environments (arm64/riscv64). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function Invoke-ArtifactDownload { | ||
| param( | ||
| [Parameter(Mandatory = $true)][string]$Url, | ||
| [Parameter(Mandatory = $true)][string]$OutFile | ||
| ) | ||
| Write-Host "Downloading: $Url" | ||
| Invoke-WebRequest -Uri $Url -OutFile $OutFile | ||
| } |
There was a problem hiding this comment.
Invoke-ArtifactDownload downloads release artifacts but does not perform any integrity verification (checksum/signature). For an installer that will unpack/install executables (potentially as Administrator/root), consider verifying against a published checksums.txt (or signature) from the same release before proceeding.
| freebsd) echo "freebsd" ;; | ||
| netbsd) echo "netbsd" ;; | ||
| mingw*|msys*|cygwin*) | ||
| die "Windows shell detected ($u). Please run the PowerShell installer instead: pwsh ./static/scripts/picoclaw/install-picoclaw.ps1 -InstallMode user" |
There was a problem hiding this comment.
The Windows-shell error message points to pwsh ./static/scripts/picoclaw/install-picoclaw.ps1 ..., but this repo’s PowerShell installer is located at scripts/install/picoclaw/install-picoclaw.ps1. As written, the guidance will send users to a non-existent path.
| die "Windows shell detected ($u). Please run the PowerShell installer instead: pwsh ./static/scripts/picoclaw/install-picoclaw.ps1 -InstallMode user" | |
| die "Windows shell detected ($u). Please run the PowerShell installer instead: pwsh ./scripts/install/picoclaw/install-picoclaw.ps1 -InstallMode user" |
| fi | ||
|
|
||
| if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then | ||
| printf 'Error: GNU bash 4+ is required. Current version: %s\n' "$BASH_VERSION" >&2 |
There was a problem hiding this comment.
The installer enforces GNU bash 4+ (BASH_VERSINFO[0] < 4), which will fail on stock macOS (ships bash 3.2) even though this script supports macOS assets. Consider either lowering/removing the version constraint (if no bash-4-only features are used) or updating the UX to instruct macOS users to run with a newer bash (e.g., brew install bash and execute via that binary).
| printf 'Error: GNU bash 4+ is required. Current version: %s\n' "$BASH_VERSION" >&2 | |
| printf 'Error: GNU bash 4+ is required. Current version: %s\n\n' "$BASH_VERSION" >&2 | |
| printf 'On macOS, /bin/bash is often an older 3.x version.\n' >&2 | |
| printf 'To install a newer GNU bash using Homebrew:\n' >&2 | |
| printf ' brew install bash\n' >&2 | |
| printf 'Then re-run this installer explicitly with the new bash, for example:\n' >&2 | |
| printf ' /usr/local/bin/bash %s [arguments...]\n' "$0" >&2 |
| with tarfile.open(tar_file, "r:gz") as tar: | ||
| tar.extractall(path=install_dir) | ||
|
|
There was a problem hiding this comment.
tar.extractall() is used on a downloaded archive without validating member paths. A malicious tarball can write files outside /opt/picoclaw via ../ or absolute paths (path traversal). Use a safe extraction routine that validates each member’s resolved path stays within the destination directory before extracting.
| def cleanup_dir(target_dir: Path) -> None: | ||
| if target_dir.exists(): | ||
| shutil.rmtree(target_dir) | ||
|
|
There was a problem hiding this comment.
cleanup_dir() deletes the entire /root/picoclaw directory. Since ensure_picoclaw_dir() uses exist_ok=True, this directory may pre-exist and contain unrelated user data; running the installer would then delete it. Use a unique temporary download directory (e.g., under /tmp), or only delete the specific downloaded file(s) created by this run.
|
|
||
| log "Downloading: $url" | ||
| if command_exists curl; then | ||
| curl -fsSL "$url" -o "$out" | ||
| return | ||
| fi | ||
| if command_exists wget; then | ||
| wget -qO "$out" "$url" | ||
| return | ||
| fi | ||
| die "Neither curl nor wget is available." |
There was a problem hiding this comment.
Downloads are performed over HTTPS but without any integrity verification (e.g., validating against checksums.txt from the release). Since this is an installer fetching and executing binaries as the user (or root in system mode), consider downloading the corresponding checksums/signature and verifying before extracting/installing.
| log "Downloading: $url" | |
| if command_exists curl; then | |
| curl -fsSL "$url" -o "$out" | |
| return | |
| fi | |
| if command_exists wget; then | |
| wget -qO "$out" "$url" | |
| return | |
| fi | |
| die "Neither curl nor wget is available." | |
| local checksum="${3:-}" | |
| local checksum_type="${4:-sha256}" | |
| log "Downloading: $url" | |
| if command_exists curl; then | |
| curl -fsSL "$url" -o "$out" | |
| elif command_exists wget; then | |
| wget -qO "$out" "$url" | |
| else | |
| die "Neither curl nor wget is available." | |
| fi | |
| # Optional integrity verification: if an expected checksum is provided, | |
| # verify the downloaded file before proceeding. | |
| if [[ -n "$checksum" ]]; then | |
| local actual | |
| case "$checksum_type" in | |
| sha256) | |
| if command_exists sha256sum; then | |
| actual="$(sha256sum "$out" | awk '{print $1}')" | |
| elif command_exists shasum; then | |
| actual="$(shasum -a 256 "$out" | awk '{print $1}')" | |
| else | |
| die "Checksum verification requested, but no sha256sum or shasum found." | |
| fi | |
| ;; | |
| *) | |
| die "Unsupported checksum type: $checksum_type" | |
| ;; | |
| esac | |
| if [[ "$actual" != "$checksum" ]]; then | |
| die "Checksum verification failed for $out (expected $checksum, got $actual)." | |
| fi | |
| fi |
| [Parameter(Mandatory = $true)][string]$BinDir | ||
| ) | ||
|
|
||
| $line = 'export PATH="$HOME/.local/bin:$PATH"' |
There was a problem hiding this comment.
Set-UnixUserPathExport accepts a BinDir parameter but ignores it and always writes export PATH="$HOME/.local/bin:$PATH". This makes the function misleading and will be wrong if BinDir ever differs from $HOME/.local/bin.
| $line = 'export PATH="$HOME/.local/bin:$PATH"' | |
| # Represent BinDir in a portable way for shell rc files: | |
| # - If BinDir is under HomeDir, rewrite the HomeDir prefix as $HOME. | |
| # - Otherwise, use BinDir as-is. | |
| $binPathForExport = $BinDir | |
| if ($BinDir.StartsWith($HomeDir, [StringComparison]::Ordinal)) { | |
| $relative = $BinDir.Substring($HomeDir.Length) | |
| if (-not $relative.StartsWith('/', [StringComparison]::Ordinal)) { | |
| $relative = '/' + $relative | |
| } | |
| $binPathForExport = '$HOME' + $relative | |
| } | |
| # Use a literal $PATH so the target shell expands it, not PowerShell. | |
| $line = "export PATH=""$binPathForExport:`$PATH""" |
| New-Item -ItemType File -Path $rcPath -Force | Out-Null | ||
| } | ||
|
|
||
| $hasLine = Select-String -Path $rcPath -Pattern [regex]::Escape($line) -SimpleMatch -Quiet -ErrorAction SilentlyContinue |
There was a problem hiding this comment.
Set-UnixUserPathExport uses Select-String -SimpleMatch but also escapes the pattern via [regex]::Escape($line). With -SimpleMatch, the escaped backslashes become literal and the existing line will never match, so the PATH export can be appended repeatedly on every run.
| $hasLine = Select-String -Path $rcPath -Pattern [regex]::Escape($line) -SimpleMatch -Quiet -ErrorAction SilentlyContinue | |
| $hasLine = Select-String -Path $rcPath -Pattern [regex]::Escape($line) -Quiet -ErrorAction SilentlyContinue |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| Get-ChildItem -Path '/tmp' -Filter 'picoclaw-*' -Directory -ErrorAction SilentlyContinue | | ||
| ForEach-Object { | ||
| if ($_.FullName -ne $tempInstallRoot) { | ||
| Remove-Item -LiteralPath $_.FullName -Recurse -Force -ErrorAction SilentlyContinue | ||
| } | ||
| } | ||
| } catch { | ||
| # Best-effort cleanup; ignore any failures to avoid breaking installation. | ||
| } | ||
|
|
There was a problem hiding this comment.
This function deletes all /tmp/picoclaw-* directories (except the one it just named). That can remove unrelated user data or other concurrent installs, and it runs even though /tmp is shared across users. Avoid deleting broad globs in /tmp; only remove the directory you created for this run (or use a GUID-based temp dir and rely on OS cleanup).
| try { | |
| Get-ChildItem -Path '/tmp' -Filter 'picoclaw-*' -Directory -ErrorAction SilentlyContinue | | |
| ForEach-Object { | |
| if ($_.FullName -ne $tempInstallRoot) { | |
| Remove-Item -LiteralPath $_.FullName -Recurse -Force -ErrorAction SilentlyContinue | |
| } | |
| } | |
| } catch { | |
| # Best-effort cleanup; ignore any failures to avoid breaking installation. | |
| } |
| return matches[0] | ||
|
|
||
|
|
||
| def install_riscv64(tar_file: Path, work_dir: Path) -> None: |
There was a problem hiding this comment.
install_riscv64() takes a work_dir parameter but never uses it. This makes the function signature misleading and can confuse callers/maintainers; either remove the parameter or use it (e.g., as the working directory for downloads/extraction).
| def install_riscv64(tar_file: Path, work_dir: Path) -> None: | |
| def install_riscv64(tar_file: Path, work_dir: Path) -> None: | |
| # Ensure the provided work directory exists so it can be used as a staging area if needed. | |
| work_dir.mkdir(parents=True, exist_ok=True) |
| def ensure_picoclaw_dir() -> Path: | ||
| target_dir = Path("/root/picoclaw") | ||
| target_dir.mkdir(parents=True, exist_ok=True) | ||
| return target_dir |
There was a problem hiding this comment.
This installer writes into /root, /opt, and /usr/bin but doesn't check for root privileges up front. When run without sufficient permissions it will fail mid-way with a less clear exception (e.g., PermissionError). Consider asserting effective UID==0 early (or documenting that it must be run as root) and exiting with a clear message.
| New-Item -ItemType File -Path $rcPath -Force | Out-Null | ||
| } | ||
|
|
||
| $hasLine = Select-String -Path $rcPath -Pattern [regex]::Escape($line) -SimpleMatch -Quiet -ErrorAction SilentlyContinue |
There was a problem hiding this comment.
Select-String is invoked with both -SimpleMatch and a regex-escaped pattern. With -SimpleMatch, the backslashes inserted by [regex]::Escape($line) are treated literally, so the check will not match an existing PATH export line and the script will append duplicates on each run. Use either -Pattern $line -SimpleMatch or drop -SimpleMatch and keep the regex pattern.
| $hasLine = Select-String -Path $rcPath -Pattern [regex]::Escape($line) -SimpleMatch -Quiet -ErrorAction SilentlyContinue | |
| $hasLine = Select-String -Path $rcPath -Pattern $line -SimpleMatch -Quiet -ErrorAction SilentlyContinue |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
e4da51f to
79dff4a
Compare
|
@lc6464 Hi! This PR has been inactive for over a week. If there's no update in the next 7 days, it will be closed automatically. If you're still working on it, just leave a comment to keep it open! |
|
👀 |
📝 Description
Move installation scripts from docs repo to here.
🔗 Related PR
sipeed/picoclaw_docs#14 (comment)