Fixes to support Python 2.6 again. by andy-maier · Pull Request #541 · gitpython-developers/GitPython

andy-maier · 2016-10-21T10:13:24Z

GitPython 2.0.9 has some issues on Python 2.6 (see issue #540).

This PR fixes them. For details, see the commit message.
The Travis runs now pass on all Python versions including 2.6.

Please review.

codecov-io · 2016-10-21T10:32:55Z

Current coverage is 94.51% (diff: 96.72%)

Merging #541 into master will increase coverage by 0.14%

@@ master #541 diff @@
==========================================
Files 63 63 Lines 9882 9920 +38 Methods 0 0 Messages 0 0 Branches 0 0 ==========================================
+ Hits 9326 9376 +50 + Misses 556 544 -12 
Partials 0 0

Powered by Codecov. Last update 5149c80...f3d5df2

Byron · 2016-10-22T11:22:52Z

@andy-maier In the meanwhile, merge conflicts have shown up. I will gladly merge this PR once they are fixed. Thank you.

andy-maier · 2016-10-23T17:14:18Z

Will do, tomorrow.

Details: - Added Python 2.6 again to .travis.yml (it was removed in commit 4486bcb). - Replaced the use of dictionary comprehensions in `git/cmd.py` around line 800 with the code before that change (in commit 25a2ebf). Reason: dict comprehensions were introduced only in Python 2.7. - Changed the import source for `SkipTest` and `skipIf` from `unittest.case` to first trying `unittest` and upon ImportError from `unittest2`. This was done in `git/util.py` and in several testcases. Reason: `SkipTest` and `skipIf` were introduced to unittest only in Python 2.7, and `unittest2` is a backport of `unittest` additions to Python 2.6. - In git/test/lib/helper.py, fixed the definition of `assertRaisesRegex` to work on py26. - For Python 2.6, added the `unittest2` dependency to `requirements.txt` and changed `.travis.yml` to install `unittest2`. Because git/util.py uses SkipTest from unittest/unittest2, the dependency could not be added to `test-requirements.txt`. - Fixed an assertion in `git/test/test_index.py` to also allow a Python 2.6 specific exception message. - In `is_cygwin_git()` in `git/util.py`, replaced `check_output()` with `Popen()`. It was added in Python 2.7. - Enabled Python 2.6 for Windows: - Added Python 2.6 for MINGW in .appveyor.yml. - When defining `PROC_CREATIONFLAGS` in `git/cmd.py`, made use of certain win32 and subprocess flags that were introduced in Python 2.7, dependent on whether we run on Python 2.7 or higher. - In `AutoInterrupt.__del__()` in `git/cmd.py`, allowed for `os` not having `kill()`. `os.kill()` was added for Windows in Python 2.7 (For Linux, it existed in Python 2.6 already).

andy-maier · 2016-10-24T14:25:54Z

The "rebase" got a little larger because I enabled Python 2.6 also for Windows now, and I added a Python 2.6 environment for both Appveyor (MINGW) and Travis (An earlier commit removed it from Travis).

Unlike earlier, Python 2.6 is now mandatory in Travis in this PR. I think this is the best way to keep it working in the future. If and when an unsurmountable issue with Python 2.6 pops up, it can quickly be removed again.

Please review again.

ankostis · 2016-10-24T21:20:57Z

@andy-maier have you taken into account my 2 review comments?

andy-maier · 2016-10-24T22:23:24Z

No, I'm sorry. I was not aware of your comments. Where can I find them?

ankostis · 2016-10-24T23:20:50Z

They are visibile in this PR, a couple of comments above embedded.

andy-maier · 2016-10-25T07:01:11Z

That's where I looked first, but I don't see them. Maybe something behaves different because this is a PR from my fork and I force-pushed to the branch?

Anyway, could you please copy them here?

ankostis · 2016-10-25T09:50:19Z

Never mind, i will push on top some commits.

ankostis · 2016-10-29T12:36:58Z

I'm really having problem working with PY26 - there are still some unofficial python-sites for this release, but are really slow or unmaintained.
Really @andy-maier can you explain again why you need PY26 support?

As I said, this deprecated platform is not only a development burden for us but a security liability for you.

Byron · 2016-12-08T12:25:06Z

@andy-maier Thanks a lot for your contribution, it's much appreciated!

I have had a look at the commit, and believe it's OK to merge under the assumption that we can remove or revert it at any time should there be any issues cropping up, or if it becomes an additional burden to maintain it.
However, looking at the changes, I believe the likelihood of that to actually happen is low enough.

@ankostis Please feel free to revert this commit in case it is a security liability in itself.

ankostis · 2016-10-21T13:16:39Z

 # otherwise these'll end up in args, which is bad.
- _kwargs = {k: v for k, v in kwargs.items() if k in execute_kwargs}
- kwargs = {k: v for k, v in kwargs.items() if k not in execute_kwargs}
+ _kwargs = dict()


Can you replace it with this more pythonic and quicker to follow (in debug-mode also) code?
_kwargs=dict(k, v) fork, vinkwargs.items() ifkinexecute_kwargs) kwargs=dict((k, v) fork, vinkwargs.items() ifknotinexecute_kwargs)

ankostis · 2016-10-21T13:20:28Z

 ddt
-mock
+mock
+unittest2; python_version < '2.7'


Can you modify also setup.py and test-requirements.txt?
ifsys.version_info[:2] < (2, 6): test_requires.append('unittest2')

Apply codereview comments of gitpython-developers#541 (review)

Apply codereview comments of gitpython-developers#541.

ankostis · 2016-12-08T13:52:20Z

No prob, the PR is fine.

Just did a minor pythonism included in #555 to convert a loop as a 2 lines, to facilitate debuging.

andy-maier force-pushed the py26_fixes branch from 1d68375 to 3498c54Compare October 21, 2016 10:32

andy-maier force-pushed the py26_fixes branch 4 times, most recently from 7e35290 to a795f40Compare October 21, 2016 11:04

Byron added waiting for feedback acknowledged labels Oct 22, 2016

Byron added this to the v2.1.1 - Bugfixes milestone Oct 22, 2016

andy-maier force-pushed the py26_fixes branch 7 times, most recently from 9364ea0 to 9ff3f02Compare October 24, 2016 13:56

andy-maier force-pushed the py26_fixes branch from 9ff3f02 to f3d5df2Compare October 24, 2016 14:08

Byron merged commit 2f207e0 into gitpython-developers:masterDec 8, 2016

ankostis reviewed Dec 8, 2016
View reviewed changes

ankostis added a commit to ankostis/GitPython that referenced this pull request Dec 8, 2016
style(cmd): pythonize cmd-args filtering for PY26, improve docstring
82b2e01
Apply codereview comments of gitpython-developers#541 (review)

ankostis added a commit to ankostis/GitPython that referenced this pull request Dec 8, 2016
style(cmd): pythonize cmd-args filtering for PY26, improve docstring
1994cd9
Apply codereview comments of gitpython-developers#541 (review)

ankostis added a commit to ankostis/GitPython that referenced this pull request Dec 8, 2016
style(cmd): pythonize cmd-args filtering for PY26, improve docstring
5366bfd
Apply codereview comments of gitpython-developers#541 (review)

ankostis added a commit to ankostis/GitPython that referenced this pull request Dec 8, 2016
style(cmd): pythonize cmd-args filtering for PY26, improve docstring
f858c44
Apply codereview comments of gitpython-developers#541.

ankostis mentioned this pull request Dec 8, 2016
Retrofit repo class as context-man to cleanup global mman on repo-delete #555
Merged

hugovk mentioned this pull request Mar 12, 2018
2.0.9 fails on Python 2.6 #540
Closed

Uh oh!

Uh oh!

Conversation

andy-maier commented Oct 21, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Oct 21, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 94.51% (diff: 96.72%)

Uh oh!

Byron commented Oct 22, 2016

Uh oh!

andy-maier commented Oct 23, 2016

Uh oh!

andy-maier commented Oct 24, 2016

Uh oh!

ankostis commented Oct 24, 2016

Uh oh!

andy-maier commented Oct 24, 2016

Uh oh!

ankostis commented Oct 24, 2016

Uh oh!

andy-maier commented Oct 25, 2016

Uh oh!

ankostis commented Oct 25, 2016

Uh oh!

ankostis commented Oct 29, 2016

Uh oh!

Byron commented Dec 8, 2016

Uh oh!

ankostisOct 21, 2016

Choose a reason for hiding this comment

Uh oh!

ankostisOct 21, 2016

Choose a reason for hiding this comment

Uh oh!

ankostis commented Dec 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

andy-maier commented Oct 21, 2016•
edited
Loading

codecov-io commented Oct 21, 2016•
edited
Loading