Skip to content

refactor: Split install_pypi into own pixi_install_pypi crate#4302

Closed
haecker-felix wants to merge 13 commits intoprefix-dev:mainfrom
haecker-felix:pixi_install_pypi
Closed

refactor: Split install_pypi into own pixi_install_pypi crate#4302
haecker-felix wants to merge 13 commits intoprefix-dev:mainfrom
haecker-felix:pixi_install_pypi

Conversation

@haecker-felix
Copy link
Copy Markdown
Collaborator

This PR moves the install_pypi mod into a new pixi_install_pypi crate. Long term goal is to split all the logic out of pixi so that it only contains the parts which render the command line interface.

Once that is done we can start designing a proper pixi-api crate which then gets consumed by pixi (CLI) and later pixi-gui.

Since install_pypi had some imports from other modules, we also need to move them into separate crates, since we can't depend on pixi (circular dependency).

  • moves prefix::Prefix into pixi_utils crate
  • moves python_status and pypi_prefix into new pixi_environment crate
  • moves UvResolutionContext into new pixi_lockfile crate

Next step is to split out the other modules as well. I'll do that in separate PRs, so the diffs remain "small" / reviewable.

@Hofer-Julian Hofer-Julian requested a review from tdejager August 11, 2025 08:47
@haecker-felix haecker-felix changed the title Split install_pypi into own pixi_install_pypi crate refactor: Split install_pypi into own pixi_install_pypi crate Aug 11, 2025
Copy link
Copy Markdown
Contributor

@tdejager tdejager left a comment

Choose a reason for hiding this comment

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

Hey Felix!

Good first start, however, two things that I see that kind of stopped me doing the same thing.

  1. The UvResolutionContext is actually not aptly named, as its more a SharedVariables kind of thing and handles more than just resolution. This was kind-of changed over time. Then the pixi_lockfile crate for just that structure is not correct and that would need to be changed.
  2. The pixi_environment is not the correct name for these structures either, except if we move all the enviroment stuff from the main crate in there. Which, just from this PR, I cannot see if that will happen. Otherwise pixi_pypi_prefix/env would make more sense.

So I'd either rename things first or move other (more) isolated things first.

@haecker-felix
Copy link
Copy Markdown
Collaborator Author

haecker-felix commented Aug 11, 2025

@tdejager ah, sorry, one important thing I forgot to mention is that I intent to move the rest of src/lock_file/* to the new pixi_lockfile and src/environment/* to the new pixi_environment crate as well - but in a follow-up PR as next step.

Then the pixi_lockfile crate for just that structure is not correct and that would need to be changed.

Yep, I guess it makes more sense once the rest of src/lock_file/* is there as well?

The pixi_environment is not the correct name for these structures either, except if we move all the enviroment stuff from the main crate in there.

Yep, that's the idea.

So I'd either rename things first or move other (more) isolated things first.

I fear it'll be hard to find more isolated things, as for example workspace / environment / task are pretty much intertwined - but I'll have a look 👍

@baszalmstra
Copy link
Copy Markdown
Contributor

fear it'll be hard to find more isolated things, as for example workspace / environment / task are pretty much intertwined - but I'll have a look 👍

That would provide one of the biggest benefits I think!

@tdejager
Copy link
Copy Markdown
Contributor

Yeah, I can imagine that takes some time, but it will make the rest a lot easier afterwards :)

…formUnsat, LockedCondaPackages, LockedPypiPackages, PypiRecord, PypiRecordsByName, PixiRecordsByName into pixi_lockfile crate
@lucascolley lucascolley added the refactor Specifies PR or Issue is related to a refactor label Aug 11, 2025
@haecker-felix
Copy link
Copy Markdown
Collaborator Author

I moved some more (somewhat) self-contained stuff into the pixi_environment and pixi_lockfile crates. In this PR I intentionally do not touch environment / workspace to keep it manageable.

But I'm not sure if it makes sense to continue down this path. I think I'll end up in a situation that the pixi_environment and pixi_workspace crate would depend on each other and would cause a dependency cycle.

@tdejager
Copy link
Copy Markdown
Contributor

Can you point at what part will cause a circular reference?

@tdejager
Copy link
Copy Markdown
Contributor

I'm a bit sorry to barge in here :) But I'd rather first move environment or lock, into its seperate crates, the one that's the least connected at least. Figure out what would cause circular references, then fix that. And then move into install_pypi.

@tdejager
Copy link
Copy Markdown
Contributor

@haecker-felix Oh and seperate PR's for that would be great :)

@haecker-felix
Copy link
Copy Markdown
Collaborator Author

For example, if I would start with splitting out into pixi_environment I would have trouble with

  • src/environment/mod.rs -> there is plenty stuff there which depends on workspace::Workspace etc
  • src/environment/conda_prefix.rs -> depends on grouped_environment::{GroupedEnvironment, GroupedEnvironmentName} etc

But I can't make pixi_environment depending on pixi, since pixi already would depend on pixi_environment.

I wonder if it would make more sense to have some modules in some pixi_core crate or similar.

It may also complicate stuff in future when you develop something in pixi_environment but can't use stuff from pixi_workspace, since pixi_workspace already depends on pixi_environment (just as example, the more separate crates we split into, the worse that may get).

@haecker-felix Oh and seperate PR's for that would be great :)

That's what I try to do 👍

@tdejager
Copy link
Copy Markdown
Contributor

@haecker-felix Yeah for sure I'm open to something like pixi_core. I know that uv also has a hairy circular dependency problem in the form of source builds.

They solve this in the following way: https://github.com/astral-sh/uv/blob/main/crates/uv-types/src/traits.rs#L63

Note that they may have uv_types for the same reason you are encountering.

@haecker-felix
Copy link
Copy Markdown
Collaborator Author

Replaced by #4328

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

Labels

refactor Specifies PR or Issue is related to a refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants