◐ Shell
reader mode source ↗
Skip to content

Fixes to support Python 2.6 again.#541

Merged
Byron merged 1 commit into
gitpython-developers:masterfrom
andy-maier:py26_fixes
Dec 8, 2016
Merged

Fixes to support Python 2.6 again.#541
Byron merged 1 commit into
gitpython-developers:masterfrom
andy-maier:py26_fixes

Conversation

@andy-maier

@andy-maier andy-maier commented Oct 21, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

codecov-io commented Oct 21, 2016 •
edited
Loading

Copy link
Copy Markdown

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

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

Byron commented Oct 22, 2016

Copy link
Copy Markdown
Member

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

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

andy-maier commented Oct 23, 2016

Copy link
Copy Markdown
Contributor Author

Will do, tomorrow.

@andy-maier andy-maier force-pushed the py26_fixes branch 7 times, most recently from 9364ea0 to 9ff3f02 Compare October 24, 2016 13:56
@andy-maier
Fixes to support Python 2.6 again. …
f3d5df2 
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

andy-maier commented Oct 24, 2016

Copy link
Copy Markdown
Contributor Author

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

ankostis commented Oct 24, 2016

Copy link
Copy Markdown
Contributor

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

@andy-maier

andy-maier commented Oct 24, 2016

Copy link
Copy Markdown
Contributor Author

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

@ankostis

ankostis commented Oct 24, 2016

Copy link
Copy Markdown
Contributor

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

@andy-maier

andy-maier commented Oct 25, 2016

Copy link
Copy Markdown
Contributor Author

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

ankostis commented Oct 25, 2016

Copy link
Copy Markdown
Contributor

Never mind, i will push on top some commits.

@ankostis

ankostis commented Oct 29, 2016

Copy link
Copy Markdown
Contributor

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 Byron merged commit 2f207e0 into gitpython-developers:master Dec 8, 2016
@Byron

Byron commented Dec 8, 2016

Copy link
Copy Markdown
Member

@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 added a commit to ankostis/GitPython that referenced this pull request Dec 8, 2016
@ankostis
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
@ankostis
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
@ankostis
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
@ankostis
style(cmd): pythonize cmd-args filtering for PY26, improve docstring …
f858c44 
Apply codereview comments of gitpython-developers#541.
@ankostis ankostis mentioned this pull request Dec 8, 2016
Merged
@ankostis

ankostis commented Dec 8, 2016

Copy link
Copy Markdown
Contributor

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.

@hugovk hugovk mentioned this pull request Mar 12, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ankostis ankostis ankostis left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

acknowledged waiting for feedback

Milestone

v2.1.1 - Bugfixes

Development

Successfully merging this pull request may close these issues.

4 participants

@andy-maier @codecov-io @Byron @ankostis