GeezerVison is showing github.com — Remove Python 2 compatibility shims by Harmon758 · Pull Request #979 · gitpython-developers/GitPython
Conversation
Harmon758
commented
Feb 7, 2020
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
3.0.0 was supposed to have removed Python 2 support, but didn't actually remove any of the compatibility shims for Python 2.
In fact, with 3b13c11 (#860), further shims were added for Python 2 and going so far as to add future as a dependency and check for Python <= 2.3 (when sys.getfilesystemencoding was added). I'm not sure why #860 was merged with those shims, especially since it was failing a test due to switching to Unicode strings in Python 2, but this commit that did so improperly merged requirements.txt as well, readding gitdb and ddt as dependencies when they had been removed with ce21f63 (#826), so that setup.py could use requirements.txt. gitdb itself had actually already been removed as a dependency over 2 years before requirements.txt was updated, with 2ca7c01 in v2.0.9.
The commit was then reverted with cc664f0, referencing failing Travis CI tests due to future having been added as a dependency in requirements.txt and the Travis CI configuration installing dependencies from test-requirements.txt, rather than requirements.txt and/or setup.py. The changes were then reintroduced with 74a0507, adding installation of requirements.txt (despite what the commit summary says) to the Travis CI configuration. It was then again reverted with c9dbaab, referencing the failing Travis CI build for the previous commit due to the same failing test for the PR.
Afterwards, for some reason, the changes were yet again reintroduced with dac619e, the very commit that dropped Python 2 support. This time, future wasn't even added to requirements.txt, so Python 2 would have failed to import the library even if the commit hadn't dropped support for it. The changes were again reverted with 913d806 (for the 3rd time) when Python 2 support was brought back after being dropped in a patch version. They were then introduced again (for the 4th time) with 2e7e82b and 3.0.0 was subsequently released with the readded dependencies from almost 3 years ago, leading to #908. #908 was closed with ca080bb in 3.0.1, only removing gitdb as a dependency, leading to #911. #911 was then merged with df5c941 and released as 3.0.2, resolving the dependency issues caused by the improper merge, but leaving the pointless and broken compatibility shims that caused this mess in place.
This PR removes those and other compatibility shims for Python 2 and old Python 3 versions:
- Remove
from builtins import str - This was never even in any release that supported Python 2 and no release ever included future as a dependency.
- Remove check for the existence of
sys.getfilesystemencoding - It has existed since Python 2.3 as mentioned earlier.
- Remove and replace
compat._unichr with chr, comapt.bchr with bytes([...]), compat.binary_type with bytes, compat.bytes_chr with bytes((...,)), compat.FileType with io.IOBase, compat.izip with zip, compat.MAXSIZE with sys.maxsize, compat.mviter with .values(), compat.string_types with str, compat.text_type with str, compat.unicode with str, compat.xrange with range, compat.UnicodeMixin subclass by using __str__ rather than __unicode__
- Remove
compat.byte_ord, compat.PY3, compat.range
- Remove surrogateescape error handler for Python 2
- Remove Python 2 checks for
compat.defenc, TestFun.test_tree_entries_from_data_with_failing_name_decode_py3, TestIndex.test_add_utf8P_path
- Remove Python 2 checks in
Git.__unpack_args, compat.with_metaclass, Diff.__str__, TestRepo.test_untracked_files, Actor._main_actor
- Remove Python 3 checks in
GitConfigParser._read, _octal_repl, run_commit_hook, git.objects.tree, RefLogEntry.__repr__, Repo._get_untracked_files, TestGit.test_call_unpack_args_unicode, TestGit.test_call_unpack_args
- Remove
TestFun.test_tree_entries_from_data_with_failing_name_decode_py2 Python 2 test
- Remove attempt to import
ConfigParser for Python 2 in git.config
- Remove Python 2.7 check in
TestBase
- Remove check for
logging.NullHandler for Python 2.6 in git.util
- Remove Python < 3.3 check for
PermissionError in git.cmd
This PR also fixes the formatting of the version specifier in requirements.txt (so as not to be in parentheses as #826 changed it to be with ce21f63) and improves setup.py's python_requires so as to succinctly specify >=3.4.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
codecov-io
commented
Feb 7, 2020
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Merging #979 into master will increase coverage by 0.92%.
The diff coverage is 97.33%.

@@ Coverage Diff @@
## master #979 +/- ##
==========================================
+ Coverage 93.35% 94.27% +0.92%
==========================================
Files 59 59
Lines 9715 9540 -175
==========================================
- Hits 9069 8994 -75
+ Misses 646 546 -100
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e27496...c5f5911. Read the comment docs.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am thoroughly impressed by your detective work, and the time and energy you must have spent to make this PR happen 🙇🏽♂️. and can't thank you enough for that!
Reading the script to a drama I myself created clearly shows my lack of aptitude in maintaining this project, and only gives a hint on the negative impact this may have had on the users. Despite me trying my best, it's fair to apologize to those I may have put into harms way.
To me nothing of this comes at a surprise, after all I am not writing Python anymore, nor am I using GitPython. This is my call for help to prevent accidents like the above from happening in the future, because I can't make any promises while I am the sole maintainer. No, I even fear it may get worse.
This PR is a step in the right direction - it removes code instead of adding it, and will hopefully be the first of more to come.
In the meantime, any help in reviewing PRs is appreciated, and if you would like to become a maintainer, please let me know 🙏.
Please expect a new release to happen soon.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Harmon758 reacted with thumbs up emoji
Harmon758 reacted with hooray emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
When merging, for some reason, I see this test breaking. I am certain it's related to the test suite not being properly isolated, but hope that given your prior GitPython experience, this makes more sense to you.
If you cannot help, please let me know and I will dig in.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks for the merge and release 🎉
I wouldn't mind helping to review PRs and/or becoming a maintainer.
In the latter case, it might take me a bit to get to know all of the code base, and I'd probably still make PRs for more significant changes, but I actually did have some other minor updates that I'm looking into.
When merging, for some reason, I see this test breaking. I am certain it's related to the test suite not being properly isolated, but hope that given your prior GitPython experience, this makes more sense to you.
Odd, this seems to be a transient issue with the number of reference types for the repo.
I'll look into it, but it seems it hasn't reoccurred yet? At any rate, I don't think it's related to this PR.
|
Byron reacted with heart emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Fantastic news! You should receive an invite momentarily! And I just checked travis, it's all green now. Let's ignore it, I am sure there is more interesting things to do than chasing phantoms :D.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Byron
commented
Feb 8, 2020
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Please take your time to get acquainted, I am happy about any help no matter how small. You are entirely unconstrained as I trust you know what you are doing, so I will never say 'no', but am happy to share my opinion. Once you need a release, just ping me and I will do it with low delay.
Lastly, generally I work off some emails once a month or so (or if there are too many piling up), giving preference to PRs. The more elaborate the PRs, the more likely I will respond quickly (as you might have noticed). This also gives you ample time to pick up PRs or issues that arrive in your inbox, before I do.
In GitPython, I think it's safe to receive notifications for all issues and PR, the volume is quite low with less than 10 notifications per month.
And if there are any questions, or anything really, please reach out right away.
Thanks, and have fun :)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks! Happy to join the team.
Let's ignore it, I am sure there is more interesting things to do than chasing phantoms :D.
Yeah, I'll look into it further if it reoccurs.
You are entirely unconstrained as I trust you know what you are doing, so I will never say know, but am happy to share my opinion.
I generally make branches and/or PRs for substantial changes so that they can be reviewed and/or tested, so I'll probably request reviews for those.
This also gives you ample time to pick up PRs or issues that arrive in your inbox, before I do.
I do maintain and help with other libraries, but I'll try to triage when I have time.
In GitPython, I think it's safe to receive notifications for all issues and PR, the volume is quite low with less than 10 notifications per month.
I'm already watching the repo now 👍
Once you need a release, just ping me and I will do it with low delay.
And if there are any questions, or anything really, please reach out right away.
Is there a good place to contact you besides GitHub? e.g. the email in your commits, Keybase chat, or?
|
Byron reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Probably easiest is to use the (brand new) gitter room: https://gitter.im/gitpython-developers/GitPython . I wouldn't want to advertise it to avoid chatter, so right now it's really just you and me there. Otherwise, the email address in my recent commits will work, too.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Feb 16, 2020
EliahKagan
added a commit
to EliahKagan/GitPython
that referenced
this pull request
Jan 23, 2024
The NullHandler class in git.util was added when merging gitpython-developers#300, to
allow a noop handler to be used on Python 2.6, since the standard
library logging.NullHandler class was added in Python 2.7.
When introduced in d1a9a23, the git.util.NullHandler class was also
patched into the logging module, but that has no longer been done
since 2fced2e (gitpython-developers#979), nor does GitPython make other use of it.
This also changes the parameter type annotation on the emit method
from `object` to `logging.LogRecord`, which is the expeced type.
Reviewers
Byron
Byron approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|