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

Add pixi self-update #675

Merged
merged 21 commits into from
Jan 16, 2024
Merged

Add pixi self-update #675

merged 21 commits into from
Jan 16, 2024

Conversation

hadim
Copy link
Contributor

@hadim hadim commented Jan 16, 2024

Supersede #582


You can test it locally in your dev env with:

$ pixi run cargo run -- self-update --force --version 0.8.0
$ ./target/debug/pixi --version
pixi 0.8.0

See the current API:

$ pixi self-update
✔ Pixi will be updated from '0.1.0' to '0.12.0'
✔ Pixi archive downloaded.
✔ Pixi archive uncompressed.
✔ Pixi has been updated to version 0.12.0.
$ pixi self-update # wrong pixi location
✘ pixi is not installed in the default location:
- Default pixi location: /Users/hadim/.pixi/bin/pixi
- Pixi location detected: /Users/hadim/Code/libs/pixi/pixi/target/debug/pixi

It can happen when pixi has been installed via a dedicated package manager (such as Homebrew on macOS).
You can always use `pixi self-update --force` to force the update.
$ pixi self-update --version 0.8.0
✔ Pixi will be updated from '0.1.0' to '0.8.0'
✔ Pixi archive downloaded.
✔ Pixi archive uncompressed.
✔ Pixi has been updated to version 0.8.0.
$ pixi self-update
✔ pixi is already up-to-date (version 0.12.0)
$ pixi self-update --version 0.8.99
✘ The version you specified is not available: 0.8.99

src/cli/self_update.rs Outdated Show resolved Hide resolved
src/cli/self_update.rs Outdated Show resolved Hide resolved
src/cli/self_update.rs Outdated Show resolved Hide resolved
@wolfv
Copy link
Member

wolfv commented Jan 16, 2024

I am happy with the logic. I'll let Bas and Ruben chime in with their thoughts!

src/cli/self_update.rs Outdated Show resolved Hide resolved
src/cli/self_update.rs Outdated Show resolved Hide resolved
src/cli/self_update.rs Show resolved Hide resolved
src/cli/self_update.rs Show resolved Hide resolved
@hadim
Copy link
Contributor Author

hadim commented Jan 16, 2024

@baszalmstra ready for another round!

I removed all the unwrap and replaced most of them by except (when the error is quite unlikely to happen).

I am also now using miette for "expected" errors.

I also replaced all the println to eprintln.

@hadim hadim requested a review from baszalmstra January 16, 2024 19:30
@hadim
Copy link
Contributor Author

hadim commented Jan 16, 2024

I also replaced all the println to eprintln.

I am actually unsure whether those should be printed to stdout or stderr looking at other cli commands.

Copy link
Contributor

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small nitpicks but looks good to me!

src/cli/self_update.rs Outdated Show resolved Hide resolved
src/cli/self_update.rs Outdated Show resolved Hide resolved
src/cli/self_update.rs Outdated Show resolved Hide resolved
src/cli/self_update.rs Outdated Show resolved Hide resolved
hadim and others added 3 commits January 16, 2024 15:37
Co-authored-by: Bas Zalmstra <zalmstra.bas@gmail.com>
Co-authored-by: Bas Zalmstra <zalmstra.bas@gmail.com>
@hadim
Copy link
Contributor Author

hadim commented Jan 16, 2024

Thanks @baszalmstra - should be good now.

@wolfv wolfv merged commit 754039b into prefix-dev:main Jan 16, 2024
11 checks passed
@wolfv
Copy link
Member

wolfv commented Jan 16, 2024

@hadim maybe you can follow up with a few docs. Great feature! :)

@hadim hadim deleted the self_update branch January 16, 2024 21:12
@hadim
Copy link
Contributor Author

hadim commented Jan 16, 2024

@hadim maybe you can follow up with a few docs. Great feature! :)

I already added the cli doc at docs/cli.md in that PR. Do you mean something else?

@wolfv
Copy link
Member

wolfv commented Jan 16, 2024

perfect, nope. sorry. didn't look closely enough 🤦 (getting late over here haha)

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