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

add branch/tag support [wip] #1245

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wolfv
Copy link
Member

@wolfv wolfv commented Apr 20, 2024

Just tried this. Looks like it kinda works? - For lockfile validation I am not sure yet. Maybe we can store the requested URL somehow?

@tdejager
Copy link
Contributor

Did you try seeing if the VerbatimUrl in the resolves still contains the correct given? The problem was that in an older version 'uv' was overwriting the given url when a branch or tag was used.

I think this still seems to be the case because the lock file of the example did not change? Or maybe it was not committed.

@wolfv
Copy link
Member Author

wolfv commented Apr 20, 2024

I don't see the tag being recorded in the direct_url.json. I am not sure where we would get the VerbatimUrl back. However, since we know the original requirement, I am wondering if we can not just insert that in the lockfile as URL ... with the branch. And the resolved requirement + Git hash would be stored in the package object. So that it looks something like:

osx-arm64:
- git+https://bla.com/requests.git@87d63de8739263bbe17034fba2285c79780da7e8?branch=v2.29.0

...

And later.

- name: requests
  version: 2.29.0
  url: git+https://github.com/psf/requests.git@87d63de8739263bbe17034fba2285c79780da7e8

@wolfv
Copy link
Member Author

wolfv commented Apr 22, 2024

So the idea here is currently:

  • we back-reference the original requirement. We use the name here. That means that users will have to use the proper names on the left-hand side
  • At thsi point the URL is already python-formatted wtih an @..., we just split off the original request and insert it as the query string in the URL we use for the lockfile.

I think this could work quite nicely because we can easily ignore the query string when testing for up-to-date-ness of the environment. And when check if the requirements have changed, we could also have some special code that checks the ?request=....

@ruben-arts ruben-arts marked this pull request as draft May 3, 2024 09:58
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.

None yet

2 participants