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

docs(rfc): pixi global proposal #1757

Merged
merged 23 commits into from
Aug 12, 2024

Conversation

Hofer-Julian
Copy link
Contributor

@Hofer-Julian Hofer-Julian commented Aug 7, 2024

Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

Initial review

docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
@Hofer-Julian Hofer-Julian marked this pull request as ready for review August 7, 2024 15:30
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
@baszalmstra baszalmstra changed the title docs: pixi global proposal docs(rfc): pixi global proposal Aug 8, 2024
@baszalmstra baszalmstra added the rfc label Aug 8, 2024
Copy link
Member

@wolfv wolfv left a comment

Choose a reason for hiding this comment

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

Added my thoughts.

docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
- Only one package can expose binaries
- Manifest is simpler
- Less room for user error
- It is still possible to install multiple versions of the same package in different environments
@Hofer-Julian
Copy link
Contributor Author

As discussed, I modified the proposal to feature a more minimalistic version of pixi global.

  • Only one package per environment can expose binaries
  • Manifest is simpler
  • Less room for user error
  • It is still possible to install multiple versions of the same package in different environments

@wolfv
Copy link
Member

wolfv commented Aug 9, 2024

Only one package per environment can expose binaries

This makes certain configurations not work - for example when you have metapackages, like ansible (where the binaries are in the ansible-core dependency).

@wolfv
Copy link
Member

wolfv commented Aug 9, 2024

Just to expand a bit more on the idea of having multiple (exposed) packages in a single environment. One of the points would be to cover functionality from e.g. homebrew or similar tools.

Especially in the case of python + pip you might want to expose both from the same environemnt, so that you can use the single python version to also create new virtual environments. Same goes for python + uv etc.

I do see a use case for this.

But since the TOML format as envisioned here would be easily extended to accomodate multiple packages in that section, I think we can get started.

@Hofer-Julian
Copy link
Contributor Author

Hofer-Julian commented Aug 9, 2024

But since the TOML format as envisioned here would be easily extended to accomodate multiple packages in that section, I think we can get started.

I don't think it makes sense to go for the proposal as is, if we already expect that we need multiple top-level packages that are allowed to expose binaries. In that case, I would go back to the former state, which didn't make the distinction between injected and package

@Hofer-Julian
Copy link
Contributor Author

I've made a new commit which removes the distinction between injected and package. I also tried to address as many comments as possible on the former proposal.

@Hofer-Julian Hofer-Julian requested a review from wolfv August 9, 2024 09:19
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
docs/features/pixi_global_manifest.md Outdated Show resolved Hide resolved
Copy link
Contributor

@nichmor nichmor left a comment

Choose a reason for hiding this comment

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

awesome job! ❤️

@Hofer-Julian Hofer-Julian merged commit 3a6b694 into prefix-dev:main Aug 12, 2024
5 checks passed
@Hofer-Julian Hofer-Julian deleted the docs/pixi-global-proposal branch August 12, 2024 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants