fix: always add git requirement to the lockfile #5187
Conversation
ruben-arts
left a comment
ruben-arts
left a comment
There was a problem hiding this comment.
Self review, @nichmor please take a look at this.
| let git_url = GitUrl::from_fields( | ||
| display_safe, | ||
| into_uv_git_reference(pinned_git_spec.source.reference.into()), | ||
| Some(git_oid), |
There was a problem hiding this comment.
No sure about this, it seems that the .with_precise didn't do what it should.
There was a problem hiding this comment.
I think the problem was with this part into_uv_git_reference ( git reference wasn't saved ) when parsing from display_safe
| { | ||
| // Use the reference from the original requirement instead of what UV resolved | ||
| pinned_git_spec.source.reference = | ||
| into_pixi_reference(original_git.reference().clone()); |
There was a problem hiding this comment.
This pushes in the actual requirement, is this right? It feels strange that that information is not in the resolved spec.
nichmor
left a comment
nichmor
left a comment
There was a problem hiding this comment.
looks good but for some reason I can't solve anymore this pixi.toml
[workspace]
authors = ["author"]
channels = ["conda-forge"]
name = "pixi-git"
platforms = ["osx-arm64", "linux-64"]
version = "0.1.0"
[tasks]
[dependencies]
python = ">=3.14.2,<3.15"
[pypi-dependencies]
ec-tools = {git = "https://github.com/echemdb/ec-tools.git", branch = "main" }| let git_url = GitUrl::from_fields( | ||
| display_safe, | ||
| into_uv_git_reference(pinned_git_spec.source.reference.into()), | ||
| Some(git_oid), |
There was a problem hiding this comment.
I think the problem was with this part into_uv_git_reference ( git reference wasn't saved ) when parsing from display_safe
| } = &original_req.source | ||
| { | ||
| // Use the reference from the original requirement instead of what UV resolved | ||
| pinned_git_spec.source.reference = |
There was a problem hiding this comment.
Do you always want to override the reference? It may be that the reference is already set, maybe we can skip in this case, searching through the original requirements?
There was a problem hiding this comment.
I don't see how to fix this. The reference is default branch but there must be multiple code paths as it sometimes is written without and sometimes with the reference in the url.
3 participants
Description
This changes that we always write the requirement to the lockfile, and proactively fix that.
branch = "main"is specified in the requirement.Fixes #5185
How Has This Been Tested?
I've tested this locally, using the example in the issue.
rev = "master"branch = "master"tag = "v0.14.0AI Disclosure
Tools: Claude
Checklist:
schema/model.py.I struggle to find a nice way to test this except for the changes I already made to the tests.