Skip to content

Conversation

LED-0102
Copy link

@LED-0102 LED-0102 commented Oct 3, 2025

Description

This is a draft PR for discussion of #1696.

For now, run_activation simply returns variables.current_env plus variables which can be determined seeing the activation() function when there is no conda prefix and no activation scripts, skipping the shell.

Opening this PR to discuss whether this approach is acceptable and how we might handle deterministic environment diffs in these cases.

@LED-0102 LED-0102 marked this pull request as draft October 3, 2025 06:19
@LED-0102 LED-0102 changed the title draft: perf: skip shell if no activation scripts or conda prefix (#1696) feat: perf: skip shell if no activation scripts or conda prefix (#1696) Oct 3, 2025
@LED-0102 LED-0102 marked this pull request as ready for review October 7, 2025 06:17
@wolfv
Copy link
Contributor

wolfv commented Oct 13, 2025

I think the idea is solid but I would like to double check whether this takes into account the environment variables from env_dir.d which are stored in JSON files?

@wolfv
Copy link
Contributor

wolfv commented Oct 13, 2025

Also I would put this into a new function and make sure it's well tested etc.

@iamthebot
Copy link
Contributor

Hi @LED-0102 do you have time to wrap up this PR? If not, I can probably take it over and address @wolfv's comments. We'd definitely benefit from this.

@LED-0102
Copy link
Author

Hi @iamthebot,

I'll look into it today if that's okay

@LED-0102
Copy link
Author

Hey all,

I’ve implemented the fast path in run_activation so that if there’s no conda_prefix and no activation scripts, we skip spawning a shell entirely. The environment variables are still computed deterministically using env_vars and post_activation_env_vars, so everything from env_dirs.d and conda-meta/state is included.

The snapshot for the fast path currently looks like this (from the new test case test_run_activation_fast_path):

CONDA_SHLVL: "1"
PKG1: "Hello, world!"
PKG2: "Hello, world!"
STATE: "Hello, world!"

I’d love some eyes on this to confirm:

  1. The snapshot looks correct
  2. The testing approach makes sense
  3. The functionality matches what we want for simple activations

After this review, I can clean up the block into a separate function, add comments, etc.

Thanks!

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