Include version constraints in derivation chains #9112

charliermarsh · 2024-11-14T04:16:28Z

Summary

Derivation chains can now include the versions at which a package was requested.

charliermarsh · 2024-11-14T04:16:51Z

crates/uv/src/commands/diagnostics.rs

@@ -327,3 +342,88 @@ pub(crate) fn no_solution_hint(err: uv_resolver::NoSolutionError, help: String)
    let report = miette::Report::new(Error { header, err, help });
    anstream::eprint!("{report:?}");
 }
+
+/// Remove local versions sentinels (`+[max]`) from the interval.
+fn strip_sentinel(


I need to DRY this up with the error reporting in the resolver.

charliermarsh · 2024-11-14T04:17:11Z

crates/uv-distribution-types/src/derivation.rs

    /// The version of the package.
-    version: Version,
+    pub version: Version,
+    /// The constraints applied to the subsequent package in the chain.


This is somewhat confusing but it is the most normalized representation...

zanieb · 2024-11-14T05:01:09Z

crates/uv/tests/it/lock.rs

@@ -19279,7 +19279,7 @@ fn lock_derivation_chain() -> Result<()> {
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)?

-      help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref`
+      help: `wsgiref` was included because `project==0.1.0` depends on `wsgiref (==0.1.2)`


I'm not sure if the parenthesis are helpful for readability here. Is there a particular reason you went with that approach instead of wsgiref==0.1.2?

In prior entries, I want to show both the selected version and the version range -- like django==1.4.0 (>=1.0.0). So it seemed nice to be consistent with the use of parentheses here?

Maybe it should be like: django v1.4.0 (>=1.0.0)?

There are like three different cases to consider in the chain:

The first dependency (project==0.1.0, which doesn't have a set of constraints, because it's just attached to the root).

Intermediary dependencies (not visible here), which have both a pinned version and a set of constraints, so we want to show both (like, django==1.4.0 (>=1.0.0)).

The final dependency (wsgiref (==0.1.2)), which we don't show the version for just to be succinct, but could.

Interesting, thanks for explaining. I'm happy with whatever you do here. django v1.4.0 (>=1.0.0) seems easier to understand.

Regarding the root package, we might want to do something more consistent with the resolver reporter

uv/crates/uv-resolver/src/pubgrub/report.rs

Lines 341 to 355 in 744a909

fn format_root_requires(&self, package: &PubGrubPackage) -> Option<String> {

if self.is_workspace() {

if matches!(&**package, PubGrubPackageInner::Root(_)) {

if self.is_single_project_workspace() {

return Some("your project requires".to_string());

}

return Some("your workspace requires".to_string());

}

}

match &**package {

PubGrubPackageInner::Root(Some(name)) => Some(format!("{name} depends on")),

PubGrubPackageInner::Root(None) => Some("you require".to_string()),

_ => None,

}

}

I'd omit the * throughout, I don't think we usually show that to users.

I'm kind of into wsgiref was included because project v0.1.0 depends on child>=0.1.0 and child v0.1.4 depends on wsgiref

Actually the last one you gave isn't bad either.

Whatever you're more into — we can iterate later if we need to.

Okay, I went with:

help: `wsgiref` (v0.1.2) was included because `project` (v0.1.0) depends on `child` (v0.1.0) which depends on `wsgiref>=0.1.2`

charliermarsh · 2024-11-15T02:56:05Z

crates/uv-resolver/src/error.rs

+        }
+        (lower, upper)
+    }
+}


Just moved and restructured to make it public.

charliermarsh added the error messages Messaging when something goes wrong label Nov 14, 2024

charliermarsh commented Nov 14, 2024

View reviewed changes

charliermarsh force-pushed the charlie/chain-ii branch from f990682 to f6dbdaf Compare November 14, 2024 04:18

charliermarsh force-pushed the charlie/chain branch from 4561df7 to 9629f13 Compare November 14, 2024 04:37

charliermarsh force-pushed the charlie/chain-ii branch from f6dbdaf to b3bbb50 Compare November 14, 2024 04:38

charliermarsh force-pushed the charlie/chain branch 2 times, most recently from debb749 to cf667fc Compare November 14, 2024 04:50

charliermarsh force-pushed the charlie/chain-ii branch from b3bbb50 to 735635b Compare November 14, 2024 04:50

zanieb reviewed Nov 14, 2024

View reviewed changes

charliermarsh force-pushed the charlie/chain branch from cf667fc to 3e30713 Compare November 14, 2024 14:51

charliermarsh force-pushed the charlie/chain-ii branch from 735635b to 2a6e904 Compare November 14, 2024 14:51

charliermarsh force-pushed the charlie/chain branch from 3e30713 to 7bb60fa Compare November 14, 2024 20:25

Base automatically changed from charlie/chain to main November 14, 2024 20:48

charliermarsh force-pushed the charlie/chain-ii branch 2 times, most recently from 3d37271 to 6b06633 Compare November 15, 2024 02:55

charliermarsh requested a review from zanieb November 15, 2024 02:55

charliermarsh marked this pull request as ready for review November 15, 2024 02:55

charliermarsh commented Nov 15, 2024

View reviewed changes

crates/uv-resolver/src/error.rs

}

(lower, upper)

}

}

Copy link

Member Author

charliermarsh Nov 15, 2024

Choose a reason for hiding this comment

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

Just moved and restructured to make it public.

charliermarsh force-pushed the charlie/chain-ii branch 3 times, most recently from af267d0 to 87c15db Compare November 15, 2024 19:36

charliermarsh enabled auto-merge (squash) November 15, 2024 19:37

zanieb approved these changes Nov 15, 2024

View reviewed changes

Include version constraints in derivation chains

734ccdb

charliermarsh force-pushed the charlie/chain-ii branch from 87c15db to 734ccdb Compare November 15, 2024 19:54

charliermarsh disabled auto-merge November 15, 2024 20:06

charliermarsh merged commit 8dd095c into main Nov 15, 2024
63 of 64 checks passed

charliermarsh deleted the charlie/chain-ii branch November 15, 2024 20:06

BrewTestBot mentioned this pull request Nov 19, 2024

uv 0.5.3 Homebrew/homebrew-core#198292

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include version constraints in derivation chains #9112

Include version constraints in derivation chains #9112

charliermarsh commented Nov 14, 2024

charliermarsh Nov 14, 2024

charliermarsh Nov 14, 2024

zanieb Nov 14, 2024

charliermarsh Nov 15, 2024

charliermarsh Nov 15, 2024

charliermarsh Nov 15, 2024

zanieb Nov 15, 2024

zanieb Nov 15, 2024

charliermarsh Nov 15, 2024

charliermarsh Nov 15, 2024

zanieb Nov 15, 2024

charliermarsh Nov 15, 2024

charliermarsh Nov 15, 2024

	fn format_root_requires(&self, package: &PubGrubPackage) -> Option<String> {
	if self.is_workspace() {
	if matches!(&**package, PubGrubPackageInner::Root(_)) {
	if self.is_single_project_workspace() {
	return Some("your project requires".to_string());
	}
	return Some("your workspace requires".to_string());
	}
	}
	match &**package {
	PubGrubPackageInner::Root(Some(name)) => Some(format!("{name} depends on")),
	PubGrubPackageInner::Root(None) => Some("you require".to_string()),
	_ => None,
	}
	}

Include version constraints in derivation chains #9112

Include version constraints in derivation chains #9112

Conversation

charliermarsh commented Nov 14, 2024

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment