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

feat: add cli modifications of the system requirements #2765

Merged

Conversation

ruben-arts
Copy link
Contributor

@ruben-arts ruben-arts commented Dec 24, 2024

Why?

We want to work towards more automation around the system requirements. Most likely, a part of that is automated serialization for the manifest. This is a preparation for that. And secondly, it's better to have CLI options for DX and especially for tutorial.

Feature

Allows users to run:

pixi project system-requirements add linux 5.13

pixi project system-requirements add glibc 2.12

pixi project system-requirements add cuda 12.6

TODO:

  • system-requirements remove: Not in this pr
  • system-requirements list
  • Documentatie

Copy link
Contributor

@Hofer-Julian Hofer-Julian left a comment

Choose a reason for hiding this comment

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

This looks like a useful addition @ruben-arts!

Apart from my comments, I would also like to see an integration test, testing the CLI

src/cli/project/system_requirements/add.rs Outdated Show resolved Hide resolved
src/cli/project/system_requirements/add.rs Outdated Show resolved Hide resolved
Comment on lines 51 to 60
let (version, family) = args
.version
.split_once(' ')
.ok_or_else(|| {
miette::miette!("Invalid version and family string. Expected format: <version> <family>, e.g. '2.17 libc'")
})?;

let version = version.parse().into_diagnostic().wrap_err(
"Invalid version string, expected format: <version> <family>, e.g. '2.17 libc'",
)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

By shifting this logic to clap, --help and completions will be much more useful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this

src/cli/project/system_requirements/mod.rs Outdated Show resolved Hide resolved
src/cli/project/system_requirements/mod.rs Outdated Show resolved Hide resolved
src/cli/project/system_requirements/mod.rs Outdated Show resolved Hide resolved
crates/pixi_manifest/src/manifests/source.rs Outdated Show resolved Hide resolved
@ruben-arts ruben-arts marked this pull request as draft January 2, 2025 14:42
@ruben-arts ruben-arts marked this pull request as ready for review January 3, 2025 12:38
@ruben-arts ruben-arts merged commit e247a08 into prefix-dev:main Jan 3, 2025
31 checks passed
tdejager pushed a commit to tdejager/pixi that referenced this pull request Jan 3, 2025
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.

3 participants