gitpython-developers / gitdb

IO of git-style object databases
http://gitdb.readthedocs.org
Other
212 stars 65 forks source link

Update requirements for smmap #67

Closed tmcclintock closed 3 years ago

tmcclintock commented 3 years ago

smmap cut a release for version 4.0.0 a few hours ago, but the requirements.txt specifies smmap>=3.0.1,<4. To avoid dependency snares for downstream packages this upper bound should be removed.

Byron commented 3 years ago

Thank you for posting - I forward this to @Harmon758 who will know what is best be done here.

Harmon758 commented 3 years ago

I plan on bumping the version requirement for the next release, but the upper bound itself will not be removed, as it futureproofs against any breaking changes and issues upstream (with smmap) that could occur, e.g. see https://github.com/gitpython-developers/gitdb/issues/61#issuecomment-590374993.

mbunkus commented 3 years ago

Any update on this? Arch Linux has smmap 4.0.0, and it breaks existing gitdb users.

Byron commented 3 years ago

I am happy to help but would need some guidance by @Harmon758 just to be sure I don't cause more damage 😅. Even though there should be close to none gitdb users I would hope we can fix this sooner than later.

nathanhi commented 3 years ago

To avoid dependency snares for downstream packages this upper bound should be removed.

I'm also strongly against removing the upper range from the requirements; with semantic versioning a major update is bound to lead to API mismatches/incompatibilities and it will break the pip install gitdb workflow if one were to e.g. try out gitdb in an interactive shell. It also leads to issues when bugfix releases for old versions are made and one expects that sane ranges are in place.

We usually pin our dependencies (and sub-dependencies) with pip-compile (from the pip-tools package) for releases of tooling. In this case we had to do a bugfix release for a fairly old tool. So we modified the requirements.in file to update our own dependency (which in turn depends on GitPython 2.1.15, which depends on gitdb2>=2,<3, which depends on smmap2>=2.0.0 - as can be found here), recompile the requirements file and suddenly got smmap2 in version 3.0.1, which got a new release back in February, which breaks compatibility with the existing code. We now have to pin smmap to a sane version range on behalf of the gitdb2 dependency, which is less than ideal:

image

Just my two cents on this topic since I recently came across this issue.

mbunkus commented 3 years ago

Sure, having an upper bound is good and well, but it requires the project to be under active maintenance. The consequences can be seen in this issue, which is two months old now, without a new release addressing the problem — and by all reports the fix is really as simple as bumping the upper bound or simply removing it.

nathanhi commented 3 years ago

Yes, having rolling release paired with legacy software is troublesome indeed. In general I'd recommend using a virtualenv/venv instead of using the distribution supplied packages to avoid exactly this issue - I even do so on more stable distributions like Debian.

This also has the benefit that it is possible to run multiple tools with conflicting requirements (i.e. one package needs something in v1, while the other one needs v2) on the same machine. But yes, on the other hand that requires more maintenance effort, especially since you will also not get security patches for those packages in a venv and have to maintain them manually.

tmcclintock commented 3 years ago

FWIW I don't care either way if the upper bound is removed or increased, only that this package stop causing downstream issues. My intention was not to argue, but to point out an issue that was very fresh (only a few hours old) at the time of posting.

Byron commented 3 years ago

Maybe somebody could submit a PR which is right for the python/pip ecosystem and I can make a release happen.

On Mar 24, 2021, at 11:32 PM, Tom McClintock @.***> wrote:

 FWIW I don't care either way if the upper bound is removed or increased, only that this package stop causing downstream issues. My intention was not to argue, but to point out an issue that was very fresh (only a few hours old) at the time of posting.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

tmcclintock commented 3 years ago

@Byron I bumped the upper bound in #68. Let me know if there is a changelog somewhere I should add a note to.

Byron commented 3 years ago

Thanks a bunch! Changes.rst should be the place if I remember correctly.

Harmon758 commented 3 years ago

Sorry for the delay in getting to this.

@Byron v4.0.6 (https://github.com/gitpython-developers/gitdb/commit/aa7228e8dbdc2ee6b6bc385e8bee21245a10e98d), which resolves this, is ready to be released.

Byron commented 3 years ago

v4.0.6 was released 🙌

mbunkus commented 3 years ago

Thank you very much!