gitpython-developers / gitdb

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

Bump smmap dependency major-version upper bound #95

Closed EliahKagan closed 10 months ago

EliahKagan commented 10 months ago

This does not change the lower bound (nor does it prohibit any versions in the range), so all the combinations of versions that could be resolved before can still be resolved. As such, this is not a major or breaking change to gitdb.

This is to permit gitdb to use smmap 6.*.* going forward (https://github.com/gitpython-developers/smmap/issues/51), including future releases that may change more than metadata. Because it does not keep any combinations of versions that worked before from continuing to work, this should not break anything.

Still, whether or not we actually want this, and also when, may depend to some extent on what is done in https://github.com/gitpython-developers/smmap/pull/52 and more broadly to matters discussed in https://github.com/gitpython-developers/smmap/issues/51. However, I suspect that a change like this will be wanted however that goes, unless the outcome is to yank smmap 6.0.0 with no further action.

This PR is conceptually related to https://github.com/gitpython-developers/smmap/pull/52 but I regard them to really be independent. Any combination of either of these PRs and making or not making a release after them could be done in any order and I believe everything would be fine (though obviously I do think these PRs should be regarded as improvements, though admittedly small ones, since if I didn't think that then I wouldn't have opened them). Since unlike smmap 6.0.0 the version bump in gitdb in a release after this PR would just be to a minor or patch version, the GitPython dependencies don't need to be updated.

EliahKagan commented 10 months ago

Closing based on https://github.com/gitpython-developers/smmap/issues/51#issuecomment-1722457502. The one and only 6-series smmap has been yanked, and the latest non-yanked version is intentionally 5.0.1. Since no version too high for the current <6 currently exists that should ever be used, the upper bound should not be changed at this time.

Byron commented 10 months ago

Thanks a lot, this all would make perfect sense if smmap would have been released as 6.0.1, but instead 6.0.0 was yanked and the more open specification was released as 5.0.1 instead. Now that I think about it, maybe that is a breaking change after all as that might have removed python 3.5 support (or was it 3.6?).

It's probably worth it for you to double-check the versions to be sure the currently released smmap is truly non-breaking in terms of python version constraints. If it is, maybe it would be easiest to release a fix adds back yet another EOL python version. I may add that I don't know what the consensus is in the python world - is breaking version compatibility a breaking change, or is it merely a fix and thus a patch release?

You advice is definitely appreciated here.

PS: This conversation is a good indicator for why I'd love to merge gitdb into GitPython and only have to deal with one python package in the future. This is particularly true as nobody actually uses smmap (nobody should be using gitdb as object database).

EliahKagan commented 10 months ago

I closed this just before your comment https://github.com/gitpython-developers/gitdb/pull/95#issuecomment-1722458491 came in, but of course it could be reopened if it turns out to be useful in some form.

Now that I think about it, maybe that is a breaking change after all as that might have removed python 3.5 support (or was it 3.6?).

It's probably worth it for you to double-check the versions to be sure the currently released smmap is truly non-breaking in terms of python version constraints. If it is, maybe it would be easiest to release a fix adds back yet another EOL python version.

I'll look into that.

I may add that I don't know what the consensus is in the python world - is breaking version compatibility a breaking change, or is it merely a fix and thus a patch release?

Although I'm realizing I'm without citations to confirm this (at least at the moment), in general I think breaking version compatibility in a way that cannot be fixed merely by upgrading other dependencies--and without changing the code that consumes those dependencies--is a breaking change. In a sense, the Python implementation is itself a dependency (it literally is with conda, and it's listed as though it were one in a poetry-managed pyproject.toml even though it's treated specially and not downloaded or installed by the package manager). However, even if we take that to be the case, upgrading it is not something that can generally be done without changing code, because most if not all Python version updates other than between patch versions remove something or change the behavior of something in an incompatible way.

(I think there are some ambiguities with this. For example, suppose operating system A is used by interpreter B that runs the code C. When B came out, it supported A, but B has a later end of life date than A, much later. Now A is itself no longer supported. B never stopped declaring support for A, and B can still be installed on A, though no one expects that, or anything else, to go well on A today. If C releases a patch version that breaks on A, is that a bug in C? If so, suppose the patch simply doesn't do anything different on A, even as it fixes some bug otherwise. Can C no longer be claimed to have fixed the bug? Can it no longer be claimed to support B?)

Byron commented 10 months ago

Thanks for sharing, I take it that the python interpreter is considered a dependency, and removing support for running on one is thus considered a breaking change. From what I can tell (without having caught up with anything else yet), the current state of GitPython is OK.

EliahKagan commented 10 months ago

Thanks for sharing, I take it that the python interpreter is considered a dependency, and removing support for running on one is thus considered a breaking change.

I had meant to suggest that if we regard the Python implementation (interpreter, standard library, etc.) to be a dependency then that would support the idea that it is more often okay for a library to break compatibility with it... but only in cases where a later version of it is readily available and doesn't require the code using the library to be changed. Since going from, e.g., Python 3.11 to 3.12 almost always incurs some breaking change (removing a deprecated standard library function, for example), and there is effectively no limit to what code that uses a library might also be doing, I think dropping support for a major or minor version of Python is a breaking change, provided support for it was in some way claimed. In contrast, I'm inclined to think it's at least in principle non-breaking to make changes that only require a patch version upgrade to the Python implementation, since a patch-version upgrade to Python shouldn't itself break anything else. But anyway, those were mostly idle thoughts, and not necessarily all that useful.

From what I can tell (without having caught up with anything else yet), the current state of GitPython is OK.

GitPython should definitely be fine, because the issue here is declared installability, and pip dependency resolution doesn't rely on packages using SemVer as their versioning convention. So whether or not dropping Python 3.6 support going from smmap 5.0.0 to 5.0.1 ought to be considered a breaking change in a patch version, and therefore as something that goes against SemVer expectations, pip on old versions of Python will still be able to choose 5.0.0 over 5.0.1 when looking for a version of smmap that can be installed.

By the way, what I am thinking of looking into but haven't yet (and alluded to opaquely in https://github.com/gitpython-developers/gitdb/pull/95#issuecomment-1722461293) is whether or not smmap has adhered closely enough to SemVer throughout its history, even in low-stakes cases like this, for confusion to be likely. Although I think SemVer is a good idea whenever there is no reason to use some other convention, if this kind of Python lower bound change in smmap has happened a number of times before without any evidence of anyone being alarmed by it then I wouldn't consider it a priority to "fix" it by releasing a 5.0.2 with declared 3.6 support.

Unlike 3.7, I think it's much harder to test 3.6 these days and be confident that the tests are representative of real-world usage on 3.6. Anyone still using 3.6 is doing something rather odd, whereas plenty of people still use 3.7 in order to maintain libraries that haven't dropped support for it yet to make things easier for people who are struggling to get their applications off 3.7. There is also the issue that 3.6 has been without security updates longer enough than 3.7 that I would be more reluctant to run it on CI.

Byron commented 10 months ago

Thanks for elaborating!

I wholly rely on your experience with python to decide which python versions to support and test for, EOL or not, and whether or not to treat the removal of declared support for installing on a particular python interpreter is supposed to be signalled with a breaking semver change or not. From what I can gather, pip can install any package on any python interpreter version even if it is not officially declared to be supported. So probably incrementing the major version for that as happened in case of smmap isn't required or even desirable.

I think the same happened in GitPython as well, but I never changed its major version because of that as it would have caused too many questions and churn downstream, and doing it like that worked fine. People only ever showed up here if support for older versions was effectively removed, like happened with the switch from python 2 to python 3, which caused GitPython to change its version number from 2 to 3 as well.

So I think there we have it :): Major version bumps aren't needed to signal the drop of support of EOL python interpreter versions.

EliahKagan commented 10 months ago

From what I can gather, pip can install any package on any python interpreter version even if it is not officially declared to be supported.

I think it can be forced to do so. But it does not do so automatically. I'd say that's fine here, though, because it's fine for people to have the older package. They will be using lots of outdated packages if they're on Python 3.6.

Please note that I'm only talking about required version metadata, which is specified by python_requires in setup.py. (I think the actual metadata is called python_version and python_full_version but in a call to setuptools.setup it's set that way, but I'm not actually clear on the precise details.) I am not talking about classifiers, which don't impose anything.

Byron commented 10 months ago

I am not talking about classifiers, which don't impose anything.

Ah, that's perfect then, as I had these in mind primarily. Then all the effort I caused by bumping major versions was definitely unnecessary (in case of smmap in particular.

EliahKagan commented 10 months ago

Ah, that's perfect then, as I had these in mind primarily. Then all the effort I caused by bumping major versions was definitely unnecessary (in case of smmap in particular.

In smmap 5.0.0, python_requires had the value >=3.6, so your original thinking that this might be breaking made sense (just on account of python_requires, rather than classifiers). But anyway I am inclined to agree this may not need to be regarded as a breaking change; the kind of thing I would want to avoid would be the opposite situation, where pip automatically selects a version so high it does not work on the version of Python being used.