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

Support locks path deps with path deps #6438

Closed
wants to merge 1 commit into from

Conversation

konstin
Copy link
Member

@konstin konstin commented Aug 22, 2024

Pass the location of uv.lock, the base of our relative lockfile paths, around when locking in project mode. This fixes previous incorrect paths that were using the root of the other workspace as reference for the relative path.

Fixes #6371.

@konstin konstin added the bug Something isn't working label Aug 22, 2024
Pass the location of `uv.lock`, the base of our relative lockfile pass, through when locking in project mode. This fixes previous incorrect paths that were using the root of the other workspace as reference for the relative path.

Fix #6371.
@konstin konstin force-pushed the konsti/support-relative-path-hopping branch from 2867f8c to ccfdae1 Compare August 22, 2024 14:23
@@ -336,6 +336,8 @@ pub(crate) async fn pip_install(
dev,
source_trees,
project,
// Not needed, there's no `uv.lock`.
None,
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this a custom enum so you don't need this comment everywhere? Something self-documenting, like WorkspaceRoot::None?

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise I'd probably just remove the comments.

@@ -310,6 +310,10 @@ pub(crate) async fn pip_compile(
BuildIsolation::SharedPackage(&environment, &no_build_isolation_package)
};

// Currently no effect (only used for `uv.lock`), but there's no reason not to preserve this
// information.
let main_workspace_root = output_file.and_then(|file| file.parent());
Copy link
Member

Choose a reason for hiding this comment

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

Should we just use None then?

@charliermarsh
Copy link
Member

Those Windows failures are real -- we need to consistently use the canonical or non-canonical paths.

@konstin
Copy link
Member Author

konstin commented Aug 23, 2024

Waiting for #6490 first

@konstin konstin marked this pull request as draft August 23, 2024 08:12
konstin added a commit that referenced this pull request Aug 23, 2024
konstin added a commit that referenced this pull request Aug 23, 2024
charliermarsh pushed a commit that referenced this pull request Aug 23, 2024
charliermarsh pushed a commit that referenced this pull request Aug 23, 2024
charliermarsh pushed a commit that referenced this pull request Aug 23, 2024
charliermarsh pushed a commit that referenced this pull request Aug 23, 2024
charliermarsh pushed a commit that referenced this pull request Aug 23, 2024
charliermarsh pushed a commit that referenced this pull request Aug 23, 2024
charliermarsh pushed a commit that referenced this pull request Aug 23, 2024
charliermarsh pushed a commit that referenced this pull request Aug 24, 2024
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.

uv lock collapses relative paths to editable builds in monorepo, breaks uv sync
2 participants