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

refactor: Make self-update a compile time feature #2213

Merged
merged 10 commits into from
Oct 11, 2024

Conversation

freundTech
Copy link
Contributor

@freundTech freundTech commented Oct 7, 2024

Currently pixi refuses to self-update if it is not installed at its default location. This can be overridden using the --force flag.

This is not ideal for two reasons: Some people might intentionally install pixi to a non-default location, such as ~/.local/bin/pixi, while not using a package manager. On the other hand, passing --force still allows pixi installations installed by a package manager to be broken.

A better solution would be disabling the self-update functionallity at compile time. This PR adds a compile-time feature called self_update, which is not enabled by default.
This PR also enables this feature in the CI build so releases produced by CI can still be self-updated. Because the feature is not enabled by default, other distributions of pixi, such as when built by other package managers, will not support self-update.

Instead of completely removing the self-update sub-command, disabling this feature only replaces it with a stub command that outputs an error message. This is to prevent user-confusion, when users read about self-update in documentation, only to find the sub-command missing in their installation.

With this implemented the default location check and --force flag will not be needed anymore.

Currently pixi refuses to self-update if it is not installed at its
default location. This can be overridden using the `--force` flag.

This is not ideal for two reasons: Some people might intentionally
install pixi to a non-default location, such as `~/.local/bin/pixi`,
while not using a package manager. On the other hand, passing `--force`
still allows pixi installations installed by a package manager to be
broken.

A better solution would be disabling the self-update functionallity at
compile time. This PR adds a compile-time feature called
`self_update`, which is not enabled by default.
This PR also enables this feature in the CI build so releases produced
by CI can still be self-updated. Because the feature is not enabled by
default, other distributions of pixi, such as when built by other package
managers, will not support self-update.

Instead of completely removing the self-update sub-command, disabling
this feature only replaces it with a stub command that outputs an error
message. This is to prevent user-confusion, when users read about
self-update in documentation, only to find the sub-command missing in
their installation.

With this implemented the default location check and `--force` flag will
not be needed anymore.
@freundTech freundTech changed the title Make self-update a compile time feature refactor: Make self-update a compile time feature Oct 7, 2024
@freundTech freundTech marked this pull request as draft October 7, 2024 08:53
@freundTech freundTech marked this pull request as ready for review October 7, 2024 09:40
Copy link
Contributor

@Hofer-Julian Hofer-Julian left a comment

Choose a reason for hiding this comment

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

Hey @freundTech, thanks for your contribution. I haven't tested it yet, but the spirit of this PR looks good to me.

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

@Hofer-Julian Hofer-Julian left a comment

Choose a reason for hiding this comment

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

I checked out the docs

docs/packaging.md Outdated Show resolved Hide resolved
docs/packaging.md Show resolved Hide resolved
docs/packaging.md Outdated Show resolved Hide resolved
docs/packaging.md Outdated Show resolved Hide resolved
freundTech and others added 3 commits October 8, 2024 09:21
Co-authored-by: Hofer-Julian <30049909+Hofer-Julian@users.noreply.github.com>
The top level header is inserted automatically
@ruben-arts
Copy link
Contributor

Thanks for this PR, I think it should be ready. I'll wait with the merge for a little bit as we're dealing with some nasty Rust 1.81 issues and I don't want to mix this PR into to Rust issues.

docs/packaging.md Outdated Show resolved Hide resolved
@ruben-arts ruben-arts enabled auto-merge (squash) October 11, 2024 14:24
@ruben-arts ruben-arts merged commit ddc54f7 into prefix-dev:main Oct 11, 2024
43 checks passed
@pavelzw pavelzw mentioned this pull request Oct 16, 2024
6 tasks
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.

5 participants