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

Lock and resolve with project name where available #7488

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Sep 18, 2024

This populates the project package name (if available) for lock-and-resolve operations.

This populates the project package name (if available) for
lock-and-resolve operations.
.pyproject_toml()
.project
.as_ref()
.map(|proj| proj.name.clone()),
Copy link
Contributor Author

@lucab lucab Sep 18, 2024

Choose a reason for hiding this comment

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

Note: drive-by fix, I noticed this detail was missing when looking into a derivation tree result from PubGrub. I think in practice this should be a no-op change for any observable behavior, but I'll let CI check that.

@lucab lucab marked this pull request as ready for review September 18, 2024 12:26
@zanieb
Copy link
Member

zanieb commented Sep 18, 2024

I believe the PubGrub "root" package is intentionally None when you're working in a project. Instead, the root has a dependency on all of the workspace members. This modeling is important for the resolver in some ways I don't fully understand, but we workaround this in the error reports:

if self.is_workspace() {
if matches!(&**package, PubGrubPackageInner::Root(_)) {
if self.is_single_project_workspace() {
return Some("your projects's requirements".to_string());
}
return Some("your workspace's requirements".to_string());
}
}

/// Given a [`DerivationTree`], drop dependency incompatibilities from the root
/// to the project.
///
/// Intended to effectively change the root to a workspace member in single project
/// workspaces, avoiding a level of indirection like "And because your project
/// requires your project, we can conclude that your projects's requirements are
/// unsatisfiable."
fn drop_root_dependency_on_project(

I think adding the name here would be confusing, since we'd have Root("my-project") -> Package("my-project") in the tree?

@lucab lucab marked this pull request as draft September 18, 2024 17:21
@lucab
Copy link
Contributor Author

lucab commented Sep 19, 2024

What you say makes plenty of sense in the context of workspaces with packages, and I'm inclined to drop this PR as it may have some side-effect implications that I currently can't fully evaluate.

At the same time, it is notable that this is the only invocation of pip::operations::resolve() explicitly without a project.
All the other 5 occurrences do forward a project name, and in virtually all the cases it seems that the source of that value can be traced back to this:

// Use the first project name discovered.
if spec.project.is_none() {
spec.project = source.project;
}

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.

2 participants