conda / conda-lock

Lightweight lockfile for conda environments
https://conda.github.io/conda-lock/
Other
467 stars 102 forks source link

feat: add support for git to lockfile #394

Closed mjlbach closed 1 year ago

mjlbach commented 1 year ago

Closes https://github.com/conda/conda-lock/issues/392

This is an incredibly hacky solution for https://github.com/conda/conda-lock/issues/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[bot] commented 1 year ago

Deploy Preview for conda-lock ready!

Name Link
Latest commit aa6f374147e58e7f3ed28bf7288f5f5a9d05eba0
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 commented 1 year ago

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

mjlbach commented 1 year ago

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 commented 1 year ago

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

mjlbach commented 1 year ago

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 commented 1 year ago

@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 https://github.com/conda/conda-lock/pull/310?

maresb commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

maresb commented 1 year ago

Already merged in #435