Skip to content

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

Merged
ruben-arts merged 6 commits intoprefix-dev:mainfrom
tdejager:fix/find-links
Nov 14, 2024
Merged

feat: better path handling when using --find-links especially#2417
ruben-arts merged 6 commits intoprefix-dev:mainfrom
tdejager:fix/find-links

Conversation

@tdejager
Copy link
Copy Markdown
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
Copy Markdown
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
tdejager added a commit to tdejager/pixi that referenced this pull request Nov 15, 2024
…ix-dev#2417)

Co-authored-by: Ruben Arts <ruben.arts@hotmail.com>
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