Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(MapWindow): unicode foes in read_into_memory() used by gitpython TCs #30

Merged
merged 1 commit into from Oct 22, 2016

Conversation

@ankostis
Copy link
Contributor

@ankostis ankostis commented Oct 22, 2016

Drop Windows only codepath bypassing memory-mapping due to some leaks in
the past.
Now Appveyor proves everything run ok.
Additionally, this codepath had unicode problems on PY3. So deleting it,
fixes 2 TCs in gitpython:

  • TestRepo.test_file_handle_leaks()
  • TestObjDbPerformance.test_random_access()

See gitpython-developers/GitPython#525

@ankostis
Copy link
Contributor Author

@ankostis ankostis commented Oct 22, 2016

@Byron please delay the release, to include this fix (and maching code from gitpython project).

@coveralls
Copy link

@coveralls coveralls commented Oct 22, 2016

Coverage Status

Changes Unknown when pulling 1bb6ff0 on ankostis:win_mmap into * on gitpython-developers:master*.

@ankostis ankostis force-pushed the ankostis:win_mmap branch from 1bb6ff0 to 91814e6 Oct 22, 2016
@coveralls
Copy link

@coveralls coveralls commented Oct 22, 2016

Coverage Status

Changes Unknown when pulling 91814e6 on ankostis:win_mmap into * on gitpython-developers:master*.

1 similar comment
@coveralls
Copy link

@coveralls coveralls commented Oct 22, 2016

Coverage Status

Changes Unknown when pulling 91814e6 on ankostis:win_mmap into * on gitpython-developers:master*.

@ankostis
Copy link
Contributor Author

@ankostis ankostis commented Oct 22, 2016

But is not ready yet...

Drop Windows only codepath bypassing memory-mapping due to some leaks in
the past. 
Now Appveyor proves everything run ok.  
Additionally, this codepath had unicode problems on PY3. So deleting it,
fixes 2 TCs in gitpython:
+ TestRepo.test_file_handle_leaks()
+ TestObjDbPerformance.test_random_access()

See gitpython-developers/GitPython#525
@ankostis ankostis force-pushed the ankostis:win_mmap branch from 91814e6 to b2eafcc Oct 22, 2016
@coveralls
Copy link

@coveralls coveralls commented Oct 22, 2016

Coverage Status

Changes Unknown when pulling b2eafcc on ankostis:win_mmap into * on gitpython-developers:master*.

ankostis added a commit to ankostis/GitPython that referenced this pull request Oct 22, 2016
Now 2 more TCs pass in Windows:
+ TestRepo.test_file_handle_leaks()
+ TestObjDbPerformance.test_random_access()

See gitpython-developers/smmap#30
@ankostis
Copy link
Contributor Author

@ankostis ankostis commented Oct 22, 2016

It is ok now.

@Byron
Copy link
Member

@Byron Byron commented Oct 22, 2016

Looks good! Thank you.
Interesting that this 'test_read_into_memory' codepath is entirely unused after all.

@Byron Byron merged commit 9b21759 into gitpython-developers:master Oct 22, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 93.85%
Details
@Byron
Copy link
Member

@Byron Byron commented Oct 22, 2016

Released via v2.0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.