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

feat: better path handling when using --find-links especially #2417

Merged
merged 6 commits into from
Nov 14, 2024

Conversation

tdejager
Copy link
Contributor

@tdejager tdejager commented Nov 5, 2024

What does it fix

  1. This should help with locking and trying to use relative paths when possible. For example before the find-links example had absolute path in the lock file, now we should not. Also compared to before the code has been documented a lot better, and there were some assumption errors, like assuming a path would be relative.

  2. I've noticed that if a panic occurs during a resolve that this sometimes led to a silent error, very annoying. So I just removed all expect's from the resolve code and decided to use errors and miette for all of it.

How the design is approached

The assumption now is to use a relative path in the lock file if possible and otherwise fall back to an absolute path. Note that this behavior is only changed for path based indexes. We now check what kind of index the package is coming from, the latest uv updates made these distinctions a lot easier.

Why not always use relative paths?

Because (and some tests do this) you can supply absolute paths to indexes on disk or flat indexes (which are basically just a list of files) with absolute paths. However, if the project root is contained in this path we will make it relative, this way you can supply a relative path to a flat index, and it should work. Currently, we allow absolute paths, and like I said above some tests make absolute paths to extra-index-url on disk.

How to test

  • Find links examples
  • clean run of all examples
  • Maybe some manual testing with both absolute and non-absolute indexes.

Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

Great PR and nice fix!

I've tested what was listed and I wasn't able to create an unexpected result. I don't have all knowledge on this front but it looks good to me!

@ruben-arts ruben-arts enabled auto-merge (squash) November 14, 2024 15:14
@ruben-arts ruben-arts merged commit 775811f into prefix-dev:main Nov 14, 2024
43 checks passed
tdejager added a commit to tdejager/pixi that referenced this pull request Nov 15, 2024
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