Skip to content

refactor: standardise target prefix across commands#2311

Open
agriyakhetarpal wants to merge 9 commits intoconda:mainfrom
agriyakhetarpal:standardise-target-prefix
Open

refactor: standardise target prefix across commands#2311
agriyakhetarpal wants to merge 9 commits intoconda:mainfrom
agriyakhetarpal:standardise-target-prefix

Conversation

@agriyakhetarpal
Copy link
Copy Markdown

@agriyakhetarpal agriyakhetarpal commented Apr 2, 2026

Description

As a follow-up to #2263, which was contributed in by @soapy1, @danyeaw, and me :)

This is a table of what I made out from the current state, i.e., before this PR. I could have made a mistake as I'm only human, of course – please cross-check! The current state is as follows:

Command Field Short Long Alias Type Default CONDA_PREFIX is used? How does the Path resolution
create target_prefix -p --prefix --target-prefix Option<PathBuf> .prefix (runtime) No std::path::absolute
run target_prefix -p --prefix --target-prefix Option<PathBuf> .prefix (runtime) No std::path::absolute
shell_hook target_prefix -p --prefix --target-prefix PathBuf .prefix (clap default_value) No std::path::absolute
menu (i.e., both structs, corresponding to installer-menu and remove-menu) target_prefix -t --target-prefix None PathBuf .prefix (clap default_value) No fs::canonicalize
list prefix -p --prefix None Option<PathBuf> error (runtime) Yes None

Questions

  • What should we do for list, since it uses prefix – is backwards compat a concern? (see changes table below)
  • What should we do for menu commands – similarly, is backwards compat a concern? (see changes table below)
  • What do we do about CONDA_PREFIX? I am aware that it is an env var set by conda/mamba/Pixi when an environment is activated. It makes sense for "operate on current environment" commands.
    • It is used by shell_hook.rs, not as a prefix fallback, but as part of the activation machinery. But it already defaults to .prefix, and the activated shell related context is handled separately via ActivationVariables.
    • It is used by the list command right now
    • I don't think it makes sense for create as it's supposed to create or modify a specific prefix, and defaulting to the active prefix sounds kinda destructive 😅 Same for menu as well.
    • run: maybe yes? See 887eac6 (and cc: @danyeaw). My reasoning:
      • This is useful for, say, running rattler run python without needing -p. rattler list gracefully falls back to the prefix, so run can too.
      • Since the run command is about "Run a command in an activated conda environment", it kinda implies awareness about the CONDA_PREFIX

Changes I made

Command Change(s)
create This had Option<PathBuf> and a runtime fallback. I changed this to PathBuf and default_value = ".prefix". Removed debug println!
run Same as create. I removed an unused env import and debug println!. I also added the CONDA_PREFIX fallback like list.
shell_hook I only updated a comment. This already had default_value and std::path::absolute. We're all good here!
menu (InstallOpt and RemoveOpt) I fixed clap attrs from #[clap(long, short)] (it was deriving -t/--target-prefix) and changed it to -p/--prefix with visible_alias. Also, I changed fs::canonicalize to std::path::absolute like the rest.
list I renamed prefix to target_prefix and added visible_alias = "target-prefix" and short = 'p', again like the rest of them. I have kept the CONDA_PREFIX fallback here! See above for questions.

How Has This Been Tested?

I ran and printed the help sections of the commands locally using Pixi tasks after following the instructions in the Contributing guide. Also, I waited for CI to run and be green.

AI Disclosure

No AI-generated content was included in this PR. However, I used Claude Sonnet 4.6 to perform a self-review of my changes, besides a manual review, of course.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added sufficient tests to cover my changes.

@danyeaw danyeaw moved this from Refinement 🃏 to In Progress 🏗️ in conda Roadmap and Sprint Planning Apr 2, 2026
@danyeaw danyeaw moved this from In Progress 🏗️ to In review 🔍 in conda Roadmap and Sprint Planning Apr 2, 2026
@agriyakhetarpal agriyakhetarpal changed the title Standardise target prefix across commands refactor: standardise target prefix across commands Apr 2, 2026
@agriyakhetarpal agriyakhetarpal force-pushed the standardise-target-prefix branch from 826dc98 to 887eac6 Compare April 3, 2026 14:09
@agriyakhetarpal agriyakhetarpal marked this pull request as ready for review April 3, 2026 14:09
@agriyakhetarpal
Copy link
Copy Markdown
Author

Ready for initial review, thank you!

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

Labels

None yet

Projects

Status: In review 🔍

Development

Successfully merging this pull request may close these issues.

2 participants