GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fixes to support Python 2.6 again. by andy-maier · Pull Request #541 · gitpython-developers/GitPython

Conversation

andy-maier commented Oct 21, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

andy-maier force-pushed the py26_fixes branch from 1d68375 to 3498c54 Compare

October 21, 2016 10:32

codecov-io commented Oct 21, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

andy-maier force-pushed the py26_fixes branch 4 times, most recently from 7e35290 to a795f40 Compare

October 21, 2016 11:04

Byron added waiting for feedback acknowledged labels

Oct 22, 2016

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Byron added this to the v2.1.1 - Bugfixes milestone

Oct 22, 2016

andy-maier commented Oct 23, 2016

Copy link

Copy Markdown

Contributor Author

Will do, tomorrow.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

andy-maier force-pushed the py26_fixes branch 7 times, most recently from 9364ea0 to 9ff3f02 Compare

October 24, 2016 13:56


          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 force-pushed the py26_fixes branch from 9ff3f02 to f3d5df2 Compare

October 24, 2016 14:08

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ankostis commented Oct 24, 2016

Copy link

Copy Markdown

Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ankostis commented Oct 24, 2016

Copy link

Copy Markdown

Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ankostis commented Oct 25, 2016

Copy link

Copy Markdown

Contributor

Never mind, i will push on top some commits.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Byron merged commit 2f207e0 into gitpython-developers:master

Dec 8, 2016

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ankostis reviewed

Dec 8, 2016

View reviewed changes

Comment thread

git/cmd.py

		# 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()

ankostis Oct 21, 2016

Copy link

Copy Markdown

Contributor

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.

Can you replace it with this more pythonic and quicker to follow (in debug-mode also) code?

_kwargs = dict(k, v) for k, v in kwargs.items() if k in execute_kwargs)
kwargs = dict((k, v) for k, v in kwargs.items() if k not in execute_kwargs)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

requirements.txt

		ddt
		mock No newline at end of file
		mock
		unittest2; python_version < '2.7'

ankostis Oct 21, 2016

Copy link

Copy Markdown

Contributor

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.

Can you modify also setup.py and test-requirements.txt?

if sys.version_info[:2] < (2, 6):
    test_requires.append('unittest2')

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

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

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk mentioned this pull request

Mar 12, 2018

2.0.9 fails on Python 2.6 #540

Closed

Reviewers

1 more reviewer

ankostis

ankostis left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

acknowledged waiting for feedback

Milestone

v2.1.1 - Bugfixes

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

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!

ankostis Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

ankostis Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

ankostis commented Dec 8, 2016

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

andy-maier commented Oct 21, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

codecov-io commented Oct 21, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.