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

make gitpython dependency optional #297

Merged
merged 5 commits into from
Dec 15, 2022

Conversation

bollwyvl
Copy link
Contributor

References

Changes

  • move gitpython from requirements.txt to requirements-dev.txt
  • return None when creating GitMeta if gitpython is not installed

@netlify
Copy link

netlify bot commented Dec 14, 2022

Deploy Preview for conda-lock ready!

Name Link
🔨 Latest commit 43b4947
🔍 Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/639b84cc4a5a2b000a20894d
😎 Deploy Preview https://deploy-preview-297--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.

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Dec 14, 2022 via email

@maresb
Copy link
Contributor

maresb commented Dec 15, 2022

so maybe the fields need to be lazy

That's what I thought at first, but I believe bollwyvl#1 suffices.

@maresb
Copy link
Contributor

maresb commented Dec 15, 2022

Argh, sorry, not sure what was wrong with me last night... 😵

Could you please delete the redundant get_metadata = ... block immediately under what I added in the PR?

@bollwyvl
Copy link
Contributor Author

Like this?

9e1e311

@maresb
Copy link
Contributor

maresb commented Dec 15, 2022

Perfect, thanks!!!

@maresb
Copy link
Contributor

maresb commented Dec 15, 2022

What are your thoughts regarding the future of the GitPython dependency? Namely, suppose someone fixes the vulnerability. Then are we okay to reinclude it as a required dependency? It seems to be fairly lightweight and pure-Python.

The reason I ask is that if we can reinclude it after some time, then perhaps we could avoid introducing git_support. (It will soon become yet another thing we need to deprecate.)

@bollwyvl
Copy link
Contributor Author

Since you asked: I'd prefer conda-lock had zero dependencies, which is of course unreasonable. I use it as the "bootstrap" layer for a lot of things, and every extra dep is another thing to track that could inject weirdness, get a CVE, etc.

Specifically for git: if using conda-lock, I check in the locks, so having git metadata in them is... not very useful from an entropy point of view.

As for the metadata stuff in general: Any time i can avoid invoking any conda/mamba stuff, I do. Basically 100% of the time, I strip out the conda-lock integrity hash, because it's basically impossible to reconstruct what it would be, and doesn't particularly help git diffs (ha!). If anything, I'll inject the sorted set of depdendencies as a comment, and then run/not-run conda-lock if that has changed.

As to the other optional: I don't use the pip stuff at all (pypi/packaging/poetry/etc. has been so flaky of late), or the new yaml format. It's just another thing that reduces the portability vs. systems that already have conda or whatever.

So sure, it's fine if a non-vulnerable gitpython, comes back, and pip extras are of course hot trash, but I'd really rather only pay for what I need.

@maresb
Copy link
Contributor

maresb commented Dec 15, 2022

Thanks a lot for typing that all out, I really appreciate the perspective.

I'd prefer conda-lock had zero dependencies, which is of course unreasonable.

👍 My motivation for vendoring Poetry was to remove a dependency, and hopefully eventually eliminate it entirely.

pypi/packaging/poetry/etc. has been so flaky of late

👍 Though I don't have many complaints about Hatchling so far.

or the new yaml format

Any way to win you over on this? It definitely requires some more work for stability, but Micromamba support is to me a killer feature.

Improving stability and reducing dependencies are high on my todos here, but those unfortunately require me to somehow find some weekend days... 😆

@maresb
Copy link
Contributor

maresb commented Dec 15, 2022

For this PR, would you be amenable to dropping git_support?

@bollwyvl
Copy link
Contributor Author

Any way to win you over on this?

Probably not? My use cases are highly automation/forensics driven, and minimally entropic, reusable files on disk, checked into git are pretty much my happy place. Parsing YAYAML (Yet Another YAML) format without a JSON schema, vs a list of URLs is pretty much no contest for me.

Micromamba support is to me a killer feature

But micromamba, along with every version of conda back to the oldest running CI machine, also supports --kind=explicit. As do other tools, such as jake (sorta).

find some weekend days

don't i know it, friend! But conda-lock is a critical daily driver for me, so I'm happy to help when I can.

@bollwyvl
Copy link
Contributor Author

Hatchling

I'm happy it's working for you!

And you didn't ask, but: to me, the cambrian explosion of build systems vs the cartesian product of duplicative plugins is extremely tiresome. If important information doesn't appear in pyproject.toml in the standards compliant fields, I feel like the point has partially been missed. The failure modes of python -m build (or whatever is the non-arbitrary-code-running way to invoke it), with the layers of tracebacks, are getting to nodejs-levels of ick. Oh, and then there's the plugins that install node, or other language runtimes, inside some magical hidden place. That's the best.

@maresb
Copy link
Contributor

maresb commented Dec 15, 2022

ping @mariusvniekerk

@mariusvniekerk mariusvniekerk merged commit dd529b4 into conda:main Dec 15, 2022
@bollwyvl bollwyvl mentioned this pull request Jan 17, 2023
3 tasks
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.

Make gitpython dependency optional?
3 participants