-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Uv not getting installed during setup on macos #476
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?
Conversation
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.
cubic analysis
2 issues found across 1 file • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
if [[ "$OS_TYPE" == "macOS" ]]; then | ||
brew install uv || error "Failed to install uv via Homebrew" | ||
else | ||
curl -LsSf https://astral.sh/uv/install.sh | sh || error "Failed to install uv" |
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.
Piping a remotely fetched script directly into sh
is insecure; download the installer to a file, verify its integrity, then execute it instead. (Based on previous feedback about avoiding curl | sh
patterns in production scripts.)
Prompt for AI agents
Address the following comment on setup.sh at line 106:
<comment>Piping a remotely fetched script directly into `sh` is insecure; download the installer to a file, verify its integrity, then execute it instead. (Based on previous feedback about avoiding `curl | sh` patterns in production scripts.)</comment>
<file context>
@@ -99,9 +99,14 @@ success "pip3 is available"
# ensure uv
if ! command -v uv &> /dev/null; then
- info "uv not found; installing via Astral.sh…"
- curl -LsSf https://astral.sh/uv/install.sh | sh
- export PATH="$HOME/.local/bin:$PATH"
+ info "uv not found; installing…"
+ if [[ "$OS_TYPE" == "macOS" ]]; then
+ brew install uv || error "Failed to install uv via Homebrew"
</file context>
export PATH="$HOME/.local/bin:$PATH" | ||
info "uv not found; installing…" | ||
if [[ "$OS_TYPE" == "macOS" ]]; then | ||
brew install uv || error "Failed to install uv via Homebrew" |
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.
brew is invoked without first verifying that Homebrew is installed, so the script will crash with an unclear error on macOS systems that lack Homebrew. Add an explicit command -v brew
check or install instructions before calling brew. (Based on your team's feedback about making setup scripts resilient to missing dependencies.)
Prompt for AI agents
Address the following comment on setup.sh at line 104:
<comment>brew is invoked without first verifying that Homebrew is installed, so the script will crash with an unclear error on macOS systems that lack Homebrew. Add an explicit `command -v brew` check or install instructions before calling brew. (Based on your team's feedback about making setup scripts resilient to missing dependencies.)</comment>
<file context>
@@ -99,9 +99,14 @@ success "pip3 is available"
# ensure uv
if ! command -v uv &> /dev/null; then
- info "uv not found; installing via Astral.sh…"
- curl -LsSf https://astral.sh/uv/install.sh | sh
- export PATH="$HOME/.local/bin:$PATH"
+ info "uv not found; installing…"
+ if [[ "$OS_TYPE" == "macOS" ]]; then
+ brew install uv || error "Failed to install uv via Homebrew"
</file context>
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~6 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings
setup.sh (3)Learnt from: srbhr Learnt from: srbhr Learnt from: elliotclee 🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Pull Request Title
Improving setup script to install uv on macos with homebrew
Related Issue
#475
Description
if macos added the installation script for uv using homebrew
Type
Proposed Changes
use homebrew to install uv on mac
Screenshots / Code Snippets (if applicable)
ensure uv
if ! command -v uv &> /dev/null; then
info "uv not found; installing…"
if [[ "$OS_TYPE" == "macOS" ]]; then
brew install uv || error "Failed to install uv via Homebrew"
else
curl -LsSf https://astral.sh/uv/install.sh | sh || error "Failed to install uv"
export PATH="$HOME/.local/bin:$PATH"
fi
success "uv installed"
fi
check_cmd uv
success "All prerequisites satisfied."
How to Test
Checklist
Additional Information
Summary by cubic
Fixed the setup script so that uv is installed on macOS using Homebrew if it is missing.
Summary by CodeRabbit
uv
tool with OS-specific handling, using Homebrew on macOS and curl-based installation on other systems.