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

Update requirements for smmap #67

Closed
tmcclintock opened this issue Jan 26, 2021 · 14 comments · Fixed by #68
Closed

Update requirements for smmap #67

tmcclintock opened this issue Jan 26, 2021 · 14 comments · Fixed by #68

Comments

@tmcclintock
Copy link
Contributor

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
Copy link
Member

Byron commented Jan 27, 2021

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

@Harmon758
Copy link
Member

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 #61 (comment).

@mbunkus
Copy link

mbunkus commented Mar 14, 2021

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

@Byron
Copy link
Member

Byron commented Mar 15, 2021

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
Copy link

nathanhi commented Mar 24, 2021

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
Copy link

mbunkus commented Mar 24, 2021

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
Copy link

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
Copy link
Contributor Author

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
Copy link
Member

Byron commented Mar 24, 2021 via email

@tmcclintock
Copy link
Contributor Author

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

@Byron
Copy link
Member

Byron commented Mar 24, 2021

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

@Harmon758
Copy link
Member

Sorry for the delay in getting to this.

@Byron v4.0.6 (aa7228e), which resolves this, is ready to be released.

@Byron
Copy link
Member

Byron commented Mar 25, 2021

v4.0.6 was released 🙌

@mbunkus
Copy link

mbunkus commented Mar 28, 2021

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants