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

Track source incompatibilities in prioritized distribution #2066

Closed
wants to merge 1 commit into from

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Feb 29, 2024

Summary

Like with wheels, we can track the compatibility of source distributions in PrioritizedDist and use those to both backtrack and surface better error messages to users.

Closes #2003.

@charliermarsh charliermarsh requested a review from zanieb February 29, 2024 00:39
@charliermarsh charliermarsh added the bug Something isn't working label Feb 29, 2024
@charliermarsh charliermarsh marked this pull request as ready for review February 29, 2024 00:39

#[derive(Debug, PartialEq, Eq, Ord, PartialOrd, Clone, Copy)]
pub enum IncompatibleSource {
NoBuild,
Copy link
Member Author

Choose a reason for hiding this comment

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

Interestingly, if we track RequiresPython here, error messages regress quite a bit, because we no longer track the Python incompatibility in the resolver:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot: compile_python_37
Source: crates/uv/tests/pip_compile.rs:656
────────────────────────────────────────────────────────────────────────────────
Expression: snapshot
────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬───────────────────────────────────────────────────────────────────
    2     2 │ ----- stdout -----
    3     3 │
    4     4 │ ----- stderr -----
    5     5 │   × No solution found when resolving dependencies:
    6       │-  ╰─▶ Because the requested Python version (3.7) does not satisfy Python>=3.8
    7       │-      and black==23.10.1 depends on Python>=3.8, we can conclude that
    8       │-      black==23.10.1 cannot be used.
    9       │-      And because you require black==23.10.1, we can conclude that the
   10       │-      requirements are unsatisfiable.
          6 │+  ╰─▶ Because black==23.10.1 is unusable because no source distributions
          7 │+      are available that meet your required Python version and you require
          8 │+      black==23.10.1, we can conclude that the requirements are unsatisfiable.

Here's the diff, for posterity:

diff --git a/crates/distribution-types/src/prioritized_distribution.rs b/crates/distribution-types/src/prioritized_distribution.rs
index 148e82cd..13a516a8 100644
--- a/crates/distribution-types/src/prioritized_distribution.rs
+++ b/crates/distribution-types/src/prioritized_distribution.rs
@@ -45,6 +45,7 @@ pub enum SourceCompatibility {
 #[derive(Debug, PartialEq, Eq, Ord, PartialOrd, Clone, Copy)]
 pub enum IncompatibleSource {
     NoBuild,
+    RequiresPython,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs
index 7bc5c710..e7e02fc1 100644
--- a/crates/uv-resolver/src/resolver/mod.rs
+++ b/crates/uv-resolver/src/resolver/mod.rs
@@ -18,10 +18,7 @@ use tracing::{debug, info_span, instrument, trace, warn, Instrument};
 use url::Url;
 
 use distribution_filename::WheelFilename;
-use distribution_types::{
-    BuiltDist, Dist, DistributionMetadata, Incompatible, IncompatibleSource, IncompatibleWheel,
-    Name, RemoteSource, SourceDist, VersionOrUrl,
-};
+use distribution_types::{BuiltDist, Dist, DistributionMetadata, Incompatible, IncompatibleSource, IncompatibleWheel, Name, RemoteSource, SourceDist, VersionOrUrl};
 use pep440_rs::{Version, VersionSpecifiers, MIN_VERSION};
 use pep508_rs::{MarkerEnvironment, Requirement};
 use platform_tags::{IncompatibleTag, Tags};
@@ -393,6 +390,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
                                             IncompatibleTag::Platform => "no wheels are available with a matching platform".to_string(),
                                         }
                                     }
+                                    Incompatible::Source(IncompatibleSource::RequiresPython) => "no source distributions are available that meet your required Python version".to_string(),
                                     Incompatible::Source(IncompatibleSource::NoBuild) => "building from source is disabled".to_string(),
                                 }
                             } else {
@@ -663,7 +661,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
                         // If the version is incompatible because no distributions match, exit early.
                         return Ok(Some(ResolverVersion::Unavailable(
                             candidate.version().clone(),
-                            UnavailableVersion::NoDistributions(*incompatibility),
+                            UnavailableVersion::NoDistributions(incompatibility.clone()),
                         )));
                     }
                 };
diff --git a/crates/uv-resolver/src/version_map.rs b/crates/uv-resolver/src/version_map.rs
index e8a1cc02..b742daaf 100644
--- a/crates/uv-resolver/src/version_map.rs
+++ b/crates/uv-resolver/src/version_map.rs
@@ -391,6 +391,8 @@ impl VersionMapLazy {
                     DistFilename::SourceDistFilename(filename) => {
                         let compatibility = if self.no_build {
                             SourceCompatibility::Incompatible(IncompatibleSource::NoBuild)
+                        } else if file.requires_python.as_ref().is_some_and(|requires_python| !requires_python.contains(self.python_requirement.target())) {
+                            SourceCompatibility::Incompatible(IncompatibleSource::RequiresPython)
                         } else {
                             SourceCompatibility::Compatible
                         };
@@ -400,13 +402,7 @@ impl VersionMapLazy {
                             file,
                             self.index.clone(),
                         );
-                        priority_dist.insert_source(
-                            dist,
-                            requires_python,
-                            yanked,
-                            Some(hash),
-                            compatibility,
-                        );
+                        priority_dist.insert_source(dist, requires_python, yanked, Some(hash), compatibility);
                     }
                 }
             }

Copy link
Member Author

Choose a reason for hiding this comment

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

This actually suggests to me that we shouldn't be tracking RequiresPython for wheels like we are in this file, but it's a little complicated... I've played with a few refactors.

@charliermarsh
Copy link
Member Author

I think this should actually be closed in favor of #1298 which solves at least this problem and a few others. Gonna leave it in draft for now though until Zanie is back.

@charliermarsh charliermarsh marked this pull request as draft February 29, 2024 02:26
zanieb added a commit that referenced this pull request Mar 7, 2024
zanieb added a commit that referenced this pull request Mar 7, 2024
Supersedes #2066

# Conflicts:
#	crates/uv-resolver/src/candidate_selector.rs
zanieb added a commit that referenced this pull request Mar 7, 2024
Supersedes #2066

# Conflicts:
#	crates/uv-resolver/src/candidate_selector.rs
@charliermarsh
Copy link
Member Author

Closed in favor of #1298.

@charliermarsh charliermarsh deleted the charlie/source branch March 8, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Requirement with --only-binary apache-beam fails to resolve even though solution exists
1 participant