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 warning when dependencies are empty with Poetry metadata #1650

Merged
merged 6 commits into from
Feb 20, 2024

Conversation

yasufumy
Copy link
Contributor

@yasufumy yasufumy commented Feb 18, 2024

Resolve #1630

PyProjectToml doesn't seem to have a tool field, so instead of checking it, I check if requirements is empty.

/// A pyproject.toml as specified in PEP 517
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
#[serde(rename_all = "kebab-case")]
pub struct PyProjectToml {
/// Build-related data
pub build_system: Option<BuildSystem>,
/// Project metadata
pub project: Option<Project>,
}

Comment on lines 214 to 221
if pyproject_toml.build_system.is_some_and(|build_system| {
build_system
.requires
.iter()
.any(|v| v.name.as_dist_info_name().starts_with("poetry"))
}) {
warn_user!("uv does not currently support the Poetry-style pyproject.toml (Your requirements.txt would be empty)");
}
Copy link
Member

Choose a reason for hiding this comment

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

Could we move this down and make it conditional on requirements being empty? It's possible that they've added a dependency section we can read even if they're using Poetry.

We have a similar warning for requirements.txt files

if data == Self::default() {
warn_user!(
"Requirements file {} does not contain any dependencies",
requirements_txt.as_ref().display()
);
}

Maybe we can phrase it the same with a hint that we can't read Poetry's dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

(Is it possible to make this conditional on whether tool.poetry.dependencies is populated? I assume not, since we don't include that in TOML schema. If so, this is totally fine.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zanieb Thank you for your suggestion. I updated my PR and also fixed a test case. Let me know if I miss anything!

@charliermarsh Thank you for your comment. Are you planning to support the poetry-style format? If so, and no one is working on it, I'm interested in contributing.

@zanieb zanieb added error messages Messaging when something goes wrong tracing Verbose output and debugging and removed tracing Verbose output and debugging labels Feb 19, 2024
@yasufumy yasufumy changed the title Add warning when poetry dependencies found. Add warning when dependencies are empty. Feb 19, 2024
@yasufumy yasufumy changed the title Add warning when dependencies are empty. Add warning when dependencies are empty Feb 19, 2024
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Thanks!

@charliermarsh charliermarsh enabled auto-merge (squash) February 19, 2024 23:52
@charliermarsh charliermarsh changed the title Add warning when dependencies are empty Add warning when dependencies are empty with Poetry metadata Feb 19, 2024
@charliermarsh charliermarsh merged commit 8f739c9 into astral-sh:main Feb 20, 2024
7 checks passed
.iter()
.any(|v| v.name.as_dist_info_name().starts_with("poetry"))
}) {
warn_user!("`{}` does not contain any dependencies (hint: Poetry's format is not supported for now)", path.normalized_display());
Copy link
Contributor

@hauntsaninja hauntsaninja Feb 20, 2024

Choose a reason for hiding this comment

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

Maybe this should be more specific, e.g. hint: specify dependencies in project.dependencies section, tool.poetry.dependencies is not currently supported. I can imagine users taking "Poetry's format" to mean all kinds of things, like literally the use of pyproject.toml

Copy link
Member

Choose a reason for hiding this comment

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

Nice idea: #1730

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error messages Messaging when something goes wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add warning when poetry dependencies found instead of PEP 621 dependencies
4 participants