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

Conda Shell Plugin Hook and Plugin Architecture #58

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kalawac
Copy link

@kalawac kalawac commented Jun 30, 2023

This CEP proposes a potential template for a plugin hook and associated plugin architecture that can carry out the process of activating and deactivating conda environments. Extracting this logic to plugins will enable the conda community to support activation and deactivation processes for a wider variety of shells. In addition, the use of plugins for activation will reduce the present burden on conda's maintainers in managing requests for expansion of conda's environment activation and deactivation logic to additional shells.

Please check what is currently on the CEP and use this pull request as a forum to discuss changes or points of opposition to the proposal.

Copy link
Contributor

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

Thanks for such a nice and thorough writeup! I learnt a lot while reading this. I am generally in support of the idea, but I have some questions regarding its implementation and deployment plans:

  • Deprecation plans. If I understood correctly, this method will add a subcommand conda shell that will allow users to activate environments in a subshell. This does not replace (for now) conda (de)activate. One of the points of this CEP is to reduce maintenance burden, but so far it will only add one more layer to the codebase that needs to be tested. What are the plans to replace all the previous shell logic layers (there are a few generations!) with this one?
  • In-process use cases. Assuming at some point this will be the only shell layer in active use, how is this going to achieve feature parity with the current one? You have mentioned problems with $PS1 and non-exported env vars. I didn't see anything about the valid use case of requiring in-process activation for e.g. scripts. We would need at least one way to write shell code to terminal so it can be eval'd.
  • Pluggy overhead. This new plugin system forces the shell logic to "wait" for pluggy to be ready, which might mean waiting for all plugins to be ready before we can run activation. This might be significantly slower in installations with a lot of plugins or at least a couple heavy ones. Is there a chance we can skip that runtime burden for the shell hook? The current design is a bit clunky but in part it was designed like that to avoid going through unnecessary logic layers and imports. I'd be interested in tentative benchmarks.
  • How does this compare to conda-workon? I don't think it's maintained but I think it should be in the reference list, as part of the prior art.
  • I get how conda shell activate will take you to a sub-shell because the Python process will be replaced via os.execve. However, how will conda shell deactivate force an exit? Into one more shell? Wouldn't every new conda shell leave the user in a new sub-shell? At some point the user must issue a Ctrl+D, right?

Thank you!

Please check what is currently on the CEP and use this pull request as a forum to discuss changes or points of opposition to the proposal.

## Motivation
Conda must interface with the shell to carry out environment activation and deactivation. Conda currently supports six types of shell interfaces by default, which places a burden on maintainers to be familiar with all six shell types (plus related shells with similar syntax). In addition, conda has historically received requests to expand support to other shells.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we link to some of these issues ("historically received requests") for future reference? e.g.:

### Shell Plugin Hook
The proposed shell plugin hook will facilitate the process of defining the shell-specific syntax required for conda environment activation. Plugins will declare the required shell-specific syntax as field values in the hook's named tuple. Plugins should only yield the hook's named tuple if the plugin is compatible with the shell currently in use. A new method in `CondaPluginManager` raises an error if no plugin hooks are yielded or if multiple hooks have been yielded.

The hook's fields are then used by [`PluginActivator`](https://github.com/kalawac/conda/blob/sp-cep/conda/plugins/shells/shell_plugins.py), the plugin-specific version of the `_Activator` class, to create class properties. This allows the hook to carry out the syntax-definition function previously carried out by `_Activator`'s child classes. As a result, plugin authors are not required to write a child class or rely on class inheritance.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why PluginActivator instead of ActivatorPlugin?

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.

2 participants