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

add EXTERNALLY-MANAGED #7

Merged
merged 36 commits into from
Feb 12, 2024
Merged

add EXTERNALLY-MANAGED #7

merged 36 commits into from
Feb 12, 2024

Conversation

jaimergp
Copy link
Member

@jaimergp jaimergp commented Dec 18, 2023

Closes #1

In this PR, we are implementing the following behavior:

  • conda-pip packaged files include EXTERNALLY-MANAGED by default. This protects the base environment (or whatever conda-pip has been installed to; usually base).
  • On target environments, we will place EXTERNALLY-MANAGED after conda pip -p target install ... executions.
  • Thanks to a post-command plugin:
    • We will also inject EXTERNALLY-MANAGED if the target environment has pip.
    • In Python updates, the plugin will also migrate the file from one lib/pythonX.Y directory to the new one.
    • We will remove the marker file when pip is not present in the target environment anymore.
    • This is not done in envs created by conda-build, or if conda-pip is present in base as a dependency and not explicitly installed (we check the history file for explicit presence of conda-pip).

To do / discuss:

  • What happens when conda remove --all is invoked? This should be caught by the post-command plugin anyway, but a few empty directories might be in place? Maybe conda clears the tree? Investigate.
  • Same but for conda env remove.

References:

@jaimergp
Copy link
Member Author

I'm not too sure yet about writing the files directly to the target environment. It creates the opportunity for lingering files unless the user removes packages nicely. Maybe an alternative is to inject a fake conda-meta/externally-managed-1.0-py3xy.json metadata file that lists the EXTERNALLY-MANAGED file so it can be conda removed nicely. I'll leave that for a further issue.

@jaimergp
Copy link
Member Author

Pinging @jezdez @pradyunsg for awareness / comments if time allows :)

Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

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

Noice!

externally_managed.unlink()
place_externally_managed(target_prefix)
else: # remove
if list(prefix_data.query("pip")):
Copy link
Member

Choose a reason for hiding this comment

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

Any exception expected with this call?

Copy link
Member Author

Choose a reason for hiding this comment

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

.query() returns a generator. Worst case it's empty because there were no matches.

yield plugins.CondaPostCommand(
name="conda-pip-ensure-target-env-has-externally-managed",
action=ensure_target_env_has_externally_managed,
run_for={"install", "create", "update", "remove"},
Copy link
Member

Choose a reason for hiding this comment

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

This should also cover the various env subcommands

Copy link
Member Author

Choose a reason for hiding this comment

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

It's tricky to know how we should handle this in combination with the potential conda env hook. I'd rather think about in a different PR so the conversations are well captured separately.

@jaimergp jaimergp merged commit 4d628c6 into main Feb 12, 2024
9 checks passed
@jaimergp jaimergp deleted the externally-managed branch February 12, 2024 20:51
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.

Install EXTERNALLY-MANAGED
2 participants