Skip to content
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

🐛 Allow uv to be managed outside of .local/cargo/bin #3348

Merged
merged 3 commits into from
Oct 1, 2024
Merged

Conversation

larsyencken
Copy link
Collaborator

@larsyencken larsyencken commented Oct 1, 2024

Fixes https://github.com/owid/ops/issues/229

  • Lets the user decide where and how to install uv, and makes them responsible for putting it in their PATH
  • Gives an error if uv is not installed, suggesting brew install uv as a resolution for the user

@owidbot
Copy link
Contributor

owidbot commented Oct 1, 2024

Quick links (staging server):

Site Admin Wizard

Login: ssh owid@staging-site-uv-fix

chart-diff: ✅ No charts for review.
data-diff: ✅ No differences found
Legend: +New  ~Modified  -Removed  =Identical  Details
Hint: Run this locally with etl diff REMOTE data/ --include yourdataset --verbose --snippet

Automatically updated datasets matching weekly_wildfires|excess_mortality|covid|fluid|flunet|country_profile|garden/ihme_gbd/2019/gbd_risk are not included

Edited: 2024-10-01 11:21:55 UTC
Execution time: 18.50 seconds

@Marigold
Copy link
Collaborator

Marigold commented Oct 1, 2024

@larsyencken how about this? .cargo/env adds uv to the path which works for CI, and it doesn't do anything for users who installed it via brew.

(Alternatively, I can install uv explicitly in all Buildkite steps & CI)

install-uv-default:
	@if ! command -v uv >/dev/null 2>&1; then \
		echo '==> UV not found. Installing...'; \
		curl -LsSf https://astral.sh/uv/install.sh | sh; \
	fi

.venv-default: install-uv .sanity-check
	@echo '==> Installing packages'
	@if [ -n "$(PYTHON_VERSION)" ]; then \
		echo '==> Using Python version $(PYTHON_VERSION)'; \
		export UV_PYTHON=$(PYTHON_VERSION); \
	fi
	[ -f $$HOME/.cargo/env ] && . $$HOME/.cargo/env || true && uv sync --all-extras

@larsyencken
Copy link
Collaborator Author

@Marigold Overall it looks good, especially if there's nowhere else we need uv, i.e. if all other parts of the recipe just use .venv/bin/<blarg> to do their thing.

@Marigold Marigold merged commit 78937b2 into master Oct 1, 2024
7 of 8 checks passed
@Marigold Marigold deleted the uv-fix branch October 1, 2024 11:24
paarriagadap pushed a commit that referenced this pull request Oct 6, 2024
* 🐛 Allow `uv` to be managed outside of `.local/cargo/bin`

* hotfix uv installation

---------

Co-authored-by: Marigold <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants