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: add support for git to lockfile #394

Closed
wants to merge 1 commit into from

Conversation

mjlbach
Copy link
Contributor

@mjlbach mjlbach commented Mar 25, 2023

Closes #392

This is an incredibly hacky solution for #392, could use some additional direciton on making this more robust (does anyone know a package parser/upstream efforts to get package parsers that resolve git forges?)

@netlify
Copy link

netlify bot commented Mar 25, 2023

Deploy Preview for conda-lock ready!

Name Link
🔨 Latest commit aa6f374
🔍 Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/641eba32e5294b0008161689
😎 Deploy Preview https://deploy-preview-394--conda-lock.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@maresb
Copy link
Contributor

maresb commented Mar 25, 2023

Awesome! Looks like a great start. Thanks so much! I hope to take a closer look soon.

@mjlbach
Copy link
Contributor Author

mjlbach commented Mar 25, 2023

Thanks, ended up being less code than I expected. I wish their was a more robust package for parsing the vcs + url + hash, I thought that pkg_resource being deprecated (https://setuptools.pypa.io/en/latest/pkg_resources.html) the replacement (https://packaging.pypa.io/en/stable/requirements.html#requirements) would have some better utilities for this...

@maresb
Copy link
Contributor

maresb commented Mar 25, 2023

Yes, we should migrate that. Feel free to be bold with your PR. ;)

@mjlbach
Copy link
Contributor Author

mjlbach commented Mar 25, 2023

Just leaving this here, here is how poetry core deals with the vcs problem (similar)

https://github.com/python-poetry/poetry-core/blob/e33e9d1613338f045b55cae1a7953dec7d86f311/src/poetry/core/packages/dependency.py#L414-L427

@mjlbach
Copy link
Contributor Author

mjlbach commented Mar 25, 2023

@maresb in the process of being bold, I think we can re-use some upstream poetry handling around Requirements/link parsing, although if you want to in the long run remove the vendored dependencies in favor of a custom solver that might make de-integration slightly more complicated.

Would it be possible to get a poetry bump merged soon #310?

@maresb
Copy link
Contributor

maresb commented Mar 25, 2023

Leaving the door to Poetry de-integration is really important to me. That said, I would really like to bump the vendored Poetry. Also, as opposed to importing directly from _vendored, I'd like to set up an intermediate interfaces module where we can see at-a-glance which vendored stuff we're actually using.

Doing the vendoring requires me finding a good chunk of uninterrupted time where I can see through the whole process. Over the next month or so I have a lot of major drains on my personal time, so I can't really promise anything, but I'll try to get to it soon.

@mjlbach
Copy link
Contributor Author

mjlbach commented Mar 25, 2023

Yeah 100%, totally understand the time constraints situation. I'm just excited to see this feature in conda-lock and want to do what I can (coding-wise) to make that happen :) Just lmk where I should go from here, for now this fork lets me lock chumpy from git haha.

@muyajil
Copy link
Contributor

muyajil commented Jun 6, 2023

Is there motivation to continue this? I think it would be good to support the git+ssh and git+https prefixes such that we can lock complete environments.
Is there sth that can be done to help progress this?

@maresb
Copy link
Contributor

maresb commented Jun 6, 2023

Is there motivation to continue this?

Yes, absolutely. I'm hoping to get around to revendoring Poetry very soon. But please keep in mind that we should leave open the door to removing the vendored Poetry in the future.

Is there sth that can be done to help progress this?

Testing, writing tests, code review, resolving the merge conflict, documentation, etc.

Thanks a lot for your interest!!!

@muyajil
Copy link
Contributor

muyajil commented Jun 6, 2023

Alright, I will continue working on this, as this is a PR from a personal fork I reforked and merged into mine.
I will send a PR in the next few days.

@mjlbach
Copy link
Contributor Author

mjlbach commented Jun 6, 2023

Thanks! I'm still interested in this getting merged, but was waiting on the vendored poetry update

@maresb
Copy link
Contributor

maresb commented Jul 9, 2023

Already merged in #435

@maresb maresb closed this Jul 9, 2023
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.

Github pip dependencies: InvalidRequirement: Expected end or semicolon
3 participants