Skip to content

Allow to deactivate environments activated from pixi shell-hook #3589

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

Open
Hofer-Julian opened this issue Apr 14, 2025 · 7 comments · May be fixed by #3670
Open

Allow to deactivate environments activated from pixi shell-hook #3589

Hofer-Julian opened this issue Apr 14, 2025 · 7 comments · May be fixed by #3670
Labels
enhancement Feature request

Comments

@Hofer-Julian
Copy link
Contributor

Currently, there's no way to deactivate an environment activated by pixi shell-hook. Let's implement that!

Also see discussion in #3525

@Hofer-Julian Hofer-Julian added the enhancement Feature request label Apr 14, 2025
@mrswastik-robot
Copy link
Contributor

Hi @Hofer-Julian! I would love to contribute to this. Should I start by writing an async function like generate_deactivation_script in shell_hook.rs? I have only solved some good first issues in pixi's codebase till now, so would love to hear more on how a beginner can contribute to this

@Hofer-Julian
Copy link
Contributor Author

Will there be an offical and clean implementation of pixi activate ${path} and pixi deactivate?

Please open a separate issue for that @YuanfengZhang

@Hofer-Julian
Copy link
Contributor Author

I would love to contribute to this. Should I start by writing an async function like generate_deactivation_script in shell_hook.rs? I have only solved some good first issues in pixi's codebase till now, so would love to hear more on how a beginner can contribute to this

@mrswastik-robot sounds good to me. Happy to look at a draft PR as soon as you have something :)

@mrswastik-robot
Copy link
Contributor

mrswastik-robot commented Apr 22, 2025

Hi @Hofer-Julian, so I was trying to implement this, but I was having trouble finding the code @wolfv mentioned here in rattler_shell crate. There's definitely code for built-in deactivation() method on the activator object, but it doesn't appear to be a standalone deactivation() method.

For ex. I wrote this function but there isn't any deactivation() method already available:

async fn generate_deactivation_script(
    shell: Option<ShellEnum>,
    environment: &Environment<'_>,
    project: &Workspace,
) -> miette::Result<String> {
 
    let shell = shell.unwrap_or_else(|| {
        ShellEnum::from_parent_process()
            .unwrap_or_else(|| ShellEnum::from_env().unwrap_or_default())
    });

    let activator = get_activator(environment, shell.clone()).into_diagnostic()?;

    let result = activator.deactivation().into_diagnostic()?;

    let script = result.script.contents().into_diagnostic()?;

    Ok(script.to_string())
}

@wolfv
Copy link
Member

wolfv commented Apr 23, 2025

Hey @mrswastik-robot - you are correct. There is some code missing to make it easy to deactivate environments that we need to add to rattler. However, deactivation in general is already implemented (e.g. finding all the deactivation scripts and removing env vars).

Would you be able to write the deactivation() function in rattler?

@mrswastik-robot
Copy link
Contributor

Hey @mrswastik-robot - you are correct. There is some code missing to make it easy to deactivate environments that we need to add to rattler. However, deactivation in general is already implemented (e.g. finding all the deactivation scripts and removing env vars).

Would you be able to write the deactivation() function in rattler?

sure @wolfv, I will give this a try and raise a PR in the rattler repo and link to this issue

@mrswastik-robot
Copy link
Contributor

Hi @wolfv, so I have created the PRs, after making changes in the rattler_shell crate, when I tried to use the local fork of the rattler_shell instead of the installed crate in the pixi project by providing absolute path in the Cargo.toml , I was unable to compile and run tests, mainly due to some dependency version mismatches with the rattler_conda_types crate. So, idk if the solution in the PRs will work for sure, would love to hear from your side and happy to implement further requested changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants