Discover workspaces without using them in resolution #3585

konstin · 2024-05-14T18:09:07Z

Add minimal support for workspace discovery, only used for determining paths in the bluejay commands.

We can now discover the workspace structure, namely that the pyproject.toml of a package belongs to a workspace pyproject.toml with members and exclusion. The globbing logic is inspired by cargo. We don't resolve workspace = true metadata declarations yet.

konstin · 2024-05-14T18:09:34Z

crates/uv-requirements/src/discovery.rs

+/// A package in a workspace.
+#[derive(Debug, Clone)]
+#[cfg_attr(test, derive(serde::Serialize))]
+#[allow(dead_code)] // TODO(konsti): Resolve workspace package declarations.


We want this information when resolving workspace dependencies, but we don't use it yet

konstin · 2024-05-14T18:10:59Z

crates/uv-requirements/src/discovery.rs

+    fn albatross_in_example() {
+        let (project, root_escaped) = workspace_test("albatross-root-workspace");
+        let filters = vec![(root_escaped.as_str(), "[ROOT]")];
+        insta::with_settings!({filters => filters}, {


I tried the json snapshots here for the first time and find them better than the regular debug snapshots

konstin · 2024-05-14T18:11:33Z

crates/uv-requirements/src/pyproject.rs

-use std::collections::HashMap;
+use std::collections::BTreeMap;


This makes the test snapshots deterministic

konstin · 2024-05-15T21:55:36Z

crates/uv-requirements/src/discovery.rs

See for a more used state of this file https://gist.github.com/konstin/f8c05311223fbd627b0491295915e865

crates/uv-requirements/src/discovery.rs

zanieb

Could you add some documentation explaining what's going on? Or at least link to the relevant "specification" we're implementing here? I'm having a hard time reviewing because there's a lot of behavior in here.

konstin · 2024-05-17T11:23:41Z

I've edited the spec from #3404 into a docstring

charliermarsh

If this is causing you trouble, you can merge and I'll do a post-push review.

konstin · 2024-05-21T17:05:04Z

Merging this now to avoid merge conflicts

I don't really understand why this only happens on windows clippy and not on linux too, but as usual, boxing the error variant fixes it. Fixup for #3585

@konstin

## Summary This is just a re-apply of #3696, which @konstin accidentally reverted in #3585. Co-authored-by: konsti <konstin@mailbox.org>

charliermarsh · 2024-05-21T20:53:36Z

@konstin - Gave this a read, looks like a good start.

charliermarsh · 2024-05-21T20:54:58Z

crates/uv-requirements/src/workspace.rs

+            .map(|workspace| (project_path.clone(), workspace.clone(), project.clone()));
+
+        if workspace.is_none() {
+            workspace = find_workspace(&project_path)?;


Should this be an or_else?

Does this mean we're in a member of a workspace?

yeah, added some docs in the other branch

konstin added the preview Experimental behavior label May 14, 2024

konstin commented May 14, 2024

View reviewed changes

konstin force-pushed the konsti/discover-workspaces-without-using-them branch 2 times, most recently from 18b6467 to ae5d2a3 Compare May 15, 2024 21:54

konstin commented May 15, 2024

View reviewed changes

konstin force-pushed the konsti/discover-workspaces-without-using-them branch from 4e61365 to 8220fff Compare May 16, 2024 13:31