Skip to content

refactor: remove manual conflicts check for --frozen & --locked#4359

Merged
lucascolley merged 2 commits intoprefix-dev:mainfrom
tdejager:feat/remove-manual-cli-conflict
Aug 15, 2025
Merged

refactor: remove manual conflicts check for --frozen & --locked#4359
lucascolley merged 2 commits intoprefix-dev:mainfrom
tdejager:feat/remove-manual-cli-conflict

Conversation

@tdejager
Copy link
Copy Markdown
Contributor

There was some very custom machinery to check --frozen and --locked conflicts. While just using clap will do, this removes all of that and adds clap conflict. Also removes related tests.

Tested

Ran the commands manually with --locked etc. and through env variables and conflicts seem to work.

@tdejager tdejager requested a review from ruben-arts August 15, 2025 08:53
Copy link
Copy Markdown
Collaborator

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

thanks Tim, LGTM!

@lucascolley lucascolley added the refactor Specifies PR or Issue is related to a refactor label Aug 15, 2025
@lucascolley lucascolley changed the title fix: remove manual conflicts check for --frozen --locked refactor: remove manual conflicts check for --frozen & --locked Aug 15, 2025
@lucascolley lucascolley enabled auto-merge (squash) August 15, 2025 08:57
@lucascolley lucascolley merged commit 48083a7 into prefix-dev:main Aug 15, 2025
57 of 75 checks passed
tdejager added a commit to tdejager/pixi that referenced this pull request Aug 15, 2025
tdejager added a commit to tdejager/pixi that referenced this pull request Aug 15, 2025
tdejager added a commit to tdejager/pixi that referenced this pull request Aug 15, 2025
tdejager added a commit to tdejager/pixi that referenced this pull request Aug 15, 2025
tdejager added a commit to tdejager/pixi that referenced this pull request Aug 15, 2025
@gshiba
Copy link
Copy Markdown
Contributor

gshiba commented Aug 29, 2025

The custom handling was to address #4031 (comment)

$ pixi --version
pixi 0.52.0

$ PIXI_FROZEN=false pixi list --locked > /dev/null  # works fine

$ pixi self-update --version 0.53.0

$ PIXI_FROZEN=false pixi list --locked > /dev/null
error: the argument '--locked' cannot be used with '--frozen'

@tdejager
Copy link
Copy Markdown
Contributor Author

Oh I'm sorry I missed that. I didn't mean to remove your contribution!

I do feel that, it might be better to either stick to the clap machinery. You can also unset the env variable after all, or make something for all current boolean env variables, and future ones as we would otherwise forget to parse "truthy" values for these.

@tdejager
Copy link
Copy Markdown
Contributor Author

That said I'd be happy to bring back the change for now.

@gshiba
Copy link
Copy Markdown
Contributor

gshiba commented Aug 29, 2025

I understand that this is a limitation of clap (and not pixi) but I argue PIXI_FROZEN=false pixi list --locked should not error out.

My preference is that this be rolled back, since it is a breaking change (the command above did not barf in the previous version).

Probably best to continue this discussion in #4466

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Specifies PR or Issue is related to a refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants